make -fsanitize-coverage=pc-table friendly with -ffunction-sections -Wl,-gc-sections

[kcc]

Bugzilla Link	34636
Version	unspecified
OS	Linux
CC	@compnerd,@efriedma-quic,@eugenis,@froydnj,@bogner,@glandium,@Dor1s,@pcc,@vedantk,@yuanfang-chen

Extended Description

[discussed previously at http://lists.llvm.org/pipermail/llvm-dev/2017-September/117315.html]

On linux we have two problems with -fsanitize=fuzzer and -ffunction-sections -Wl,-gc-sections

Test for these: projects/compiler-rt/test/fuzzer/GcSectionsTest.cpp

First problem:

% clang -std=c++11 -ffunction-sections -Wl,-gc-sections -fsanitize=fuzzer GcSectionsTest.cpp
/tmp/GcSectionsTest-065286.o: In function sancov.module_ctor': GcSectionsTest.cpp:(.text.sancov.module_ctor[sancov.module_ctor]+0x22): undefined reference to __start___sancov_pcs'
GcSectionsTest.cpp:(.text.sancov.module_ctor[sancov.module_ctor]+0x2c): undefined reference to `__stop___sancov_pcs'

Here, the table produced by -fsanitize-coverage=pc-table __sancov_pcs gets dropped by the bfd linker (on Ubuntu 14.04).

With a better linker (-fuse-ld=gold or -fuse-ld=lld) this doesn't happen,
but we still may need to find a workaround suitable for old ld. Or maybe not.

Second:

% clang -std=c++11 -ffunction-sections -Wl,-gc-sections -fsanitize=fuzzer GcSectionsTest.cpp -fuse-ld=lld && nm a.out | grep Unused
000000000023f590 t UnusedFunctionShouldBeRemovedByLinker

here, gc-sections is essentially disabled because -fsanitize-coverage=pc-table makes all functions used.

eugenis@ suggests that all we need here is to use https://llvm.org/docs/LangRef.html#associated-metadata to mark the pc-table as associated with it's function.

This is important, we need gc-sections to work.
But once we fix it, we'll need a test (in test/fuzzer/gc-sections.test)
but the test won't work with bfd linker, see above.

[kcc]

assigned to @morehouse

[efriedma-quic]

I think the first issue is the same issue I ran into with source-based code coverage. (https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#drawbacks-and-limitations)

[bogner]

I think the first issue is the same issue I ran into with source-based code coverage.

Yes, the pc-table handling is done in the same way as clang's PGO and coverage counter mapping. I believe this also applies to clang's IR level PGO profiling.

[vedantk]

here, gc-sections is essentially disabled because
-fsanitize-coverage=pc-table makes all functions used.

eugenis@ suggests that all we need here is to use
https://llvm.org/docs/LangRef.html#associated-metadata to mark the pc-table
as associated with it's function.

I haven't tried this, but I'll note that with ld64 we use the "live_support" section modifier to do this (see llvm/r283947 for an example). This approach might be more convenient than attaching metadata to each sancov global, but would require broad linker support.

[kcc]

From compnerd@:
It can be made as aggressive. You need to compile with -ffunction-sections -fdata-sections and then link with -Xlinker --gc-sections. ld64 is more aggressive due to .subsections_via_symbols (aka the two compile flags).

[llvmbot]

If I understand this bug correctly, we expect the BFD ld linker to generate this failure and don't have a short-term solution?

In the meantime, can we bind LLVM_USE_SANITIZE_COVERAGE to gold/lld? Or generate a warning/error if LLVM_USE_LINKER==ld && LLVM_USE_SANITIZE_COVERAGE? I suppose that won't quite work if ld is ld.gold on some systems. But is there something we can do to make this less likely to blow up in this mysterious way?

[efriedma-quic]

If I understand this bug correctly, we expect the BFD ld linker to generate this failure and don't have a short-term solution?

There are solutions: you can upgrade to binutils 2.26 or newer, or turn off --gc-sections. But I don't think anyone is working on a solution to get the particular combination of an old linker with gc-sections working, no. (Might be possible with something like fake global references, but it would be ugly.)

For LLVM_USE_SANITIZE_COVERAGE in particular, it would probably be easy to make the LLVM build system itself detect this situation?

[morehouse]

Partial fix for problem #​2: https://reviews.llvm.org/D48203.

Adds associated metadata as suggested by @​eugenis. In my local tests, LLD made use of the metadata and stripped unused functions. However my versions of BFD (2.30) and Gold (1.15) did not do any stripping.

[Dor1s]

Just hit this issue in Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=926588

Had to disable pc-table instrumentation as a workaround for now, as sizes of the binaries grew like 2-5x.

[froydnj]

We hit this error when trying to upgrade our Firefox fuzzing builds to clang 8: https://bugzilla.mozilla.org/show_bug.cgi?id=1541943

We are using a relatively new version of binutils ld to link everything. We haven't investigated much further than that.

[llvmbot]

mentioned in issue llvm/llvm-bugzilla-archive#35674

[morehouse]

mentioned in issue llvm/llvm-bugzilla-archive#43147

[llvmbot]

mentioned in issue llvm/llvm-bugzilla-archive#43435

[morehouse]

I believe this has been fixed for LLD for a while. Other linkers still have issues due to differences in how linkers do dead stripping.

[nickdesaulniers]

Perhaps it's time to clean up the comment in https://llvm.org/docs/FuzzingLLVM.html#configuring-llvm-to-build-fuzzers that links to this closed issue?