[PATCH] D36274: Introduce FuzzMutate library

Justin Bogner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 11 18:15:46 PDT 2017

bogner added a comment.

Adding some docs in docs/Fuzzer sounds like a good idea. I'll see about doing that soon but I think it makes sense for that to be a separate patch.

Comment at: include/llvm/FuzzMutate/IRMutator.h:78
+  using IRMutationStrategy::mutate;
+  void mutate(Function &F, RandomIRBuilder &IB) override;
vsk wrote:
> Out of curiosity, why is this using decl needed? Is 'mutate' somehow ambiguous here?
This pulls in the full overload set, so that the base class's mutate(Module) will choose a function and call mutate(Function) here.

Comment at: include/llvm/FuzzMutate/OpDescriptor.h:48
+  SourcePred(PredT Pred, MakeT Make) : Pred(Pred), Make(Make) {}
+  SourcePred(PredT Pred, NoneType) : Pred(Pred) {
+    Make = [Pred](ArrayRef<Value *> Cur, ArrayRef<Type *> BaseTypes) {
vsk wrote:
> Why have an unused NoneType parameter?
I found it clearer to force the user to explicitly opt in to using the default generator - SourcePred(P, None) rather than SourcePred(P). If you think that's overkill I could drop this.

Comment at: include/llvm/FuzzMutate/OpDescriptor.h:54
+        Constant *V = UndefValue::get(T);
+        if (Pred(Cur, V))
+          makeConstantsWithType(T, Result);
vsk wrote:
> Nit: It may enhance readability to capture this and use matches() directly. Feel free to ignore this if you don't feel that way.
It's kind of sketchy to use member functions in a constructor. I'd rather not.

Comment at: lib/FuzzMutate/Operations.cpp:110
+  case Instruction::Or:
+  case Instruction::Xor:
+    return {Weight, {anyIntType(), matchFirstType()}, buildOp};
vsk wrote:
> It looks like this list is duplicated in describeFuzzerIntOpts. Would it make sense to share it with the HANDLE_X pattern?
These aren't in their own list anywhere today (they're mixed with the float ops in Instructions.def) so I think that would entail creating a new .def file and including that in a couple of places here. I'm not convinced the decreased readability of having to go look up what's in that list is worth the couple of lines of savings here.


More information about the llvm-commits mailing list