This is an archive of the discontinued LLVM Phabricator instance.

[SanitizerCoverage] Add associated metadata to pc-tables.
ClosedPublic

Authored by morehouse on Jun 14 2018, 5:44 PM.

Details

Summary

Using associated metadata rather than llvm.used allows linkers to
perform dead stripping with -fsanitize-coverage=pc-table. Unfortunately
in my local tests, LLD was the only linker that made use of this metadata.

Partially addresses https://bugs.llvm.org/show_bug.cgi?id=34636 and fixes
https://github.com/google/sanitizers/issues/971.

Event Timeline

morehouse created this revision.Jun 14 2018, 5:44 PM
morehouse edited the summary of this revision. (Show Details)Jun 14 2018, 5:45 PM
morehouse updated this revision to Diff 151449.Jun 14 2018, 6:47 PM
  • Require LLD for gc-sections.test
Dor1s added a comment.Jun 14 2018, 8:06 PM

Nice, thanks for the quick patch, Matt!

Did you try BFD with more than one function? There was a fun bug where the "first" (for some definition of the word) input section of an output section was treated differently from the rest.

llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
601–602

I think pctable needs to be in llvm.compiler.used.
!associated alone will not prevent it being thrown away.

morehouse updated this revision to Diff 151525.Jun 15 2018, 9:46 AM
morehouse marked an inline comment as done.
  • Add pc-table to llvm.compiler.used.

Neither BFD (version 2.30) nor gold (version 1.15) do dead stripping on my machine, even with multiple unused functions.

eugenis added inline comments.Jun 15 2018, 9:50 AM
llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
408

this is unnecessary, llvm.used is strictly stronger than llvm.compiler.used

morehouse added inline comments.Jun 15 2018, 9:53 AM
llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
408

I'm not sure I understand... We're appending different things to each list.

eugenis added inline comments.Jun 15 2018, 9:59 AM
llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
408

Right. Sorry.
A different question then: why only MachO?

morehouse added inline comments.Jun 15 2018, 10:03 AM
llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
408
eugenis added inline comments.Jun 15 2018, 10:30 AM
llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
408

I don't understand that thread :(
What I know is, without compiler.used, neither comdat nor !associated nor a named section would save pctable from the optimizer. Try passing this through opt -O3:

$a = comdat any
@a = global i32 0, align 4, comdat($a)
@b = internal global i32 0, align 4, section "abc", !associated !0, comdat($a)
; @llvm.compiler.used = appending global [1 x i8*] [i8* bitcast (i32* @b to i8*)]
!0 = !{i32 *@a}

morehouse added inline comments.Jun 15 2018, 12:40 PM
llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
408

clang++ -fsanitize=fuzzer -O3 -S -emit-llvm -o- shows the pc-tables still there. But if I run the output of that through opt -O3, the pc-tables get stripped.

So maybe running directly through opt performs some stripping that otherwise wouldn't be done?

eugenis added inline comments.Jun 15 2018, 12:41 PM
llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
408

Most optimizations are done before fuzzer instrumentation.
Try with MSan, it adds a few more at the end.

morehouse added inline comments.Jun 15 2018, 1:00 PM
llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
408

With -fsanitize=fuzzer,memory -O3, the pc-tables are still there.

But either way, I don't think it would hurt to add pc-tables to llvm.compiler.used on all platforms.

morehouse updated this revision to Diff 151547.Jun 15 2018, 1:04 PM
  • Add pc-table to llvm.compiler.used on all platforms.
morehouse marked an inline comment as done.Jun 15 2018, 1:05 PM
eugenis accepted this revision.Jun 15 2018, 1:10 PM
This revision is now accepted and ready to land.Jun 15 2018, 1:10 PM
This revision was automatically updated to reflect the committed changes.