Ensure functions are not defined twice

Create issue
Issue #456 resolved
Ian Hinder created an issue

When multiple source files define non-static functions with the same name, these are all compiled together into the resulting executable. Which one gets called is not predictable by the user.

This can happen if a user copies a thorn, modifies it slightly, and compiles both thorns into the same configuration. This can lead to very difficult-to-find bugs, and it would be helpful if Cactus was able to prevent this, or at least to mitigate the problem.

At the CST level, Cactus should be able to tell that there are two thorns which schedule functions with the same name. At the moment, this is not caught. I propose that this should be a fatal error, as there is no way to predict which function will be called eventually.

This will solve the problem in some cases, but not in the case where there are instances of duplicate function names which are not scheduled. It should be possible to scan the object/library files using standard tools (nm etc) to determine if there are multiple globally visible symbols with the same name. There might even be standard tools for this purpose. Doing this in a portable way might not be straightforward, but having an implementation for Linux, for example, would catch the majority of cases.

Keyword:

Comments (13)

  1. Erik Schnetter
    • removed comment

    This is quite easy to achieve with GNU ld. We need to add the flag -Wl,--whole-archive before the list of thorns, and -Wl,--no-whole-archive after the list of thorns. This ensures that all routines from all files from all thorns are included in the executable, leading to "duplicate symbols" errors if there are duplicate symbols.

  2. Ian Hinder reporter
    • removed comment

    How easy would it be to add a check to the CST for scheduled functions?

  3. Erik Schnetter
    • removed comment

    That would be straightforward.

    However, I do not think that this is about scheduled functions. Any function defined twice is an error. In most cases, it is a helper function that has a non-unique name when a thorn is copied (e.g. called SpatialDeterminant).

    The best solution would be to check while linking. Most linkers (at least the linux linkers) have options for this (--whole-archive). One would need to introduce new configuration variables, autoconf these, and then uses these while linking.

  4. Ian Hinder reporter
    • removed comment

    The linker fix would catch everything. The CST fix would catch a common cause of error at an earlier stage. I think we should do both.

  5. Frank Löffler
    • removed comment

    So, for the solution using the linker we need an autoconf-test that -Wl,--whole-archive is supported, and then use it in Cactus, right?

  6. Erik Schnetter
    • removed comment

    Yes.

    However, --whole-archive does not work on OS X, but a different mechanism is working there. We can either autoconf both, or allow OS X users to override this in their option list.

    This works on OS X:

    CACTUSLIBLINKLINE         = -L$(CCTK_LIBDIR) -filelist $(CCTK_LIBDIR)/LINKLIST,$(TOP)/build
    BEGIN_WHOLE_ARCHIVE_FLAGS = -L$(CCTK_LIBDIR)   # must not be empty
    END_WHOLE_ARCHIVE_FLAGS   =
    
  7. Roland Haas

    This just happened again in ticket #2101 the --whole-archive flags should be added to at least all Linux clusters (and the Jenkins system). Cannot be added to generic.cfg since it is now used by both Linux and OSX. It may be good to check if --whole-archive-flags can be made to work by using a different linker (we require MacPorts or HomeBrew anyway).

  8. Log in to comment