This is an archive of the discontinued LLVM Phabricator instance.

[FuzzMutate] Don't crash when we can't remove instruction from empty function

Authored by igor-laevsky on Nov 23 2017, 7:19 AM.



We don't need to crash when we decide to delete something from the empty function. It is possible that module has other non empty functions which we are going to choose on the next fuzzer iteration.

Diff Detail


Event Timeline

igor-laevsky created this revision.Nov 23 2017, 7:19 AM
bogner accepted this revision.Nov 27 2017, 4:39 PM

Looks good. I have a couple of comments about things we should think about, but I don't think they need to block us.

150–151 ↗(On Diff #124081)

This is reasonable and should fix the problem, but it would be kind of nice if we could also make it so the deleter strategy was never chosen in this case. That's pretty tricky with the current getWeight API (it doesn't really have enough information), but we should think about it for the future.

99–101 ↗(On Diff #124081)

Thanks for adding the test! I'm not a huge fan of the probabilistic nature of it, but I can't think of a better way to do that without making either the sampler or the mutators quite a bit more complicated.

This revision is now accepted and ready to land.Nov 27 2017, 4:39 PM
This revision was automatically updated to reflect the committed changes.
igor-laevsky added inline comments.Nov 30 2017, 7:13 AM
150–151 ↗(On Diff #124081)

I guess it boils down to the question of wether or not we want to guarantee "forward progress" of the mutator (i,e guarantee that at least some transformation is always performed). Initially I implemented more involved version of this change - if randomly chosen function appeared to be empty we would have picked the first non empty one and removed from it. Then if there would be no such function we would assert. However in the end I realised that forward progress guarantee is not that important considering the amount of tests OSSFuzz runs.

99–101 ↗(On Diff #124081)

If it makes things slightly better - since seed is fixed this is deterministic with the given libc implementation. Hopefully there shouldn't be too much different implementations.