Sancov guard semantics for usage between comdats

Given the following C++ code:


// test.cpp

struct Foo {

int public_foo();

int outside_foo();

[[gnu::always_inline]] int inline_foo() {

int x = outside_foo();

if (x % 17) {

x += 1;

}

return x;

} 

[[gnu::noinline]] int inline_bar1() {

int x = inline_foo();

if (x % 23) {

x += 2;

}

return x;

} 

[[gnu::noinline]] int inline_bar2() {

int x = inline_foo();

if (x % 37) {

x += 3;

}

return x;

}

}; 

int Foo::public_foo() {

return inline_foo() ? inline_bar1() : inline_bar2();

}

Compiling this with clang++ -fsanitize-coverage=trace-pc-guard /tmp/test.cpp -c -O1 generates sancov guards (_sancov_gen.X) that are used outside of their comdat group due to inlining:


@__sancov_gen_.1 = private global [3 x i32] zeroinitializer, section "__sancov_guards", comdat($_ZN3Foo10inline_fooEv)

define dso_local i32 @_ZN3Foo10public_fooEv(%struct.Foo* %0) local_unnamed_addr #0 comdat align 2 {

call void @__sanitizer_cov_trace_pc_guard(i32* getelementptr inbounds ([3 x i32], [3 x i32]* @__sancov_gen_, i64 0, i64 0))

call void asm sideeffect "", ""() #4

; This is from inlining Foo::inline_foo into Foo::public_foo

call void @__sanitizer_cov_trace_pc_guard(i32* getelementptr inbounds ([3 x i32], [3 x i32]* @__sancov_gen_.1, i64 0, i64 0))

...

This can lead to a discarded section error for __sancov_guards when linking this with another TU that contains the prevalent comdat for $_ZN3Foo10public_fooEv. 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, @_sancov_gen.1 is a part of comdat group $_ZN3Foo10inline_fooEv but is being referenced by a symbol in comdat group $_ZN3Foo10public_fooEv.

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 @_sancov_gen.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:

  1. 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

@__sancov_gen_.local = private global [3 x i32] zeroinitializer, section "__sancov_guards"

define dso_local i32 @_ZN3Foo10public_fooEv(%struct.Foo* %0) local_unnamed_addr #0 comdat align 2 {

call void @__sanitizer_cov_trace_pc_guard(i32* getelementptr inbounds ([3 x i32], [3 x i32]* @__sancov_gen_, i64 0, i64 0))

call void asm sideeffect "", ""() #4

; This is from inlining Foo::inline_foo into Foo::public_foo

call void @__sanitizer_cov_trace_pc_guard(i32* getelementptr inbounds ([3 x i32], [3 x i32]* @__sancov_gen_.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

  1. 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::inline_foo, but will be in the same comdat as the function it’s inlined into

@__sancov_gen_.inlined = private global [3 x i32] zeroinitializer, section "__sancov_guards", comdat($_ZN3Foo10public_fooEv)

define dso_local i32 @_ZN3Foo10public_fooEv(%struct.Foo* %0) local_unnamed_addr #0 comdat align 2 {

call void @__sanitizer_cov_trace_pc_guard(i32* getelementptr inbounds ([3 x i32], [3 x i32]* @__sancov_gen_, i64 0, i64 0))

call void asm sideeffect "", ""() #4

; This is from inlining Foo::inline_foo into Foo::public_foo

call void @__sanitizer_cov_trace_pc_guard(i32* getelementptr inbounds ([3 x i32], [3 x i32]* @__sancov_gen_.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

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.

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.