This is an archive of the discontinued LLVM Phabricator instance.

[libFuzzer] Report at most one crash per input.
ClosedPublic

Authored by morehouse on Apr 30 2018, 11:15 AM.

Details

Summary

Fixes https://github.com/google/sanitizers/issues/788/, a deadlock
caused by multiple crashes happening at the same time. Before printing
a crash report, we now test and set an atomic flag. If the flag was
already set, the crash handler returns immediately.

Event Timeline

morehouse created this revision.Apr 30 2018, 11:15 AM
kcc added inline comments.May 1 2018, 12:19 PM
compiler-rt/lib/asan/asan_report.cc
137

Will check-asan pass with this?
__sanitizer_acquire_crash_state is weak, and is not defined w/o libFuzzer, so you should get a null deref here. No?

compiler-rt/lib/fuzzer/FuzzerTracePC.cpp
382 ↗(On Diff #144595)

I was thinking about implementing this function in sanitizer_common, and not making it weak.

morehouse added inline comments.May 1 2018, 12:26 PM
compiler-rt/lib/asan/asan_report.cc
137

check-asan passes. This is defined weakly in sanitizer_common.cc, so it shouldn't be a nullptr.

compiler-rt/lib/fuzzer/FuzzerTracePC.cpp
382 ↗(On Diff #144595)

I defined it weakly in sanitizer_common.cc so that recovery mode shouldn't be affected.

kcc added inline comments.May 1 2018, 12:51 PM
compiler-rt/lib/asan/asan_report.cc
137

I would rather define this function (non-weak) in sanitizer_common and check halt_on_error_ before calling it there.
This will make it work with the recovery mode. Right?

morehouse updated this revision to Diff 144770.May 1 2018, 1:16 PM
  • Define __sanitizer_acquire_crash_state() in sanitizer_common.
morehouse marked an inline comment as done.May 1 2018, 1:16 PM
kcc added inline comments.May 1 2018, 1:21 PM
compiler-rt/lib/sanitizer_common/sanitizer_common.cc
353

redundant empty line

compiler-rt/lib/sanitizer_common/sanitizer_interface_internal.h
59

Since this is used by libFuzzer, it's not an internal interface, but a public one, so move it to include/sanitizer/common_interface_defs.h
Also, make it return int, to make more C-friendly, update the comment accordingly.

compiler-rt/test/fuzzer/AcquireCrashStateTest.cpp
10

Instead of this, include sanitizer/common_interface_defs.h

morehouse updated this revision to Diff 144776.May 1 2018, 1:44 PM
  • Add __sanitizer_acquire_crash_state to public interface.
morehouse marked 3 inline comments as done.May 1 2018, 1:46 PM
morehouse added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_interface_internal.h
59

Still kept this declaration also, following the pattern of other public interfaces that are defined internally.

kcc accepted this revision.May 1 2018, 1:49 PM

LGTM

This revision is now accepted and ready to land.May 1 2018, 1:49 PM
This revision was automatically updated to reflect the committed changes.
morehouse marked an inline comment as done.