[PATCH] D28498: [asan] Make ASan compatible with linker dead stripping on Linux.

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

In https://reviews.llvm.org/D28498#642272, @pcc wrote:

> The `InstrumentGlobals` function is now basically four functions in one at this point. I'm wondering whether this code could be made more readable if we split each target object format into its own function.


I think I agree. We can extract any common code into helpers.



================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:1602
+  // comdats keyed off similarly named internal symbols, one of them will be
+  // discarded. To avoid this, we append a module-unique string to such comdat
+  // names; and when this is not possible, we fall back to the old metadata
----------------
pcc wrote:
> @rnk Can you remind me why this is not an issue for COFF?
In COFF, comdats are not really groups with a string. It's more like, you have N separate comdat sections, one is the "leader", everyone points at the leader by referring to its symbol, and the leader may have static linkage. Basically, because the comdat key is a real symbol with linkage from an object file and not some globally unique string, this isn't a problem.

Also, I thought the point of the LLD change was so that we wouldn't have to do this? I thought the plan was to change our object emission to create a section group without a comdat key.


================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:1608
+  bool UseCOFFComdatMetadata = TargetTriple.isOSBinFormatCOFF();
+  bool UseELFComdatMetadata = ELFUniqueModuleId.size() > 0;
+  bool UseComdatMetadata = UseCOFFComdatMetadata || UseELFComdatMetadata;
----------------
`!ELFUniqueModuleId.empty()` maybe?


Repository:
  rL LLVM

https://reviews.llvm.org/D28498