[libFuzzer] Disable LoopVectorize pass while fuzzing

[george.karpenkov]

Summary

Loop vectorization pass can also mess with fuzzing,
from what I have seen, interleaving stops counters.test from passing on ARM64.
Disabling that pass would not entail a large performance penalty,
but would allow more tests to pass on different architectures.

[kcc]

don't we need a test for this?

[george.karpenkov]

@kcc counters.test on ARM64?

[george.karpenkov]

oh, you mean specifically no optimizations here. you are right.

[fhahn]

If interleaving in LoopVectorize is the problem, maybe setting ForceTargetMaxInterleaveFactor to 1 would be worth a try to just disable interleaving in LV.

[george.karpenkov]

@fhahn Setting force-target-max-scalar-interleave=1 seems to work on this example.
Would it make more sense to read the attribute in LoopVectorize, or to try to set this option in this option in the driver?

[george.karpenkov]

I did some more digging: it seems the source of the problem is the same as for the loop unroller.
Before the transformation, on the example of interest (counters.test) there exists a single sanitizer counter responsible for the "interesting" branch,
and the test is doing the search for the input which would cause the counter to be incremented 4 times.

Vectorization and unrolling potentially greatly increase the number of counters: and that makes the search space much larger, and the likelihood of hitting the "right" counter smaller.
With that picture in mind, I would think it makes more sense to disable vectorization for fuzzing entirely, rather than just interleaving.

[kcc]

I don't agree that we should disable transformations that expand the code (unrolling, vectorizing, inlinening, etc) in the fuzzing mode.
The typically improve the fuzzing results since they give more signal.
If you want to fight the problem in the test, maybe just fight it in the test (e.g. add/remove some flags for the particular test)

[george.karpenkov]

@kcc My experience with this is limited, but so far I have seen at least two examples where it makes things worse.
Extra signal is not necessarily better, especially if it is noise, and hides the good signal.

1. Fixing the problem in the test does not make that much sense, if the test is meant to be representative of the input program.

We can certainly mark it as UNSUPPORTED though.

2. What about a less intrusive change which just disables interleaving?

[kcc]

What are the other examples?
counters.test tests specifically the counters, and is indeed expected to behave poorly if the counters are "diluted".

I don't really understand how interleaving changes the results of fuzzing.
Might be some effect specific to the test, not worth fixing outside the test.

Feel free to mark the test UNSUPPORTED on Mac, if it bothers you. We haven't seen it being flaky on Linux though.

[george.karpenkov]

@kcc the other one was shrink.test failing on x86_64h, because it was more eager to unroll.

I can also just disable counters.test on ARM/ARCH64, that also works for me.
I'm curious though: do you have data showing that unrolling/vectorization/etc are beneficial while fuzzing?
I could see it going either way.

I don't really understand how interleaving changes the results of fuzzing.

The interleaving pass unrolls the loop a few times. That doubles the amount of counters, and in turn, in my understanding, lowers the probability of finding the input which would hit the counter we need 4 times.

[kcc]

I don't have solid data on this. anecdotal evidence suggest that code expansion *usually* helps, but not always.
I don't want to blindly disable optimizations for all libFuzzer users just because those optimizations make a synthetic test unhappy

[george.karpenkov]

@kcc OK makes sense.
Intuitively it still seems to make sense to me that vectorization and unrolling would make fuzzing worse, but you are right that a lot more data would be needed to make that decision.

lib/Transforms/Vectorize/LoopVectorize.cpp

   BFI = &BFI_;
   TLI = TLI_;
   AA = &AA_;
   AC = &AC_;
   GetLAA = &GetLAA_;
   DB = &DB_;
   ORE = &ORE_;
 
+  // Do not vectorize functions when the target should be fuzzed.
+  if (F.hasFnAttribute(Attribute::OptForFuzzing))
+    return false;
+
   // Don't attempt if
   // 1. the target claims to have no vector registers, and
   // 2. interleaving won't help ILP.
   //
   // The second condition is necessary because, even if the target has no
   // vector registers, loop vectorization may still enable scalar
   // interleaving.
   if (!TTI->getNumberOfRegisters(true) && TTI->getMaxInterleaveFactor(1) < 2)

test/Transforms/LoopVectorize/opt-for-fuzzing.ll

+; RUN: opt < %s -loop-vectorize -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+; CHECK-NOT: vector.body
+
+define i8 @foo(i1 %c, i32 %x, i32 %n) optforfuzzing {
+entry:
+  br label %for.body
+
+for.body:
+  %i = phi i32 [ 0, %entry ], [ %i.next, %if.end ]
+  %r = phi i32 [ 0, %entry ], [ %r.next, %if.end ]
+  br i1 %c, label %if.then, label %if.end
+
+if.then:
+  %tmp0 = sdiv i32 undef, undef
+  br label %if.end
+
+if.end:
+  %tmp1 = and i32 %r, 255
+  %i.next = add nsw i32 %i, 1
+  %r.next = add nuw nsw i32 %tmp1, %x
+  %cond = icmp eq i32 %i.next, %n
+  br i1 %cond, label %for.end, label %for.body
+
+for.end:
+  %tmp2 = phi i32 [ %r.next, %if.end ]
+  %tmp3 = trunc i32 %tmp2 to i8
+  ret i8 %tmp3
+}