[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:
> morehouse wrote:
> > morehouse wrote:
> > > 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).
> > Hmm... That approach gives me link errors again:
> > 
> > ```
> > /usr/bin/ld: __sancov_pcs has both ordered [`__sancov_pcs[SSL_COMP_get_name]' in BUILD/libssl.a(ssl_ciph.o)] and unordered [`__sancov_pcs' in BUILD/libssl.a(ssl_ciph.o)] sections
> > ```
> > 
> > I guess the linker doesn't like having some arrays in comdats and some not in comdats within the same section.
> Never seen this error before. Does the linker treat names with a dot as ordered by the part of the name that follows the dot? What if you replace the dot with something else?
I don't think there's any dots in the comdat name or the section name...  In the case above the section name is `__sancov_pcs` and the comdat name is `SSL_COMP_get_name`.


Repository:
  rL LLVM

https://reviews.llvm.org/D51902