Commit ddd48e32 authored by Max Moroz's avatar Max Moroz Committed by Commit Bot

Reland "Migrate to -fsanitize=fuzzer-no-link when use_fuzzing_engine=true".

-fsanitize=fuzzer and -fsanitize=fuzzer-no-link are two compilation
flags that enable coverage instrumentation needed for libFuzzer.

The instrumentation has more stuff under the hood compared to
-fsanitize=trace-pc-guard. Also, it can be changing over time
without a need to update GN flags again and again (e.g. move from
edge to trace-pc-guard or something like that).

Bug: 764514
Change-Id: I53bf5a3355335f4f627e9024b7ed7fe601c9ecfd

> Revert "Migrate to -fsanitize=fuzzer-no-link when use_fuzzing_engine=true."
>
> This reverts commit c1406d52.
>
> Reason for revert: The builds are failing on linking of some fuzzers: https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.fyi%2FLibfuzzer_Upload_Linux_ASan%2F7088%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout
>
> Original change's description:
> > Migrate to -fsanitize=fuzzer-no-link when use_fuzzing_engine=true.
> >
> > -fsanitize=fuzzer and -fsanitize=fuzzer-no-link are two compilation
> > flags that enable coverage instrumentation needed for libFuzzer.
> >
> > The instrumentation has more stuff under the hood compared to
> > -fsanitize=trace-pc-guard. Also, it can be changing over time
> > without a need to update GN flags again and again (e.g. move from
> > edge to trace-pc-guard or something like that).
> >
> > Bug: 764514
> > Change-Id: I48ef328dee49a9620a1b44bd5cd920f116e1bc1b
> > Reviewed-on: https://chromium-review.googlesource.com/802395
> > Commit-Queue: Max Moroz <mmoroz@chromium.org>
> > Reviewed-by: Oliver Chang <ochang@chromium.org>
> > Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#522505}

Change-Id: I53bf5a3355335f4f627e9024b7ed7fe601c9ecfd
Reviewed-on: https://chromium-review.googlesource.com/846100Reviewed-by: default avatarDirk Pranke <dpranke@chromium.org>
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526592}
parent 39eaaaf0
...@@ -200,7 +200,13 @@ config("default_sanitizer_ldflags") { ...@@ -200,7 +200,13 @@ config("default_sanitizer_ldflags") {
ldflags += [ "-fsanitize=vptr" ] ldflags += [ "-fsanitize=vptr" ]
} }
if (use_sanitizer_coverage) { if (use_fuzzing_engine) {
ldflags += [
"-fsanitize=fuzzer-no-link",
# Disable PC-Table coverage as per https://crbug.com/764514#c21.
"-fno-sanitize-coverage=pc-table",
]
} else if (use_sanitizer_coverage) {
ldflags += [ "-fsanitize-coverage=$sanitizer_coverage_flags" ] ldflags += [ "-fsanitize-coverage=$sanitizer_coverage_flags" ]
} }
...@@ -397,8 +403,14 @@ config("cfi_icall_generalize_pointers") { ...@@ -397,8 +403,14 @@ config("cfi_icall_generalize_pointers") {
config("coverage_flags") { config("coverage_flags") {
cflags = [] cflags = []
if (use_fuzzing_engine) {
if (use_sanitizer_coverage) { cflags += [
"-fsanitize=fuzzer-no-link",
# Disable PC-Table coverage as per https://crbug.com/764514#c21.
"-fno-sanitize-coverage=pc-table",
]
defines = [ "SANITIZER_COVERAGE" ]
} else if (use_sanitizer_coverage) {
cflags += [ cflags += [
"-fsanitize-coverage=$sanitizer_coverage_flags", "-fsanitize-coverage=$sanitizer_coverage_flags",
"-mllvm", "-mllvm",
......
...@@ -96,8 +96,6 @@ declare_args() { ...@@ -96,8 +96,6 @@ declare_args() {
# Value for -fsanitize-coverage flag. Setting this causes # Value for -fsanitize-coverage flag. Setting this causes
# use_sanitizer_coverage to be enabled. # use_sanitizer_coverage to be enabled.
# Default value when unset and use_afl=true or use_libfuzzer=true:
# trace-pc-guard
# Default value when unset and use_sanitizer_coverage=true: # Default value when unset and use_sanitizer_coverage=true:
# trace-pc-guard,indirect-calls # trace-pc-guard,indirect-calls
sanitizer_coverage_flags = "" sanitizer_coverage_flags = ""
...@@ -138,9 +136,7 @@ if (current_toolchain != default_toolchain) { ...@@ -138,9 +136,7 @@ if (current_toolchain != default_toolchain) {
# declare_args block. User overrides are only applied at the end of a # declare_args block. User overrides are only applied at the end of a
# declare_args block. # declare_args block.
declare_args() { declare_args() {
# Enable -fsanitize-coverage. use_sanitizer_coverage = false
use_sanitizer_coverage =
use_libfuzzer || use_afl || sanitizer_coverage_flags != ""
# Detect overflow/underflow for global objects. # Detect overflow/underflow for global objects.
# #
...@@ -148,9 +144,19 @@ declare_args() { ...@@ -148,9 +144,19 @@ declare_args() {
asan_globals = !is_mac asan_globals = !is_mac
} }
if ((use_afl || use_libfuzzer) && sanitizer_coverage_flags == "") { # Whether we are doing a fuzzer build. Normally this should be checked instead
sanitizer_coverage_flags = "trace-pc-guard" # of checking "use_libfuzzer || use_afl" because often developers forget to
} else if (use_sanitizer_coverage && sanitizer_coverage_flags == "") { # check for "use_afl".
use_fuzzing_engine = use_libfuzzer || use_afl
assert(
!(use_fuzzing_engine &&
(use_sanitizer_coverage || sanitizer_coverage_flags != "")),
"Sanitizer coverage (either use_sanitizer_coverage or " +
"sanitizer_coverage_flags) should not be used if use_fuzzing_engine " +
"is true, i.e. when libFuzzer or AFL is being used.")
if (use_sanitizer_coverage && sanitizer_coverage_flags == "") {
sanitizer_coverage_flags = "trace-pc-guard,indirect-calls" sanitizer_coverage_flags = "trace-pc-guard,indirect-calls"
} }
...@@ -161,11 +167,6 @@ using_sanitizer = ...@@ -161,11 +167,6 @@ using_sanitizer =
is_asan || is_lsan || is_tsan || is_msan || is_ubsan || is_ubsan_null || is_asan || is_lsan || is_tsan || is_msan || is_ubsan || is_ubsan_null ||
is_ubsan_vptr || is_ubsan_security || use_sanitizer_coverage || use_cfi_diag is_ubsan_vptr || is_ubsan_security || use_sanitizer_coverage || use_cfi_diag
# Whether we are doing a fuzzer build. Normally this should be checked instead
# of checking "use_libfuzzer || use_afl" because often developers forget to
# check for "use_afl".
use_fuzzing_engine = use_libfuzzer || use_afl
assert(!using_sanitizer || is_clang, assert(!using_sanitizer || is_clang,
"Sanitizers (is_*san) require setting is_clang = true in 'gn args'") "Sanitizers (is_*san) require setting is_clang = true in 'gn args'")
......
...@@ -25,7 +25,7 @@ if (concurrent_links == -1) { ...@@ -25,7 +25,7 @@ if (concurrent_links == -1) {
"--mem_per_link_gb=10", "--mem_per_link_gb=10",
"--reserve_mem_gb=10", "--reserve_mem_gb=10",
] ]
} else if (use_sanitizer_coverage) { } else if (use_sanitizer_coverage || use_fuzzing_engine) {
# Sanitizer coverage instrumentation increases linker memory consumption # Sanitizer coverage instrumentation increases linker memory consumption
# significantly. # significantly.
_args = [ "--mem_per_link_gb=16" ] _args = [ "--mem_per_link_gb=16" ]
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment