Closed Bug 1483780 Opened 4 years ago Closed 4 years ago

--enable-fuzzing cannot be build stand-alone (ASAN-less)


(Toolkit Graveyard :: Build Config, defect)

Not set


(firefox64 fixed)

Tracking Status
firefox64 --- fixed


(Reporter: u473386, Assigned: u473386)



(2 files, 4 obsolete files)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.139 Safari/537.36

Steps to reproduce:

oss-fuzz has a build mode called profile, which provides clang coverage.

The following mozconfig works. C(XX)FLAGS is set by oss-fuzz as described in the link.

ac_add_options --disable-debug
ac_add_options --disable-elf-hack
ac_add_options --disable-jemalloc
ac_add_options --disable-crashreporter
ac_add_options --enable-fuzzing
ac_add_options --enable-optimize=-O1
ac_add_options --enable-debug-symbols=-gline-tables-only
ac_add_options --enable-address-sanitizer
mk_add_options MOZ_OBJDIR=${OBJDIR}
mk_add_options MOZ_MAKE_FLAGS=-j$(nproc)

Ideally, to prevent test cases which trigger ASAN from breaking the report, it should also work with this line commented.

# ac_add_options --enable-address-sanitizer

It doesn't. There are many linking errors like this.

mozilla-central/js/src/frontend/BinSource.cpp:2988: error: undefined reference to '__sanitizer_cov_trace_const_cmp1'

I tried setting LDFLAGS accordingly, but it doesn't work.
This is not particularly urgent, because oss-fuzz hasn't yet switched to clang coverage from sanitizer coverage.
Flags: needinfo?(choller)
I am unsure what the resulting build is supposed to do. Building with --enable-fuzzing and Clang enables libFuzzer. libFuzzer cannot be built without ASan (that is not a mozilla-central restriction) because it depends on the sanitizer coverage.
Flags: needinfo?(choller)
It can be build without ASAN, the coverage comes from -fsanitize=fuzzer-no-link (or -fsanitize-coverage flags).

In mode profile, oss-fuzz just runs libFuzzer on a cumulative corpus once, to gather function counts (like other profiling). It doesn't do any actual fuzzing.
We do the same (we even fuzz with Clang coverage enabled) and having ASan enabled does not stop you from doing that at all. We can look into this but it will be low priority. Especially if you just want to generate coverage for your corpus once, it shouldn't make a big difference because your corpus isn't supposed to crash.

For fuzzing, we limit libFuzzer to a certain amount of runs and restart it very often, so the coverage is dumped regularly (because on a crash, it would be lost).
You're right the corpus is not supposed to crash (at least on ASAN reports), but I'm not sure oss-fuzz follows that, as it expects libFuzzer to be run ASAN-less here. I guess this is more a potential bug. I filed it because I thought in principle this should work.

Sth. else: I tried limiting the profiling to just the instrumented code. For that I added the profiling flags only to libfuzzer-flags.mozbuild, not C(XX)FLAGS. Didn't work as no profiling data was produced. I assume this requires dumping manually in code on every iteration or so. Not worth pursuing, but would've been nice had it worked.
Attached file asan-less (obsolete) —
I have this working, but still need to distill it.

A main problem was me setting LDFLAGS incorrectly.

> mk_add_options LDFLAGS=-flags

When it should be.

> export LDFLAGS=-flags

With the following mozconfig, and the attached patch, it works.

ac_add_options --disable-debug
ac_add_options --disable-elf-hack
ac_add_options --disable-jemalloc
ac_add_options --disable-crashreporter
ac_add_options --enable-fuzzing
export LDFLAGS=-fsanitize=fuzzer-no-link
export LIBFUZZER=1

There may be some redundancy here. I don't understand some of the finer details of the build system.
Please correct if this is not the right component. Thanks.
Component: Untriaged → Build Config
Product: Firefox → Toolkit
Attached patch sanitizer-less (obsolete) — Splinter Review
Coincidentally, some unrelated recent changes made this a bit simpler. Works with the following mozconfig and the attached patch.

ac_add_options --disable-debug
ac_add_options --disable-elf-hack
ac_add_options --disable-jemalloc
ac_add_options --disable-crashreporter
ac_add_options --enable-fuzzing
ac_add_options --enable-optimize=-O1
ac_add_options --enable-debug-symbols=-gline-tables-only

Note that the patch doesn't change ASAN or other fuzzing builds.
Attachment #9002602 - Attachment is obsolete: true
Attachment #9004494 - Flags: review?(choller)
Comment on attachment 9004494 [details] [diff] [review]

Is there a reason why we couldn't just check if it is a fuzzing build (FUZZING_INTERFACES afaik) but we are building without ASan (MOZ_ASAN) instead of adding yet another variable?
Attachment #9004494 - Flags: review?(choller)
Yep, that works too. I just wanted to keep with the current MOZ naming of the other flags.
Attached patch sanitizer-less-2 (obsolete) — Splinter Review
Modified patch with FUZZING_INTERFACES for MOZ_FUZZ.
Assignee: nobody → pdknsk
Attachment #9004494 - Attachment is obsolete: true
Ever confirmed: true
Attachment #9005861 - Flags: review?(choller)
Attached patch sanitizer-less-3Splinter Review
Rebased against pending UBSAN changes.
By the way, there are also use cases for this other than oss-fuzz. Since sanitizer instrumentation causes a significant slowdown, someone may want to run sanitizer-less just to advance coverage more quickly, and then later run with sanitizers from there. Not a common but viable approach. Another is to make just a correctness fuzzer, such as to confirm that a round-trip (decoder, encoder, ...) is valid.
Comment on attachment 9007962 [details] [diff] [review]

Review of attachment 9007962 [details] [diff] [review]:

Sorry for the delay here. The patch looks good to me and the use case also makes sense (and is also backed up by what the Google Fuzzing Team mentioned about how they generate code coverage for their fuzzing targets). Forwarding the actual review to Nathan because I am not a build peer.
Attachment #9007962 - Flags: superreview+
Attachment #9007962 - Flags: review?(nfroyd)
Attachment #9005861 - Attachment is obsolete: true
Attachment #9005861 - Flags: review?(choller)
Comment on attachment 9007962 [details] [diff] [review]

Review of attachment 9007962 [details] [diff] [review]:

r=me with the below changes.

::: build/moz.configure/toolchain.configure
@@ +1808,4 @@
>  set_config('HAVE_LIBFUZZER_FLAG_FUZZER_NO_LINK', have_libfuzzer_flag_fuzzer_no_link)
> +add_old_configure_assignment('HAVE_LIBFUZZER_FLAG_FUZZER_NO_LINK', have_libfuzzer_flag_fuzzer_no_link)

Let's not do this, but instead use have_libfuzzer_flag_fuzzer_no_link to compute the flags we would add to LDFLAGS, then add an old configure assignment for that, and change sanitize.m4 accordingly.  Less logic in sanitize.m4 that way.
Attachment #9007962 - Flags: review?(nfroyd) → review+
Attached patch sanitizer-less-4Splinter Review
Done, with an extra change, to not have to keep the flags in-sync in two places. I'm not sure if you're conditional review covers that, so please take another look. Note that HAVE_LIBFUZZER_FLAG_FUZZER_NO_LINK can't be removed because it's still used elsewhere.
Attachment #9010782 - Flags: review?(nfroyd)
Comment on attachment 9010782 [details] [diff] [review]

Review of attachment 9010782 [details] [diff] [review]:

Excellent, thank you!
Attachment #9010782 - Flags: review?(nfroyd) → review+
Keywords: checkin-needed
Pushed by
enable sanitizer-less libfuzzer builds r=froydnj superreview=decoder
enable sanitizer-less libfuzzer builds r=froydnj
Keywords: checkin-needed
Backed out for build bustages

Push with failures:,busted,exception&classifiedState=unclassified&group_state=expanded&revision=2b0a42c589c5213e928285bea9cdf9a48bedfe21

Failure log:


[task 2018-09-21T02:41:42.338Z] 02:41:42     INFO -    File "/builds/worker/workspace/build/src/python/mozbuild/mozbuild/", line 59, in exec_
[task 2018-09-21T02:41:42.338Z] 02:41:42     INFO -      exec(object, globals, locals)
[task 2018-09-21T02:41:42.339Z] 02:41:42     INFO -    File "/builds/worker/workspace/build/src/build/moz.configure/toolchain.configure", line 1875, in <module>
[task 2018-09-21T02:41:42.339Z] 02:41:42     INFO -      add_old_configure_assignment('HAVE_LIBFUZZER_FLAG_FUZZER_NO_LINK', have_libfuzzer_flag_fuzzer_no_link)
[task 2018-09-21T02:41:42.339Z] 02:41:42     INFO -  NameError: name 'have_libfuzzer_flag_fuzzer_no_link' is not defined
[task 2018-09-21T02:41:42.346Z] 02:41:42     INFO -  *** Fix above errors and then restart with\
[task 2018-09-21T02:41:42.346Z] 02:41:42     INFO -                 "/usr/bin/make -f build"
[task 2018-09-21T02:41:42.346Z] 02:41:42     INFO - recipe for target 'configure' failed
[task 2018-09-21T02:41:42.346Z] 02:41:42     INFO -  make: *** [configure] Error 1
[task 2018-09-21T02:41:42.418Z] 02:41:42    ERROR - Return code: 2
[task 2018-09-21T02:41:42.418Z] 02:41:42  WARNING - setting return code to 2
[task 2018-09-21T02:41:42.418Z] 02:41:42    FATAL - 'mach build -v' did not run successfully. Please check log for errors.
[task 2018-09-21T02:41:42.418Z] 02:41:42    FATAL - Running post_fatal callback...
[task 2018-09-21T02:41:42.418Z] 02:41:42    FATAL - Exiting -1
[task 2018-09-21T02:41:42.418Z] 02:41:42     INFO - [mozharness: 2018-09-21 02:41:42.418815Z] Finished build step (failed)
[task 2018-09-21T02:41:42.418Z] 02:41:42     INFO - Running post-run listener: _summarize
[task 2018-09-21T02:41:42.419Z] 02:41:42    ERROR - # TBPL FAILURE #
[task 2018-09-21T02:41:42.419Z] 02:41:42     INFO - [mozharness: 2018-09-21 02:41:42.419051Z] FxDesktopBuild summary:
[task 2018-09-21T02:41:42.419Z] 02:41:42    ERROR - # TBPL FAILURE #
[task 2018-09-21T02:41:42.419Z] 02:41:42     INFO - Running post-run listener: copy_logs_to_upload_dir
Flags: needinfo?(pdknsk)
Only sanitizer-less-4 is to be commited. It inherits sr+ from sanitizer-less-3. I'm not sure what to do in that situation, so I don't obsolete the previous patch. Sorry for the confusion.
Flags: needinfo?(pdknsk)
Pushed by
enable sanitizer-less libfuzzer builds r=froydnj
I notice one build failed, so this will be backed out again.

[task 2018-09-21T02:54:12.775Z] 02:54:12     INFO -  ==============================
[task 2018-09-21T02:54:12.776Z] 02:54:12     INFO -  FATAL ERROR PROCESSING MOZBUILD FILE
[task 2018-09-21T02:54:12.776Z] 02:54:12     INFO -  ==============================
[task 2018-09-21T02:54:12.776Z] 02:54:12     INFO -  The error occurred while processing the following file:
[task 2018-09-21T02:54:12.776Z] 02:54:12     INFO -      /builds/worker/workspace/build/src/tools/fuzzing/libfuzzer-flags.mozbuild
[task 2018-09-21T02:54:12.776Z] 02:54:12     INFO -  This file was included as part of processing:
[task 2018-09-21T02:54:12.776Z] 02:54:12     INFO -      /builds/worker/workspace/build/src/security/sandbox/linux/broker/
[task 2018-09-21T02:54:12.776Z] 02:54:12     INFO -  The error was triggered on line 8 of this file:
[task 2018-09-21T02:54:12.776Z] 02:54:12     INFO -      libfuzzer_flags += CONFIG['LIBFUZZER_FLAGS']
[task 2018-09-21T02:54:12.776Z] 02:54:12     INFO -  An error was encountered as part of executing the file itself. The error appears to be the fault of the script.
[task 2018-09-21T02:54:12.777Z] 02:54:12     INFO -  The error as reported by Python is:
[task 2018-09-21T02:54:12.777Z] 02:54:12     INFO -      ["TypeError: 'NoneType' object is not iterable\n"]

Apparently this is caused by that build setting --disable-compile-environment, which makes it not pull in toolchain.configure. So libfuzzer-flags.mozbuild probably needs an additional check before it adds CONFIG['LIBFUZZER_FLAGS'].
Can you push a fix or should i backout?
Flags: needinfo?(pdknsk)
Should I make an updated complete patch or just the delta patch?
Flags: needinfo?(pdknsk)
Whichever fixes the issue and i don't have to backout again, i guess it's fine. 
I'm not the person you should be asking that, maybe try the reviewer ?
Flags: needinfo?(pdknsk)
Attached patch sanitizer-less-5 (obsolete) — Splinter Review
Updated complete patch. Only adds a single line, so doesn't need new review. I verified it locally with the fail
ing build's config.
Flags: needinfo?(pdknsk)
pdknsk: i tried to land this but couldn't
Flags: needinfo?(pdknsk)
I assume you need to back out the other first. It's a replacement patch. Or if you want a patch on top of sanitizer-less-4, this will do it.

diff --git a/tools/fuzzing/libfuzzer-flags.mozbuild b/tools/fuzzing/libfuzzer-flags.mozbuild
--- a/tools/fuzzing/libfuzzer-flags.mozbuild
+++ b/tools/fuzzing/libfuzzer-flags.mozbuild
@@ -5,4 +5,7 @@
 # file, You can obtain one at
 libfuzzer_flags = []
-libfuzzer_flags += CONFIG['LIBFUZZER_FLAGS']
+    libfuzzer_flags += CONFIG['LIBFUZZER_FLAGS']
Flags: needinfo?(pdknsk)
Pushed by
additional patch to sanitizer-less-4 r=test-fix
Comment on attachment 9010838 [details] [diff] [review]

Attachment #9010838 - Attachment is obsolete: true
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.