spirv-fuzz: Avoid creating blocks without parents (#3908)

The validity check during fuzzing and in unit tests is strengthened to
require that every block has its enclosing function as its parent.
TransformationMergeFunctionReturns is fixed so that it ensures parents
are set appropriately.

Fixes #3907.

source/fuzz/force_render_red.cpp

@@ -212,6 +212,7 @@ bool ForceRenderRed(
     auto new_exit_block = MakeUnique<opt::BasicBlock>(std::move(label));
     new_exit_block->AddInstruction(MakeUnique<opt::Instruction>(
         ir_context.get(), SpvOpReturn, 0, 0, opt::Instruction::OperandList()));
+    new_exit_block->SetParent(entry_point_function);
     entry_point_function->AddBasicBlock(std::move(new_exit_block));
   }
 

source/fuzz/fuzzer.cpp

@@ -17,6 +17,7 @@
 #include <cassert>
 #include <memory>
 #include <numeric>
+#include <sstream>
 
 #include "source/fuzz/fact_manager/fact_manager.h"
 #include "source/fuzz/fuzzer_context.h"
@@ -170,6 +171,27 @@ bool Fuzzer::ApplyPassAndCheckValidity(
                 "inspect); stopping.");
       return false;
     }
+    // Check that all blocks in the module have appropriate parent functions.
+    for (auto& function : *ir_context_->module()) {
+      for (auto& block : function) {
+        if (block.GetParent() == nullptr) {
+          std::stringstream ss;
+          ss << "Block " << block.id()
+             << " has no parent; its parent should be " << function.result_id()
+             << ".";
+          consumer_(SPV_MSG_INFO, nullptr, {}, ss.str().c_str());
+          return false;
+        }
+        if (block.GetParent() != &function) {
+          std::stringstream ss;
+          ss << "Block " << block.id() << " should have parent "
+             << function.result_id() << " but instead has parent "
+             << block.GetParent() << ".";
+          consumer_(SPV_MSG_INFO, nullptr, {}, ss.str().c_str());
+          return false;
+        }
+      }
+    }
   }
   return true;
 }

source/fuzz/fuzzer.h

@@ -121,9 +121,11 @@ class Fuzzer {
   // the number of transformations that have been applied increases.
   bool ShouldContinueFuzzing();
 
-  // Applies |pass|, which must be a pass constructed with |ir_context|, and
-  // then returns true if and only if |ir_context| is valid.  |tools| is used to
-  // check validity.
+  // Applies |pass|, which must be a pass constructed with |ir_context|.
+  // If |validate_after_each_fuzzer_pass_| is not set, true is always returned.
+  // Otherwise, true is returned if and only if |ir_context| passes validation,
+  // and if every block has its enclosing function as its parent.  |tools| is
+  // used for checking validity.
   bool ApplyPassAndCheckValidity(FuzzerPass* pass,
                                  const spvtools::SpirvTools& tools) const;
 

source/fuzz/transformation_merge_function_returns.cpp

@@ -567,6 +567,7 @@ void TransformationMergeFunctionReturns::Apply(
   }
 
   // Insert the new return block at the end of the function.
+  outer_return_block->SetParent(function);
   function->AddBasicBlock(std::move(outer_return_block));
 
   // All analyses must be invalidated because the structure of the module was

test/fuzz/fuzz_test_util.cpp

@@ -81,11 +81,39 @@ bool IsEqual(const spv_target_env env, const std::vector<uint32_t>& binary_1,
 }
 
 bool IsValid(spv_target_env env, const opt::IRContext* ir) {
+  MessageConsumer consumer = kConsoleMessageConsumer;
+
+  // First, run the validator.
   std::vector<uint32_t> binary;
   ir->module()->ToBinary(&binary, false);
   SpirvTools t(env);
-  t.SetMessageConsumer(kConsoleMessageConsumer);
-  return t.Validate(binary);
+  t.SetMessageConsumer(consumer);
+  if (!t.Validate(binary)) {
+    return false;
+  }
+
+  // Now check that every block in the module has the appropriate parent
+  // function.
+  for (auto& function : *ir->module()) {
+    for (auto& block : function) {
+      if (block.GetParent() == nullptr) {
+        std::stringstream ss;
+        ss << "Block " << block.id() << " has no parent; its parent should be "
+           << function.result_id() << ".";
+        consumer(SPV_MSG_INFO, nullptr, {}, ss.str().c_str());
+        return false;
+      }
+      if (block.GetParent() != &function) {
+        std::stringstream ss;
+        ss << "Block " << block.id() << " should have parent "
+           << function.result_id() << " but instead has parent "
+           << block.GetParent() << ".";
+        consumer(SPV_MSG_INFO, nullptr, {}, ss.str().c_str());
+        return false;
+      }
+    }
+  }
+  return true;
 }
 
 std::string ToString(spv_target_env env, const opt::IRContext* ir) {

test/fuzz/fuzz_test_util.h

@@ -53,7 +53,8 @@ bool IsEqual(spv_target_env env, const std::vector<uint32_t>& binary_1,
              const opt::IRContext* ir_2);
 
 // Assembles the given IR context and returns true if and only if
-// the resulting binary is valid.
+// the resulting binary is valid and every basic block has its enclosing
+// function as its parent.
 bool IsValid(spv_target_env env, const opt::IRContext* ir);
 
 // Assembles the given IR context, then returns its disassembly as a string.