[PATCH] D51902: [SanitizerCoverage] Create comdat for global arrays.

[Matt Morehouse via Phabricator via llvm-commits]

morehouse added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/Instrumentation/SanitizerCoverage.cpp:582
+      std::string ModuleId = getUniqueModuleId(CurModule);
+      Name += ModuleId.empty() ? CurModule->getModuleIdentifier() : ModuleId;
+    }
----------------
eugenis wrote:
> I don't think this is correct - getModuleIdentifier() is even less unique then getUniqueModuleId, and comdat names for local linkage functions really have to be unique.
> 
> Imagine two files with the same relative path, no exported symbols, defining local functions with the same name. We don't want them to share a comdat.
> 
> I think this case needs to fallback to non-comdat non-associated instrumentation (the one that suppressed linker GC).
> 
Ah, I see.  Will revert.

Hopefully suppressing GC for that case won't cause any regressions (e.g., https://github.com/google/sanitizers/issues/971).


Repository:
  rL LLVM

https://reviews.llvm.org/D51902