[PATCH] D53234: Don't remove COMDATs when internalizing global objects

[Aaron Hill via Phabricator via llvm-commits]

Aaron1011 added a comment.

> Intuitively, we would not expect comdats in LTO object files as they are not required.

I'm not certain what you mean by 'not required'. My view is that LLVM should always respect COMDATs in the IR (e.g. keep them in place, and respect removal rules),
since it can't know the exact reason why they were added in the IR. Leaving the COMDAT in place is always a correct option, but removing the COMDAT might break
things that aren't expected it.

> LTO can remove the comdats (similarly, so can the system linker)

Yes, but it needs to follow the rules set out in the spec:

In the ELF spec, §12 states that "these groups are included, or these groups are omitted, from the linked object as a unit" <https://docs.oracle.com/cd/E23824_01/html/819-0690/chapter7-26.html>.

When the linker removes COMDATS, it ensures that it either removes all of the members, or none of them.
Right now, LLVM is removing the COMDAT group, but leaving all of its members intact - something that the system linker will never do

> Presumably, such globals will contain references to the "associated" global which will also anchor those globals.

This is not the case for CoverageSanitizer and AddressSanitizer. The globals with !associated metadata are simply global variables.
There is a 'usage link' from the instrumented function to the global (since the function writes/reads from the global), but the only link from the global back to the function is the !associated
metadata and the COMDAT.

> Can you post the output of the bfd linker, and a testcase which reproduces the failure with bfd?

I've created a self-contained example at https://github.com/Aaron1011/llvm_lto_ir
It should fail without my patch, and succeed with it.


https://reviews.llvm.org/D53234