New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 643249 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Blocking:
issue 539572



Sign in to add a comment

Switch to -O1 release for libfuzzer release builds

Project Member Reported by infe...@chromium.org, Sep 1 2016

Issue description

See title and discussion in email thread with Kostya.
 
Cc: thakis@chromium.org dpranke@chromium.org
+thakis, dpranke 

Is it possible to do this with GN?
Cc: aizatsky@chromium.org
Can you paste a summary of the email discussion here?

I think this should be possible, yes (see build/config/compiler/BUILD.gn); not sure if it's a good idea though.
Thanks, so I'm guessing we'll need to add a new GN arg that makes the "default_optimization" config use an -O1 config instead. 

The summary as I understand it is that less optimization -> more branches in the generated code which is better for coverage guided fuzzing.


Comment 5 by kcc@chromium.org, Sep 1 2016

>> less optimization -> more branches 
Correct. 
> add a new GN arg

Couldn't this just be keyed off the fuzzing gn arg? use_libfuzzer or how it's called?

If this is about getting more branches, should the compiler driver handle this instead?
Hm. I can see how creating more branches leads to more fuzzing testcases, but it also seems like we'd be testing branches that (probably?) shouldn't matter at all (otherwise it'd be wrong for the compiler to optimize them away) and we'd be fuzzing a configuration that isn't what we ship. Which might turn up stuff, but wouldn't necessarily turn up things that were interesting.

In other words, I'd want to see some evidence that this was producing real value and not just creating more work before I really wanted to do this. 

Of course, we can certainly add a flag to run the experiment. However, if -O1 was better than -O2, wouldn't -O0 (i.e., no optimizations) be even better? We at least have that config today, though there isn't a way to configure that without DCHECK and the rest of the debug flags.

Comment 8 by kcc@chromium.org, Sep 1 2016

Compilers often optimize branches into branch-free code, i.e. the logic is preserved, but it's harder for the fuzzer to differentiate test inputs. 

Do you need scientific evidence of anecdotal?  
anecdotal: I've seen bugs found in O1 fuzzing builds and missed in O2 builds.

Same is true for O1 vs O0, and indeed we already have O0 build.
But O0 is slow and does not work as the only build and also it's a debug build
and we need an opt build as well. 


Components: Build
Labels: OS-All
> Compilers often optimize branches into branch-free code, i.e. the 
> logic is preserved, but it's harder for the fuzzer to differentiate 
> test inputs. 

Ah, I see. That makes sense, thanks.

(i.e., anecdotal + explanation is good enough, since it was the explanation I care about).

Still, I'd be a bit reluctant to do this unless we thought we were reliably fixing all of the existing fuzzing-derived bugs fast enough, really needed a source of new bugs, and had nothing better to do with our time :).
> Still, I'd be a bit reluctant to do this unless we thought we were reliably fixing all of the existing fuzzing-derived bugs fast enough, really needed a source of new bugs, and had nothing better to do with our time :).

From a security point of view at least, bugs exist whether or not we find them, and attackers with malicious intentions aren't going to wait.

Comment 11 by aarya@google.com, Sep 1 2016

"Still, I'd be a bit reluctant to do this unless we thought we were reliably fixing all of the existing fuzzing-derived bugs fast enough, really needed a source of new bugs, and had nothing better to do with our time :)."

Yes, we will continue to overload ourselves with finding more security and stability bugs and keeping our users happy and secure. Since this finds new coverage guided testcases, i am in extreme favor of this.
> Couldn't this just be keyed off the fuzzing gn arg? use_libfuzzer or how it's called?

Yes, something like:

if ((use_libfuzzer || use_afl) && !is_debug) {
    cflags = "-O1"
}


seems good to me.


Though an additional flag like "fuzzing_optimization = true" might be even better, because we can easily add / remove it into different configurations and for other purposes (e.g. tests, benchmarks, etc).
Cc: och...@chromium.org
 Issue 639364  has been merged into this issue.
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aaa26c162de624ebcf7a120e85fef3e19ec02151

commit aaa26c162de624ebcf7a120e85fef3e19ec02151
Author: ochang <ochang@chromium.org>
Date: Wed Oct 05 17:59:04 2016

Add an optimize_for_fuzzing GN flag to build most things with -O1

This is better for coverage guided fuzzing builds as less optimization
means more branches in the generated code.

BUG= 643249 

Review-Url: https://codereview.chromium.org/2389923003
Cr-Commit-Position: refs/heads/master@{#423217}

[modify] https://crrev.com/aaa26c162de624ebcf7a120e85fef3e19ec02151/build/config/compiler/BUILD.gn

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/edfefd55622340e0e3d73ca3b899647c820555af

commit edfefd55622340e0e3d73ca3b899647c820555af
Author: ochang <ochang@chromium.org>
Date: Thu Oct 06 01:34:58 2016

Use -O1 in more optimize configs when optimize_for_fuzzing=true.

BUG= 643249 

Review-Url: https://codereview.chromium.org/2397873003
Cr-Commit-Position: refs/heads/master@{#423383}

[modify] https://crrev.com/edfefd55622340e0e3d73ca3b899647c820555af/build/config/compiler/BUILD.gn

Project Member

Comment 16 by bugdroid1@chromium.org, Oct 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca6387dfb156c9e30002c734e0c6007781659ee5

commit ca6387dfb156c9e30002c734e0c6007781659ee5
Author: ochang <ochang@chromium.org>
Date: Thu Oct 06 01:38:27 2016

[mb] Use optimize_for_fuzzing for libfuzzer bots.

R=dpranke@chromium.org
BUG= 643249 

Review-Url: https://codereview.chromium.org/2393993002
Cr-Commit-Position: refs/heads/master@{#423385}

[modify] https://crrev.com/ca6387dfb156c9e30002c734e0c6007781659ee5/tools/mb/mb_config.pyl

Comment 17 by aarya@google.com, Oct 6 2016

This is so weird, with -O1 optimization, archives became smaller.

[DIR]	libfuzzer-linux-release-423381.zip	2016-10-06 03:34:30	6604.02MB	423381	0bf35d3beec3d662572b4795d3312cb4ffb3494b
[DIR]	libfuzzer-linux-release-423433.zip	2016-10-06 07:17:17	4741.48MB	423433	824ec594c7c9e988dc2361ee0fa64246956ec549

Comment 18 by aarya@google.com, Oct 6 2016

Ignore c#17.
Status: Fixed (was: Assigned)
This is done.

A quick test with renderer_tree_fuzzer:

with -O2: 1834210 coverage points
with -O1: 2171248 coverage points
Project Member

Comment 20 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aaa26c162de624ebcf7a120e85fef3e19ec02151

commit aaa26c162de624ebcf7a120e85fef3e19ec02151
Author: ochang <ochang@chromium.org>
Date: Wed Oct 05 17:59:04 2016

Add an optimize_for_fuzzing GN flag to build most things with -O1

This is better for coverage guided fuzzing builds as less optimization
means more branches in the generated code.

BUG= 643249 

Review-Url: https://codereview.chromium.org/2389923003
Cr-Commit-Position: refs/heads/master@{#423217}

[modify] https://crrev.com/aaa26c162de624ebcf7a120e85fef3e19ec02151/build/config/compiler/BUILD.gn

Project Member

Comment 21 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/edfefd55622340e0e3d73ca3b899647c820555af

commit edfefd55622340e0e3d73ca3b899647c820555af
Author: ochang <ochang@chromium.org>
Date: Thu Oct 06 01:34:58 2016

Use -O1 in more optimize configs when optimize_for_fuzzing=true.

BUG= 643249 

Review-Url: https://codereview.chromium.org/2397873003
Cr-Commit-Position: refs/heads/master@{#423383}

[modify] https://crrev.com/edfefd55622340e0e3d73ca3b899647c820555af/build/config/compiler/BUILD.gn

Project Member

Comment 22 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca6387dfb156c9e30002c734e0c6007781659ee5

commit ca6387dfb156c9e30002c734e0c6007781659ee5
Author: ochang <ochang@chromium.org>
Date: Thu Oct 06 01:38:27 2016

[mb] Use optimize_for_fuzzing for libfuzzer bots.

R=dpranke@chromium.org
BUG= 643249 

Review-Url: https://codereview.chromium.org/2393993002
Cr-Commit-Position: refs/heads/master@{#423385}

[modify] https://crrev.com/ca6387dfb156c9e30002c734e0c6007781659ee5/tools/mb/mb_config.pyl

Comment 23 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment