[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;
+    }
----------------
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D51902