- removed comment
Ensure functions are not defined twice
Issue #456
resolved
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)
-
-
reporter
- removed comment
How easy would it be to add a check to the CST for scheduled functions?
-
- 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.
-
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.
-
- 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?
-
- 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 = -
reporter
- removed comment
I prefer these things to be autoconf'd.
-
This just happened again in ticket
#2101the--whole-archiveflags 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-flagscan be made to work by using a different linker (we require MacPorts or HomeBrew anyway). -
Pull request is here: https://bitbucket.org/cactuscode/cactus/pull-requests/56/rhaas-duplicate-symbols/diff
-
- edited description
- changed status to open
Ready for review.
-
Issue
#1553was marked as a duplicate of this issue. -
Unless objected I will push after 2019-06-28.
-
- changed status to resolved
- Log in to 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.