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

[Justin Bogner via Phabricator via llvm-commits]

bogner accepted this revision.
bogner added a comment.
This revision is now accepted and ready to land.

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



================
Comment at: lib/FuzzMutate/IRMutator.cpp:150-151
       RS.sample(&Inst, /*Weight=*/1);
-  assert(!RS.isEmpty() && "No instructions to delete");
+  if (RS.isEmpty())
+    return;
+
----------------
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.


================
Comment at: unittests/FuzzMutate/StrategiesTest.cpp:99-101
+  // We need to choose 'func1' in order for the crash to appear.
+  // Loop 10 times and assume we are lucky.
+  for (int i = 0; i < 10; ++i) {
----------------
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.


https://reviews.llvm.org/D40393