Page MenuHomePhabricator

[SimplifyCFG] Create flag to disable simplifyCFG.
AbandonedPublic

Authored by morehouse on Feb 16 2018, 4:45 PM.

Details

Reviewers
kcc
vitalybuka
Summary

When building with libFuzzer, simplifyCFG reduces the coverage signal
available to libFuzzer when trying to find new inputs. This patch
provides a way to disable simplifyCFG when building with libFuzzer.

Addresses https://llvm.org/pr36414.

Event Timeline

morehouse created this revision.Feb 16 2018, 4:45 PM
morehouse edited the summary of this revision. (Show Details)Feb 16 2018, 4:46 PM
vitalybuka added inline comments.Feb 16 2018, 5:17 PM
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
109

why metadata instead of cl:opt like these?

Some high level comments:

  1. This is something that GCC does relatively frequently (adding frontend options to control optimization passes), but LLVM tends to not expose these details.

FWIW, I'd very much prefer the details of the optimizer wouldn't be exposed as frontend flags.

  1. I really don't think metadata is the correct way of conveying this information to the optimizer. Among all the other problems, metadata can be stripped willy-nilly. Maybe a function attribute will work better?

Some alternatives which I don't like, but I can't currently propose anything better are:

  1. Having a separate optimization pipeline that you use in these cases
  2. Having an instrumentation pass that annotates your CFG to prevent passes from destroying coverage signal. That said, I'm not really familiar to comment on whether this is feasible or not.

Please note that completely disabling SimplifyCFG will result in non-trivial performance hits (at least, in my experience), so you might want to evaluate this carefully.
@chandlerc might have thoughts on how to address this.

kcc added a comment.Feb 20 2018, 11:38 AM

We use function attributes in similar situations for asan/tsan/msan.
Similar, but not equivalent, so I don't know if we must follow the same pattern here.
The difference here is that we should keep the ability to turn optimization on and off from command line, regardless of what are the coverage instrumentation flags.

IIRC, the *function* metadata is not that easily stripped (as opposed to metadata attached to instructions).

Some high level comments:

  1. This is something that GCC does relatively frequently (adding frontend options to control optimization passes), but LLVM tends to not expose these details. FWIW, I'd very much prefer the details of the optimizer wouldn't be exposed as frontend flags.

We might not need the flag if we decide to always set no_simplify_cfg during codegen with coverage instrumentation. But we're not sure if we want to do that yet.

  1. I really don't think metadata is the correct way of conveying this information to the optimizer. Among all the other problems, metadata can be stripped willy-nilly. Maybe a function attribute will work better?

Should be simple enough to change this to a function attribute if you think that is more appropriate. Wasn't sure whether metadata or an attribute made more sense.

Some alternatives which I don't like, but I can't currently propose anything better are:

  1. Having a separate optimization pipeline that you use in these cases

This seems like a lot more work.

  1. Having an instrumentation pass that annotates your CFG to prevent passes from destroying coverage signal. That said, I'm not really familiar to comment on whether this is feasible or not.

This patch allows us to annotate our functions with no_simplify_cfg to do what you're suggesting. Writing another instrumentation pass seems like overkill.

Please note that completely disabling SimplifyCFG will result in non-trivial performance hits (at least, in my experience), so you might want to evaluate this carefully.
@chandlerc might have thoughts on how to address this.

We're still evaluating this with mixed results. On one hand, disabling simplifyCFG makes most of our libFuzzer tests pass with -O2, and libFuzzer seems to perform at the same executions/sec rate. On the other hand, executions/sec doesn't necessarily translate to more coverage/sec or bugs found.

llvm/lib/Transforms/Utils/SimplifyCFG.cpp
109

We want to be able to avoid simplifyCFG when we pass -fsanitize=fuzzer to the Clang driver. AFAICT, the frontend does not explicitly invoke simplifyCFG, so it can't control the options here.

FWIW, I'd very much prefer the details of the optimizer wouldn't be exposed as frontend flags.

Yes, definitely this. Frontend flags to control the optimizer should state an expected result, not just turn off some completely arbitrary set of transforms. Otherwise, we're likely to end up in a situation in the future where we add a new optimization, or split an existing pass, and it isn't clear which transforms the frontend flag should apply to.

vitalybuka added inline comments.Feb 21 2018, 3:22 PM
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
6056

So maybe Attribute::SanitizeFuzzer for each function with --fsanitizer=fuzzer and then

if (... getFunction()->hasFnAttribute(Attribute::SanitizeFuzzer))
    return;
vitalybuka requested changes to this revision.Mar 2 2018, 5:03 PM

I assume this is going to be abandoned in favor of D44057

This revision now requires changes to proceed.Mar 2 2018, 5:03 PM
morehouse abandoned this revision.Mar 7 2018, 2:45 PM