Modify

Opened 7 years ago

Last modified 5 years ago

#456 new enhancement

Ensure functions are not defined twice

Reported by: Ian Hinder Owned by:
Priority: major Milestone:
Component: Cactus Version:
Keywords: Cc:

Description

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.

Attachments (0)

Change History (7)

comment:1 Changed 7 years ago by Erik Schnetter

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.

comment:2 Changed 6 years ago by Ian Hinder

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

comment:3 Changed 6 years ago by Erik Schnetter

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.

comment:4 Changed 6 years ago by Ian Hinder

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.

comment:5 Changed 5 years ago by Frank Löffler

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?

comment:6 Changed 5 years ago by Erik Schnetter

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   =

comment:7 Changed 5 years ago by Ian Hinder

I prefer these things to be autoconf'd.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
Next status will be 'review'.
as The resolution will be set.
to The owner will be changed from (none) to the specified user.
Next status will be 'confirmed'.
The owner will be changed from (none) to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.