fuzzilli
fuzzilli copied to clipboard
Minimisation Improvements
There are areas where the minimiser could use improvement:
Example 1:
for (const v375 in v374) {
}
function v376(v377) {
}
function v378(v379,v380) {
for (let v384 = 0; v384 < 100; v384 = v384 || -1024) {
}
const v385 = (v386,v387,v388,v389,v390) => {
};
}
The block reducer is supposed to delete empty functions, loops etc. This doesn't seem to happen, perhaps because they are nested functions. Should we make BlockReducer recursive to tackle this?
Example 2:
There are many areas where the JS code doesn't contribute to the coverage, or the crash, or in somecases doesn't even get executed. In the example below from the corpus, only one of these returns are going to get executed. This could be tackled by looping through every instruction and removing it if redundant. I suspect the reason why it was chosen to not to add instruction or line level minimisation is due to the performance overhead of looping through every line/instruction. However you could also argue that a properly minimised corpus would negate some of that performance overhead. At the very least I think there could be value in adding this option to aggressive mode. For example users might want to start off with normal minimisation, and once they get to the point where coverage gain is minimal, they may want to turn on aggressive minimisation to start cleaning and improving their corpus.
Edit: I can see we have GenericInstructionReducer that should do exactly this. Looking into why it may not be working as intended.
return v814;
return 1337;
return 3;
return 708237492;
return 6;
return BigInt64Array;
return BigInt64Array;
return 1337;
return 1337;
Nice, thanks for the report!
Overall, I think it would be nice to have some more inspectability of the minimizer (in addition to proper tests for it), to better understand why a certain construct wasn't removed. Not sure how to best do that though.
Example 1
Interesting, the minimizer should be able to remove these constructs. Maybe they are in fact interesting for the target engine? In any case, I think there are at least two things that might improve the minimization of blocks:
- Always run minimization as a fixpoint iteration, which is currently only done for crashes (
mode == .aggressive). This would essentially be the recursive reduction you mentioned - Turn this line: https://github.com/googleprojectzero/fuzzilli/blob/ac9dafbd5ab8755c7536db4e6c8f9cf2d78e29c3/Sources/Fuzzilli/Minimization/BlockReducer.swift#L18
into
for group in Blocks.findAllBlockGroups(in: code).reversed() {to process the blocks starting from the last one to clean up any data dependencies between blocks (the way it's currently done, an empty block won't be removed if some later, unnecessary block depends on its output)
I think I'd be fine with making the aggressive mode the default, since we also have the explicit --minimizationLimit to prevent "over-minimization" if desired. WDYT?
Example 2
Yes, the GenericInstructionReducer should be able to clean this up easily. Maybe it is in fact interesting for the target engine? Like, it could very well be the case that the dead code triggers some new behaviour inside the JIT compiler, responsible for detecting and deleting dead code. But again, better inspectability of the minimizer would be great...
Closing this issue since we now have testing support for the minimizers, so if these issues come up again in the future, we can write tests for them to ensure that certain code constructs can be minimized away if they are not important.