[llvm-dev] Sancov guard semantics for usage between comdats (original) (raw)

Evgenii Stepanov via llvm-dev llvm-dev at lists.llvm.org
Thu May 14 12:40:44 PDT 2020

Running coverage pass before inlining seems suboptimal. Perhaps the easiest solution is to run it later, at the end of the IR pipeline but still before the sanitizer passes.

Alternatively, we could teach the inliner to merge comdat groups. I.e. if function A is inlined into function B, both with comdats, it makes sense for it to bring all other members of its comdat into B.

On Thu, May 14, 2020 at 12:13 PM Leonard Chan via llvm-dev <llvm-dev at lists.llvm.org> wrote:

Forgot to mention, but was probably implied by the patch I linked: This is reproducible only when using the new pass manager. The clang invocation should be: _ _clang++ -fsanitize-coverage=trace-pc-guard /tmp/test.cpp -c -O1 -fexperimental-new-pass-manager_ _ The clang we build uses it by default. On Thu, May 14, 2020 at 12:08 PM Leonard Chan <leonardchan at google.com> wrote:

Given the following C++ code:

_ _// test.cpp_ _struct Foo {_ _int publicfoo();_ _int outsidefoo();_ _[[gnu::alwaysinline]] int inlinefoo() {_ _int x = outsidefoo();_ _if (x % 17) {_ _x += 1;_ _}_ _return x;_ _}_ _[[gnu::noinline]] int inlinebar1() {_ _int x = inlinefoo();_ _if (x % 23) {_ _x += 2;_ _}_ _return x;_ _}_ _[[gnu::noinline]] int inlinebar2() {_ _int x = inlinefoo();_ _if (x % 37) {_ _x += 3;_ _}_ _return x;_ _}_ _};_ _int Foo::publicfoo() {_ _return inlinefoo() ? inlinebar1() : inlinebar2();_ _}_ _ _Compiling this with clang++ -fsanitize-coverage=trace-pc-guard /tmp/test.cpp -c -O1 generates sancov guards (sancovgen.X) that are used outside of their comdat group due to inlining: _ _@_sancovgen.1 = private global [3 x i32] zeroinitializer, section "_sancovguards", comdat($ZN3Foo10inlinefooEv)_ _define dsolocal i32 @ZN3Foo10publicfooEv(%struct.Foo* %0) localunnamedaddr #0 comdat align 2 {_ _call void @_sanitizercovtracepcguard(i32* getelementptr inbounds ([3 x i32], [3 x i32]* @_sancovgen, i64 0, i64 0))_ _call void asm sideeffect "", ""() #4_ _; This is from inlining Foo::inlinefoo into Foo::publicfoo_ _call void @_sanitizercovtracepcguard(i32* getelementptr inbounds ([3 x i32], [3 x i32]* @_sancovgen.1, i64 0, i64 0))_ _..._ _ This can lead to a discarded section error for _sancovguards when linking this with another TU that contains the prevalent comdat for $ZN3Foo10publicfooEv. We see this here when building with sancov. _The underlying issue seems to be that symbols in a comdat group that aren’t the key symbol are being referenced in other comdat groups. In this case, @sancovgen.1 is a part of comdat group ZN3Foo10inlinefooEvbutisbeingreferencedbyasymbolincomdatgroupZN3Foo10inlinefooEv but is being referenced by a symbol in comdat group ZN3Foo10inlinefooEvbutisbeingreferencedbyasymbolincomdatgroupZN3Foo10publicfooEv. _We’re seeing this when building libc on fuchsia which uses the new pass manager. The commit that seemed to trigger this is https://reviews.llvm.org/D79698 which (I believe) would run the Sancov pass before optimizations, and potentially cause inlining of @sancovgen.1 into functions outside of the comdat group it’s defined in. Is this explanation correct? To be consistent with the compiler’s behavior, we could: Change the instrumentation semantics with a single sancov guard for both all inlined instances, forcing it to be outside a comdat group: _ _; Not in a comdat_ _@_sancovgen.local = private global [3 x i32] zeroinitializer, section "_sancovguards"_ _define dsolocal i32 @ZN3Foo10publicfooEv(%struct.Foo* %0) localunnamedaddr #0 comdat align 2 {_ _call void @_sanitizercovtracepcguard(i32* getelementptr inbounds ([3 x i32], [3 x i32]* @_sancovgen, i64 0, i64 0))_ _call void asm sideeffect "", ""() #4_ _; This is from inlining Foo::inlinefoo into Foo::publicfoo_ _call void @_sanitizercovtracepcguard(i32* getelementptr inbounds ([3 x i32], [3 x i32]* @_sancovgen.local, i64 0, i64 0))_ _..._ _ This is likely incompatible with the intended semantics of instrumentation. The sancov tool expects every instrumentation call site to be a potential PC sample, and the way the runtime works is to deliver at most one PC sample for each guard. If multiple call sites share a single guard, then the sancov tool will consider only one of those instrumentation sites to have been covered even when more were actually run. Or Have a separate sancov guard for each instantiation of the inlined function and each inline gets its own separate object inside its group just like it gets its own separate instruction stream inside its group: _ _; This was inlined from Foo::inlinefoo, but will be in the same comdat as the function it’s inlined into_ _@_sancovgen.inlined = private global [3 x i32] zeroinitializer, section "_sancovguards", comdat($ZN3Foo10publicfooEv)_ _define dsolocal i32 @ZN3Foo10publicfooEv(%struct.Foo* %0) localunnamedaddr #0 comdat align 2 {_ _call void @_sanitizercovtracepcguard(i32* getelementptr inbounds ([3 x i32], [3 x i32]* @_sancovgen, i64 0, i64 0))_ _call void asm sideeffect "", ""() #4_ _; This is from inlining Foo::inlinefoo into Foo::publicfoo_ _call void @_sanitizercovtracepcguard(i32* getelementptr inbounds ([3 x i32], [3 x i32]* @_sancovgen.inlined, i64 0, i64 0))_ _..._ _ This would be consistent with the semantics of the instrumentation as they originally existed and that the runtime and tools were designed for. Thanks, Leonard Chan

LLVM Developers mailing list llvm-dev at lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

More information about the llvm-dev mailing list