Fix for symbol conflict caused by aliases

Hi,

This patch adds some name mangling to the GNU runtime's selector alias handling. Without it, selectors that have the same name as C symbols cause those symbols to be renamed, breaking linking (for example, a selector named "close" causes the close() function to be renamed @close123, and the linker to complain that _close123 is not a valid symbol). This seems like it might be a bug in LLVM - given a conflict between an internal and external global the internal one should be renamed - but this patch works around the behaviour.

Note that this diff is taken after my other diff, so if that has not been applied it will need to be manually merged (which should be easy, since it only changes two lines).

David

clang.diff (1.09 KB)

Hi David,

This fix is ok, but not the best solution. It would be better to add a new function like CreateRuntimeGlobal but that autorenames the global if there is a conflict. Maybe it should be call CreateAnonymousGlobal? All the global variables created for string constants etc could go through this as well, because they could very well conflict with random other stuff as well.

-Chris

Hi Chris,

This fix is ok, but not the best solution. It would be better to add
a new function like CreateRuntimeGlobal but that autorenames the
global if there is a conflict. Maybe it should be call
CreateAnonymousGlobal? All the global variables created for string
constants etc could go through this as well, because they could very
well conflict with random other stuff as well.

I agree that this is a good solution in general - thinking more carefully about what would be required to make LLVM do the renaming we'd just add one form of complexity to the front end in exchange for this - but in this specific case I'm creating GlobalAliases, rather than GlobalVariables. The only other code in clang that emits aliases is in CodeGenModule::EmitAliasDefinition() and this emits external aliases, so doesn't want them renamed.

Note that the reason that this was a problem is that the @close symbol relating to the close() function was being added to the module after the -close selector, and so was being renamed. A function that renamed the -close selector would need to mangle the selector name to prevent potential conflicts.

I thought initially it would be nice if LLVM could rename the existing, internal, symbol if an external symbol of the same name was added, but I can't see how to do that without making Module::getNamedGlobal() and friends behave in unexpected ways, so clients have to work around this potential conflict whatever LLVM does.

David

P.S. The MakeConstantString() method in CGObjCGNU predates CodeGenModule::GetAddrOfConstantCString() and should go away. I'll make this change soon, but I currently have a lot of outstanding changes for CGObjCGNU waiting for PR4739 to be resolved.