Page MenuHomePhabricator

[SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer
ClosedPublic

Authored by morehouse on Aug 25 2017, 11:31 AM.

Diff Detail

Event Timeline

morehouse created this revision.Aug 25 2017, 11:31 AM
kcc edited edge metadata.Aug 25 2017, 11:49 AM

Did you check this on something other than the unit tests?
E.g. a couple of benchmarks from fuzzer-test-suite?

llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
177

we already have a linear scan in SanitizerCoverageModule::runOnFunction -- don't introduce a second one.
Also, there is InvokeInst in addition to CallInst, see the loop in runOnFunction.

You can simply extend the loop in runOnFunction to set a flag if the function has non-intrin calls/ invokes

In D37156#852780, @kcc wrote:

Did you check this on something other than the unit tests?
E.g. a couple of benchmarks from fuzzer-test-suite?

Just tested on the proj4 and lcms benchmarks and no issues came up.

morehouse updated this revision to Diff 112739.Aug 25 2017, 1:43 PM
  • Use existing linear scan, and check for InvokeInst.
morehouse marked an inline comment as done.Aug 25 2017, 1:44 PM
kcc accepted this revision.Aug 25 2017, 2:07 PM

LGTM

This revision is now accepted and ready to land.Aug 25 2017, 2:07 PM
This revision was automatically updated to reflect the committed changes.
morehouse reopened this revision.Aug 25 2017, 3:47 PM

Turns out I should have been testing the benchmarks with FUZZING_ENGINE=fsanitize_fuzzer. My mistake.

After adding the weak reference to SanitizerCoverage.cpp, both lcms and proj4 build with fsanitize_fuzzer.

This revision is now accepted and ready to land.Aug 25 2017, 3:47 PM
morehouse updated this revision to Diff 112756.Aug 25 2017, 3:49 PM
  • Add weak reference in SanitizerCoverage.cpp
morehouse updated this revision to Diff 112759.Aug 25 2017, 3:56 PM

Full diff.

  • Add weak definition of __sancov_lowest_stack to runtime.

+George, in case he knows about attribute((tls_model("initial-exec"))) on Mac

compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc
218 ↗(On Diff #112923)

I wonder if this going to work on Mac.

I don't, but I can check whether tests pass.

@kcc I've disabled the relevant test on Mac in r311916, please revert my change once this CR goes through.

  • Disable stack depth tracking on Mac.
morehouse edited the summary of this revision. (Show Details)Aug 29 2017, 11:39 AM
kcc added inline comments.Aug 29 2017, 11:41 AM
clang/lib/Driver/SanitizerArgs.cpp
297

please use if(SomeCondition) instead of #if

In general: 99% of cases where you may want to use #if -- you shouldn't

compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc
20 ↗(On Diff #113129)

no standard hearder in this files. Just use the 'uptr' type.

  • Eliminate "#if".
  • Replace uintptr_t with uptr.
morehouse marked 2 inline comments as done.Aug 29 2017, 12:17 PM
kcc accepted this revision.Aug 29 2017, 12:47 PM

LGTM

This revision was automatically updated to reflect the committed changes.
morehouse reopened this revision.Aug 29 2017, 5:26 PM
This revision is now accepted and ready to land.Aug 29 2017, 5:26 PM
morehouse updated this revision to Diff 113177.Aug 29 2017, 5:26 PM
  • Only enable stack depth tracking on Linux.
  • Ignore __sancov_lowest_stack in interface symbols tests.
morehouse edited the summary of this revision. (Show Details)Aug 29 2017, 5:32 PM
This revision was automatically updated to reflect the committed changes.