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

Status: RESOLVED FIXED

(Toolkit Graveyard :: Build Config, defect)

Description

[u473386]

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.

https://clang.llvm.org/docs/SourceBasedCodeCoverage.html

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.

Comment 1

[u473386]

This is not particularly urgent, because oss-fuzz hasn't yet switched to clang coverage from sanitizer coverage.

Comment 2

[Christian Holler (:decoder)]

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.

Comment 3

[u473386]

It can be build without ASAN, the coverage comes from -fsanitize=fuzzer-no-link (or -fsanitize-coverage flags).

https://llvm.org/docs/LibFuzzer.html#fuzzer-usage

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.

Comment 4

[Christian Holler (:decoder)]

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).

Comment 5

[u473386]

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.

https://clang.llvm.org/docs/SourceBasedCodeCoverage.html#using-the-profiling-runtime-without-static-initializers

Comment 6

[u473386]

Attachment: asan-less

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.

Comment 7

[u554753]

Please correct if this is not the right component. Thanks.

Comment 8

[u473386]

Attachment: sanitizer-less

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.

Comment 9

[Christian Holler (:decoder)]

Comment on attachment 9004494 [details] [diff] [review]
sanitizer-less

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?

Comment 10

[u473386]

Yep, that works too. I just wanted to keep with the current MOZ naming of the other flags.

Comment 11

[u473386]

Attachment: sanitizer-less-2

Modified patch with FUZZING_INTERFACES for MOZ_FUZZ.

Comment 12

[u473386]

Attachment: sanitizer-less-3

Rebased against pending UBSAN changes.

Comment 13

[u473386]

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 14

[Christian Holler (:decoder)]

Comment on attachment 9007962 [details] [diff] [review]
sanitizer-less-3

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.

Comment 15

[Nathan Froyd [:froydnj]]

Comment on attachment 9007962 [details] [diff] [review]
sanitizer-less-3

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.

Comment 16

[u473386]

Attachment: sanitizer-less-4

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.

Comment 17

[Nathan Froyd [:froydnj]]

Comment on attachment 9010782 [details] [diff] [review]
sanitizer-less-4

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

Excellent, thank you!

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/48d133cbafd3
enable sanitizer-less libfuzzer builds r=froydnj superreview=decoder
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b0a42c589c5
enable sanitizer-less libfuzzer builds r=froydnj

Comment 19

[Andreea Pavel [:apavel]]

Backed out for build bustages

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=testfailed,busted,exception&classifiedState=unclassified&group_state=expanded&revision=2b0a42c589c5213e928285bea9cdf9a48bedfe21

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=200625595&repo=mozilla-inbound&lineNumber=1039

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/cf8f821c2018f6ddc7c31814fab54fe442fd59d2



[task 2018-09-21T02:41:42.338Z] 02:41:42     INFO -    File "/builds/worker/workspace/build/src/python/mozbuild/mozbuild/util.py", 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 client.mk build"
[task 2018-09-21T02:41:42.346Z] 02:41:42     INFO -  client.mk:124: 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

Comment 20

[u473386]

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.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ab8b903147a
enable sanitizer-less libfuzzer builds r=froydnj

Comment 22

[u473386]

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/moz.build
[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'].

Comment 23

[Andreea Pavel [:apavel]]

Can you push a fix or should i backout?

Comment 24

[u473386]

Should I make an updated complete patch or just the delta patch?

Comment 25

[Andreea Pavel [:apavel]]

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 ?

Comment 26

[u473386]

Attachment: sanitizer-less-5

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.

Comment 27

[Andreea Pavel [:apavel]]

pdknsk: i tried to land this but couldn't https://irccloud.mozilla.com/file/kga9U6kT/image.png

Comment 28

[u473386]

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 http://mozilla.org/MPL/2.0/.
 
 libfuzzer_flags = []
-libfuzzer_flags += CONFIG['LIBFUZZER_FLAGS']
+
+if CONFIG['LIBFUZZER_FLAGS']:
+    libfuzzer_flags += CONFIG['LIBFUZZER_FLAGS']
+

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb6bde5f4dfd
additional patch to sanitizer-less-4 r=test-fix

Comment 30

[Andreea Pavel [:apavel]]

Fixed worked: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&resultStatus=success,testfailed,busted,exception&fromchange=2c10fe6e34c0f6c62984515fc48d81295d609f03&group_state=expanded&selectedJob=200639216&searchStr=linux,x64,opt,artifact-build-linux64-artifact%2Fopt,(ab)

Comment 31

[u473386]

Comment on attachment 9010838 [details] [diff] [review]
sanitizer-less-5

Thanks.

Comment 32

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/5ab8b903147a
https://hg.mozilla.org/mozilla-central/rev/eb6bde5f4dfd