1464202 - Improve and centralize libFuzzer flag management

Status: RESOLVED FIXED

(Core :: Fuzzing, enhancement)

Description

[Christian Holler (:decoder)]

The last libFuzzer upgrade to our code base has caused some problems about PC table generation (libFuzzer does no longer emit NEW_PC messages for example).

After some digging and talking to :kcc, it turned out that with the current version of libFuzzer, we are supposed to use a different set of build flags. In general, he recommended using the combination flag -fsanitize=fuzzer(-no-link) which adds all the flags we need. However, this flag isn't supported in Clang <= 5.


So this bug is about two things:

1) Centralize our libFuzzer flag management. Right now, we spread flags to every moz.build file that has libFuzzer target code. It would be much more desirable to have configuration files to include instead and have the flags in one place.

2) Improve the flags. Use -fsanitize=fuzzer-no-link if available. If it is not available, fall back to the old combination we used before. I was also told that we are supposed to always use the trace-cmp flag as well, in that case.


The upcoming patch does all this by

1) Adding detection code for -fsanitize=fuzzer-no-link
2) Adding files in tools/fuzzer/ to include (one with the flag definitions and one having the typical CFLAGS/CXXFLAGS += directives that are almost always used).
3) Replacing all occurrences of the flags with the include.
4) special casing gyp. I wasn't able to change the code so we can simply pass the flag in. It works as it is right now in the patch, but duplicates the flags in the target gyp file which is exactly what we don't want in the long run. I'm open for suggestions, but I don't want this to block the overall refactoring.

Comment 1

[Christian Holler (:decoder)]

Attachment: Bug 1464202 - Improve and centralize libFuzzer flag management.

Review commit: https://reviewboard.mozilla.org/r/246580/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/246580/

Comment 2

[Code Review Bot [:reviewbot]]

Comment on attachment 8980409 [details]
Bug 1464202 - Improve and centralize libFuzzer flag management.

https://reviewboard.mozilla.org/r/246580/#review252704


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: build/moz.configure/toolchain.configure:1635
(Diff revision 1)
> +         check_msg='whether the C compiler supports -fsanitize=fuzzer-no-link'))
> +def have_libfuzzer_flag_fuzzer_no_link(value):
> +    if value:
> +        return True
> +
> +set_config('HAVE_LIBFUZZER_FLAG_FUZZER_NO_LINK', have_libfuzzer_flag_fuzzer_no_link)

Error: Expected 2 blank lines after class or function definition, found 1 [flake8: E305]

Comment 3

[Alex Gaynor [:Alex_Gaynor]]

Comment on attachment 8980409 [details]
Bug 1464202 - Improve and centralize libFuzzer flag management.

https://reviewboard.mozilla.org/r/246580/#review252964

::: tools/fuzzing/libfuzzer-config.mozbuild:11
(Diff revision 1)
> +
> +include('libfuzzer-flags.mozbuild')
> +
> +if CONFIG['FUZZING']:
> +    if CONFIG['LIBFUZZER']:
> +       # Add trace-pc coverage for libfuzzer

three space indent here

::: tools/fuzzing/libfuzzer-flags.mozbuild:10
(Diff revision 1)
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +libfuzzer_flags = []
> +
> +if CONFIG['HAVE_LIBFUZZER_FLAG_FUZZER_NO_LINK']:
> +  libfuzzer_flags += ['-fsanitize=fuzzer-no-link']

two space indents here, mozbuild files generally use 4 spaces

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8980409 [details]
Bug 1464202 - Improve and centralize libFuzzer flag management.

https://reviewboard.mozilla.org/r/246580/#review254016

Please do fix the reviewbot comments before landing, or the Python lint job will burn.

Comment 5

[Christian Holler (:decoder)]

Comment on attachment 8980409 [details]
Bug 1464202 - Improve and centralize libFuzzer flag management.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246580/diff/1-2/

Comment 6

[Christian Holler (:decoder)]

Comment on attachment 8980409 [details]
Bug 1464202 - Improve and centralize libFuzzer flag management.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246580/diff/2-3/

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e8eb0005b50c
Improve and centralize libFuzzer flag management. r=froydnj

Comment 8

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/e8eb0005b50c