spirv-fuzz: Do not flatten conditionals that create synonyms (#4030)

Fixes #4024.
KhronosGroup · Nov 24, 2020 · 5ffa320 · 5ffa320
1 parent cd590fa
commit 5ffa320
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 3 deletions.
diff --git a/source/fuzz/transformation_flatten_conditional_branch.cpp b/source/fuzz/transformation_flatten_conditional_branch.cpp
@@ -512,15 +512,31 @@ bool TransformationFlattenConditionalBranch::
       return false;
     }
 
+    // The base objects for all data descriptors involved in synonym facts.
+    std::unordered_set<uint32_t> synonym_base_objects;
+    for (auto* synonym :
+         transformation_context.GetFactManager()->GetAllSynonyms()) {
+      synonym_base_objects.insert(synonym->object());
+    }
+
     // Check all of the instructions in the block.
-    bool all_instructions_compatible =
-        block->WhileEachInst([ir_context, instructions_that_need_ids](
-                                 opt::Instruction* instruction) {
+    bool all_instructions_compatible = block->WhileEachInst(
+        [ir_context, instructions_that_need_ids,
+         &synonym_base_objects](opt::Instruction* instruction) {
           // We can ignore OpLabel instructions.
           if (instruction->opcode() == SpvOpLabel) {
             return true;
           }
 
+          // If the instruction is the base object of some synonym then we
+          // conservatively bail out: if a synonym ends up depending on an
+          // instruction that needs to be enclosed in a side-effect wrapper then
+          // it might no longer hold after we flatten the conditional.
+          if (instruction->result_id() &&
+              synonym_base_objects.count(instruction->result_id())) {
+            return false;
+          }
+
           // If the instruction is a branch, it must be an unconditional branch.
           if (instruction->IsBranch()) {
             return instruction->opcode() == SpvOpBranch;

diff --git a/source/fuzz/transformation_flatten_conditional_branch.h b/source/fuzz/transformation_flatten_conditional_branch.h
@@ -41,6 +41,8 @@ class TransformationFlattenConditionalBranch : public Transformation {
   //   single-exit region.
   // - The region must not contain barrier or OpSampledImage instructions.
   // - The region must not contain selection or loop constructs.
+  // - The region must not define ids that are the base objects for existing
+  //   synonym facts.
   // - For each instruction that requires additional fresh ids, then:
   //   - if the instruction is mapped to the required ids for enclosing it by
   //     |message_.side_effect_wrapper_info|, these must be valid (the

diff --git a/test/fuzz/transformation_flatten_conditional_branch_test.cpp b/test/fuzz/transformation_flatten_conditional_branch_test.cpp
@@ -2135,6 +2135,62 @@ TEST(TransformationFlattenConditionalBranchTest, ContainsContinueBlockTest) {
                    .IsApplicable(context.get(), transformation_context));
 }
 
+TEST(TransformationFlattenConditionalBranchTest, ContainsSynonymCreation) {
+  std::string shader = R"(
+               OpCapability Shader
+          %1 = OpExtInstImport "GLSL.std.450"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint Fragment %4 "main"
+               OpExecutionMode %4 OriginUpperLeft
+               OpSource ESSL 320
+               OpName %4 "main"
+          %2 = OpTypeVoid
+          %3 = OpTypeFunction %2
+          %6 = OpTypeBool
+          %7 = OpConstantFalse %6
+          %8 = OpTypeInt 32 0
+          %9 = OpTypePointer Function %8
+         %10 = OpConstant %8 42
+         %80 = OpConstant %8 0
+          %4 = OpFunction %2 None %3
+         %11 = OpLabel
+         %20 = OpVariable %9 Function
+               OpBranch %12
+         %12 = OpLabel
+               OpSelectionMerge %31 None
+               OpBranchConditional %7 %30 %31
+         %30 = OpLabel
+               OpStore %20 %10
+         %21 = OpLoad %8 %20
+               OpBranch %31
+         %31 = OpLabel
+               OpBranch %14
+         %14 = OpLabel
+               OpReturn
+               OpFunctionEnd
+  )";
+
+  const auto env = SPV_ENV_UNIVERSAL_1_3;
+  const auto consumer = nullptr;
+  const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
+  spvtools::ValidatorOptions validator_options;
+  ASSERT_TRUE(fuzzerutil::IsValidAndWellFormed(context.get(), validator_options,
+                                               kConsoleMessageConsumer));
+  TransformationContext transformation_context(
+      MakeUnique<FactManager>(context.get()), validator_options);
+
+  transformation_context.GetFactManager()->AddFactDataSynonym(
+      MakeDataDescriptor(10, {}), MakeDataDescriptor(21, {}));
+  ASSERT_FALSE(TransformationFlattenConditionalBranch(
+                   12, true, 0, 0, 0,
+                   {MakeSideEffectWrapperInfo(
+                        MakeInstructionDescriptor(30, SpvOpStore, 0), 100, 101),
+                    MakeSideEffectWrapperInfo(
+                        MakeInstructionDescriptor(21, SpvOpLoad, 0), 102, 103,
+                        104, 105, 106, 80)})
+                   .IsApplicable(context.get(), transformation_context));
+}
+
 }  // namespace
 }  // namespace fuzz
 }  // namespace spvtools