Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
kcc opened this issue Sep 15, 2017 · 15 comments
Closed
Assignees
Labels
bugzilla Issues migrated from bugzilla

Comments

@kcc
Copy link
Contributor

kcc commented Sep 15, 2017

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
Copy link
Contributor Author

kcc commented Sep 15, 2017

assigned to @morehouse

@efriedma-quic
Copy link
Collaborator

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
Copy link
Contributor

bogner commented Sep 16, 2017

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
Copy link
Collaborator

vedantk commented Sep 16, 2017

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
Copy link
Contributor Author

kcc commented Sep 19, 2017

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
Copy link
Collaborator

llvmbot commented Oct 24, 2017

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Member

Dor1s commented Jan 30, 2019

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
Copy link
Contributor

froydnj commented Apr 4, 2019

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
Copy link
Collaborator

llvmbot commented Nov 27, 2021

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

@morehouse
Copy link
Collaborator

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

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 27, 2021

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

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@morehouse
Copy link
Collaborator

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
Copy link
Member

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

9 participants