Use OverflowIdSource instead of passing overflow ids directly to the …

…transformation

source/fuzz/fuzzer_pass_flatten_conditional_branches.cpp

@@ -86,13 +86,9 @@ void FuzzerPassFlattenConditionalBranches::Apply() {
           {MakeInstructionDescriptor(GetIRContext(), instruction), info});
     }
 
-    // Add 10 overflow ids to account for possible changes in the module.
-    auto overflow_ids = GetFuzzerContext()->GetFreshIds(10);
-
     // Apply the transformation.
     ApplyTransformation(TransformationFlattenConditionalBranch(
-        header->id(), std::move(instructions_to_fresh_ids),
-        std::move(overflow_ids)));
+        header->id(), std::move(instructions_to_fresh_ids)));
   }
 }
 }  // namespace fuzz

source/fuzz/protobufs/spvtoolsfuzz.proto

@@ -1218,12 +1218,6 @@ message TransformationFlattenConditionalBranch {
   // enclosed inside smaller conditional before flattening the
   // main one, and the corresponding fresh ids needed.
   repeated InstToIdsForEnclosing inst_to_ids_for_enclosing = 2;
-
-  // A list of fresh ids to be used for instructions that need them, if
-  // they are not in instruction_to_fresh_ids. This allows the
-  // transformation to be applicable even if shrinking causes some
-  // unexpected instructions to appear in the module.
-  repeated uint32 overflow_id = 3;
 }
 
 message TransformationFunctionCall {

source/fuzz/transformation_flatten_conditional_branch.cpp

@@ -28,8 +28,7 @@ TransformationFlattenConditionalBranch::TransformationFlattenConditionalBranch(
     uint32_t header_block_id,
     std::vector<
         std::pair<protobufs::InstructionDescriptor, IdsForEnclosingInst>>
-        instructions_to_ids_for_enclosing,
-    std::vector<uint32_t> overflow_ids) {
+        instructions_to_ids_for_enclosing) {
   message_.set_header_block_id(header_block_id);
   for (auto const& pair : instructions_to_ids_for_enclosing) {
     protobufs::InstToIdsForEnclosing inst_to_ids;
@@ -41,14 +40,11 @@ TransformationFlattenConditionalBranch::TransformationFlattenConditionalBranch(
     inst_to_ids.set_placeholder_result_id(pair.second.placeholder_result_id);
     *message_.add_inst_to_ids_for_enclosing() = inst_to_ids;
   }
-  for (auto id : overflow_ids) {
-    message_.add_overflow_id(id);
-  }
 }
 
 bool TransformationFlattenConditionalBranch::IsApplicable(
     opt::IRContext* ir_context,
-    const TransformationContext& /* unused */) const {
+    const TransformationContext& transformation_context) const {
   uint32_t header_block_id = message_.header_block_id();
   auto header_block = fuzzerutil::MaybeFindBlock(ir_context, header_block_id);
 
@@ -81,14 +77,6 @@ bool TransformationFlattenConditionalBranch::IsApplicable(
 
     std::set<uint32_t> used_fresh_ids;
 
-    // Check the overflow ids.
-    for (uint32_t id : message_.overflow_id()) {
-      if (!CheckIdIsFreshAndNotUsedByThisTransformation(id, ir_context,
-                                                        &used_fresh_ids)) {
-        return false;
-      }
-    }
-
     // Check the ids in the map.
     for (const auto& inst_to_ids : instructions_to_ids) {
       // Check the ids needed for all of the instructions that need to be
@@ -115,25 +103,12 @@ bool TransformationFlattenConditionalBranch::IsApplicable(
     }
   }
 
-  // Keep track of the number of overflow ids still available in the overflow
-  // pool, as we go through the instructions.
-  int remaining_overflow_ids = message_.overflow_id_size();
-
+  // If some instructions that require ids are not in the map, the
+  // transformation needs overflow ids to be applicable.
   for (auto instruction : instructions_that_need_ids) {
-    // The ids needed depend on the type of instruction.
-    bool inst_needs_placeholder =
-        InstructionNeedsPlaceholder(ir_context, *instruction);
-
-    if (instructions_to_ids.count(instruction) == 0) {
-      // If there is no mapping, we need to rely on the pool of overflow ids,
-      // where there must be enough remaining ids, that is: 5 if the instruction
-      // needs a placeholder, 2 otherwise.
-
-      remaining_overflow_ids -= inst_needs_placeholder ? 5 : 2;
-
-      if (remaining_overflow_ids < 0) {
-        return false;
-      }
+    if (instructions_to_ids.count(instruction) == 0 &&
+        !transformation_context.GetOverflowIdSource()->HasOverflowIds()) {
+      return false;
     }
   }
 
@@ -157,9 +132,6 @@ void TransformationFlattenConditionalBranch::Apply(
   // Get the mapping from instructions to fresh ids.
   auto instructions_to_fresh_ids = GetInstructionsToIdsForEnclosing(ir_context);
 
-  // Keep track of the number of overflow ids used.
-  uint32_t overflow_ids_used = 0;
-
   auto branch_instruction = header_block->terminator();
 
   opt::BasicBlock* last_true_block = nullptr;
@@ -210,17 +182,23 @@ void TransformationFlattenConditionalBranch::Apply(
           fresh_ids = instructions_to_fresh_ids[instruction];
         } else {
           // If we could not get it from the map, use overflow ids.
-          fresh_ids.merge_block_id = message_.overflow_id(overflow_ids_used++);
+          fresh_ids.merge_block_id =
+              transformation_context->GetOverflowIdSource()
+                  ->GetNextOverflowId();
           fresh_ids.execute_block_id =
-              message_.overflow_id(overflow_ids_used++);
+              transformation_context->GetOverflowIdSource()
+                  ->GetNextOverflowId();
 
           if (InstructionNeedsPlaceholder(ir_context, *instruction)) {
             fresh_ids.actual_result_id =
-                message_.overflow_id(overflow_ids_used++);
+                transformation_context->GetOverflowIdSource()
+                    ->GetNextOverflowId();
             fresh_ids.alternative_block_id =
-                message_.overflow_id(overflow_ids_used++);
+                transformation_context->GetOverflowIdSource()
+                    ->GetNextOverflowId();
             fresh_ids.placeholder_result_id =
-                message_.overflow_id(overflow_ids_used++);
+                transformation_context->GetOverflowIdSource()
+                    ->GetNextOverflowId();
           }
         }
 

source/fuzz/transformation_flatten_conditional_branch.h

@@ -53,8 +53,7 @@ class TransformationFlattenConditionalBranch : public Transformation {
       uint32_t header_block_id,
       std::vector<
           std::pair<protobufs::InstructionDescriptor, IdsForEnclosingInst>>
-          instructions_to_ids_for_enclosing = {},
-      std::vector<uint32_t> overflow_ids = {});
+          instructions_to_ids_for_enclosing = {});
 
   // - |message_.header_block_id| must be the label id of a reachable selection
   //   header, which ends with an OpBranchConditional instruction.

test/fuzz/transformation_flatten_conditional_branch_test.cpp

@@ -14,6 +14,7 @@
 
 #include "source/fuzz/transformation_flatten_conditional_branch.h"
 
+#include "source/fuzz/counter_overflow_id_source.h"
 #include "source/fuzz/instruction_descriptor.h"
 #include "test/fuzz/fuzz_test_util.h"
 
@@ -389,32 +390,32 @@ TEST(TransformationFlattenConditionalBranchTest, LoadStoreFunctionCall) {
   TransformationContext transformation_context(&fact_manager,
                                                validator_options);
 
-  // The selection construct with header block %31 contains an OpLoad and an
-  // OpStore, requiring some fresh ids.
-  ASSERT_FALSE(TransformationFlattenConditionalBranch(31).IsApplicable(
-      context.get(), transformation_context));
+#ifndef NDEBUG
+  // The following checks lead to assertion failures, since some entries
+  // requiring fresh ids are not present in the map, and the transformation
+  // context does not have a source overflow ids.
 
-  // Not all of the instructions are given in the map and there are not enough
-  // overflow ids.
-  ASSERT_FALSE(TransformationFlattenConditionalBranch(
+  ASSERT_DEATH(TransformationFlattenConditionalBranch(31).IsApplicable(
+                   context.get(), transformation_context),
+               "Bad attempt to query whether overflow ids are available.");
+
+  ASSERT_DEATH(TransformationFlattenConditionalBranch(
                    31, {{MakeInstructionDescriptor(6, SpvOpLoad, 0),
                          {100, 101, 102, 103, 104}}})
-                   .IsApplicable(context.get(), transformation_context));
+                   .IsApplicable(context.get(), transformation_context),
+               "Bad attempt to query whether overflow ids are available.");
+#endif
 
   // The map maps from an instruction to a list with not enough fresh ids.
-  ASSERT_FALSE(
-      TransformationFlattenConditionalBranch(
-          31,
-          {{MakeInstructionDescriptor(6, SpvOpLoad, 0), {100, 101, 102, 103}}},
-          {105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115})
-          .IsApplicable(context.get(), transformation_context));
+  ASSERT_FALSE(TransformationFlattenConditionalBranch(
+                   31, {{MakeInstructionDescriptor(6, SpvOpLoad, 0),
+                         {100, 101, 102, 103}}})
+                   .IsApplicable(context.get(), transformation_context));
 
   // Not all fresh ids given are distinct.
   ASSERT_FALSE(TransformationFlattenConditionalBranch(
-                   31,
-                   {{MakeInstructionDescriptor(6, SpvOpLoad, 0),
-                     {100, 101, 102, 103, 104}}},
-                   {103, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115})
+                   31, {{MakeInstructionDescriptor(6, SpvOpLoad, 0),
+                         {100, 100, 102, 103, 104}}})
                    .IsApplicable(context.get(), transformation_context));
 
   // %48 heads a construct containing an OpSampledImage instruction.
@@ -433,17 +434,20 @@ TEST(TransformationFlattenConditionalBranchTest, LoadStoreFunctionCall) {
 
   transformation1.Apply(context.get(), &transformation_context);
 
+  // Make a new transformation context with a source of overflow ids.
+  TransformationContext new_transformation_context(
+      &fact_manager, validator_options,
+      MakeUnique<CounterOverflowIdSource>(1000));
+
   auto transformation2 = TransformationFlattenConditionalBranch(
-      36,
-      {{MakeInstructionDescriptor(8, SpvOpFunctionCall, 0),
-        {112, 108, 109, 110, 111}},
-       {MakeInstructionDescriptor(8, SpvOpStore, 0), {114, 113}}},
-      {115, 116});
+      36, {{MakeInstructionDescriptor(8, SpvOpFunctionCall, 0),
+            {112, 108, 109, 110, 111}},
+           {MakeInstructionDescriptor(8, SpvOpStore, 0), {114, 113}}});
 
   ASSERT_TRUE(
-      transformation2.IsApplicable(context.get(), transformation_context));
+      transformation2.IsApplicable(context.get(), new_transformation_context));
 
-  transformation2.Apply(context.get(), &transformation_context);
+  transformation2.Apply(context.get(), &new_transformation_context);
 
   ASSERT_TRUE(IsValid(env, context.get()));
 
@@ -527,12 +531,12 @@ TEST(TransformationFlattenConditionalBranchTest, LoadStoreFunctionCall) {
                OpBranch %45
          %45 = OpLabel
          %47 = OpAccessChain %21 %5 %14
-               OpSelectionMerge %115 None
-               OpBranchConditional %20 %115 %116
-        %116 = OpLabel
+               OpSelectionMerge %1000 None
+               OpBranchConditional %20 %1000 %1001
+       %1001 = OpLabel
                OpStore %47 %14
-               OpBranch %115
-        %115 = OpLabel
+               OpBranch %1000
+       %1000 = OpLabel
                OpBranch %46
          %46 = OpLabel
                OpStore %4 %14
@@ -615,10 +619,14 @@ TEST(TransformationFlattenConditionalBranchTest, EdgeCases) {
   TransformationContext transformation_context(&fact_manager,
                                                validator_options);
 
+#ifndef NDEBUG
   // The selection construct headed by %7 requires fresh ids because it contains
-  // a function call.
-  ASSERT_FALSE(TransformationFlattenConditionalBranch(7).IsApplicable(
-      context.get(), transformation_context));
+  // a function call. This causes an assertion failure because transformation
+  // context does not have a source of overflow ids.
+  ASSERT_DEATH(TransformationFlattenConditionalBranch(7).IsApplicable(
+                   context.get(), transformation_context),
+               "Bad attempt to query whether overflow ids are available.");
+#endif
 
   auto transformation = TransformationFlattenConditionalBranch(
       7, {{{MakeInstructionDescriptor(10, SpvOpFunctionCall, 0), {100, 101}}}});