Avoid integrity check failures caused by propagating line instructions (#4096)

Propagating the OpLine/OpNoLine to preserve the debug information
through transformations results in integrity check failures because of
the extra line instructions. This commit lets spirv-opt skip the
integrity check when the code contains OpLine or OpNoLine.

source/opt/ir_loader.cpp

@@ -41,6 +41,7 @@ bool IrLoader::AddInstruction(const spv_parsed_instruction_t* inst) {
   ++inst_index_;
   const auto opcode = static_cast<SpvOp>(inst->opcode);
   if (IsDebugLineInst(opcode)) {
+    module()->SetContainsDebugInfo();
     last_line_inst_.reset();
     dbg_line_info_.push_back(
         Instruction(module()->context(), *inst, last_dbg_scope_));
@@ -61,12 +62,12 @@ bool IrLoader::AddInstruction(const spv_parsed_instruction_t* inst) {
           inlined_at = inst->words[kInlinedAtIndex];
         last_dbg_scope_ =
             DebugScope(inst->words[kLexicalScopeIndex], inlined_at);
-        module()->SetContainsDebugScope();
+        module()->SetContainsDebugInfo();
         return true;
       }
       if (ext_inst_key == OpenCLDebugInfo100DebugNoScope) {
         last_dbg_scope_ = DebugScope(kNoDebugScope, kNoInlinedAt);
-        module()->SetContainsDebugScope();
+        module()->SetContainsDebugInfo();
         return true;
       }
     } else {
@@ -78,12 +79,12 @@ bool IrLoader::AddInstruction(const spv_parsed_instruction_t* inst) {
           inlined_at = inst->words[kInlinedAtIndex];
         last_dbg_scope_ =
             DebugScope(inst->words[kLexicalScopeIndex], inlined_at);
-        module()->SetContainsDebugScope();
+        module()->SetContainsDebugInfo();
         return true;
       }
       if (ext_inst_key == DebugInfoDebugNoScope) {
         last_dbg_scope_ = DebugScope(kNoDebugScope, kNoInlinedAt);
-        module()->SetContainsDebugScope();
+        module()->SetContainsDebugInfo();
         return true;
       }
     }

source/opt/module.h

@@ -49,7 +49,7 @@ class Module {
   using const_inst_iterator = InstructionList::const_iterator;
 
   // Creates an empty module with zero'd header.
-  Module() : header_({}), contains_debug_scope_(false) {}
+  Module() : header_({}), contains_debug_info_(false) {}
 
   // Sets the header to the given |header|.
   void SetHeader(const ModuleHeader& header) { header_ = header; }
@@ -119,9 +119,9 @@ class Module {
   // Appends a function to this module.
   inline void AddFunction(std::unique_ptr<Function> f);
 
-  // Sets |contains_debug_scope_| as true.
-  inline void SetContainsDebugScope();
-  inline bool ContainsDebugScope() { return contains_debug_scope_; }
+  // Sets |contains_debug_info_| as true.
+  inline void SetContainsDebugInfo();
+  inline bool ContainsDebugInfo() { return contains_debug_info_; }
 
   // Returns a vector of pointers to type-declaration instructions in this
   // module.
@@ -301,8 +301,8 @@ class Module {
   // any instruction.  We record them here, so they will not be lost.
   std::vector<Instruction> trailing_dbg_line_info_;
 
-  // This module contains DebugScope or DebugNoScope.
-  bool contains_debug_scope_;
+  // This module contains DebugScope/DebugNoScope or OpLine/OpNoLine.
+  bool contains_debug_info_;
 };
 
 // Pretty-prints |module| to |str|. Returns |str|.
@@ -364,7 +364,7 @@ inline void Module::AddFunction(std::unique_ptr<Function> f) {
   functions_.emplace_back(std::move(f));
 }
 
-inline void Module::SetContainsDebugScope() { contains_debug_scope_ = true; }
+inline void Module::SetContainsDebugInfo() { contains_debug_info_ = true; }
 
 inline Module::inst_iterator Module::capability_begin() {
   return capabilities_.begin();

source/opt/optimizer.cpp

@@ -583,10 +583,12 @@ bool Optimizer::Run(const uint32_t* original_binary,
 #ifndef NDEBUG
   // We do not keep the result id of DebugScope in struct DebugScope.
   // Instead, we assign random ids for them, which results in integrity
+  // check failures. In addition, propagating the OpLine/OpNoLine to preserve
+  // the debug information through transformations results in integrity
   // check failures. We want to skip the integrity check when the module
-  // contains DebugScope instructions.
+  // contains DebugScope or OpLine/OpNoLine instructions.
   if (status == opt::Pass::Status::SuccessWithoutChange &&
-      !context->module()->ContainsDebugScope()) {
+      !context->module()->ContainsDebugInfo()) {
     std::vector<uint32_t> optimized_binary_with_nop;
     context->module()->ToBinary(&optimized_binary_with_nop,
                                 /* skip_nop = */ false);

test/opt/optimizer_test.cpp

@@ -762,6 +762,124 @@ OpFunctionEnd
       << "Was expecting the OpNop to have been removed.";
 }
 
+TEST(Optimizer, AvoidIntegrityCheckForExtraLineInfo) {
+  // Test that it avoids the integrity check when no optimizations are run and
+  // OpLines are propagated.
+  const std::string before = R"(OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+%1 = OpString "Test"
+%void = OpTypeVoid
+%3 = OpTypeFunction %void
+%uint = OpTypeInt 32 0
+%_ptr_Function_uint = OpTypePointer Function %uint
+%6 = OpFunction %void None %3
+%7 = OpLabel
+OpLine %1 10 0
+%8 = OpVariable %_ptr_Function_uint Function
+OpLine %1 10 0
+%9 = OpVariable %_ptr_Function_uint Function
+OpLine %1 20 0
+OpReturn
+OpFunctionEnd
+)";
+
+  const std::string after = R"(OpCapability Shader
+OpCapability Linkage
+OpMemoryModel Logical GLSL450
+%1 = OpString "Test"
+%void = OpTypeVoid
+%3 = OpTypeFunction %void
+%uint = OpTypeInt 32 0
+%_ptr_Function_uint = OpTypePointer Function %uint
+%6 = OpFunction %void None %3
+%7 = OpLabel
+OpLine %1 10 0
+%8 = OpVariable %_ptr_Function_uint Function
+%9 = OpVariable %_ptr_Function_uint Function
+OpLine %1 20 0
+OpReturn
+OpFunctionEnd
+)";
+
+  std::vector<uint32_t> binary;
+  SpirvTools tools(SPV_ENV_VULKAN_1_1);
+  tools.Assemble(before, &binary);
+
+  Optimizer opt(SPV_ENV_VULKAN_1_1);
+
+  std::vector<uint32_t> optimized;
+  class ValidatorOptions validator_options;
+  ASSERT_TRUE(opt.Run(binary.data(), binary.size(), &optimized,
+                      validator_options, true))
+      << before << "\n";
+
+  std::string disassembly;
+  tools.Disassemble(optimized.data(), optimized.size(), &disassembly);
+
+  EXPECT_EQ(after, disassembly)
+      << "Was expecting the OpLine to have been propagated.";
+}
+
+TEST(Optimizer, AvoidIntegrityCheckForDebugScope) {
+  // Test that it avoids the integrity check when the code contains DebugScope.
+  const std::string before = R"(OpCapability Shader
+%1 = OpExtInstImport "OpenCL.DebugInfo.100"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+%3 = OpString "simple_vs.hlsl"
+OpSource HLSL 600 %3
+OpName %main "main"
+%void = OpTypeVoid
+%5 = OpTypeFunction %void
+%6 = OpExtInst %void %1 DebugSource %3
+%7 = OpExtInst %void %1 DebugCompilationUnit 2 4 %6 HLSL
+%main = OpFunction %void None %5
+%14 = OpLabel
+%26 = OpExtInst %void %1 DebugScope %7
+OpReturn
+%27 = OpExtInst %void %1 DebugNoScope
+OpFunctionEnd
+)";
+
+  const std::string after = R"(OpCapability Shader
+%1 = OpExtInstImport "OpenCL.DebugInfo.100"
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %main "main"
+OpExecutionMode %main OriginUpperLeft
+%3 = OpString "simple_vs.hlsl"
+OpSource HLSL 600 %3
+OpName %main "main"
+%void = OpTypeVoid
+%5 = OpTypeFunction %void
+%6 = OpExtInst %void %1 DebugSource %3
+%7 = OpExtInst %void %1 DebugCompilationUnit 2 4 %6 HLSL
+%main = OpFunction %void None %5
+%8 = OpLabel
+%11 = OpExtInst %void %1 DebugScope %7
+OpReturn
+%12 = OpExtInst %void %1 DebugNoScope
+OpFunctionEnd
+)";
+
+  std::vector<uint32_t> binary;
+  SpirvTools tools(SPV_ENV_VULKAN_1_1);
+  tools.Assemble(before, &binary);
+
+  Optimizer opt(SPV_ENV_VULKAN_1_1);
+
+  std::vector<uint32_t> optimized;
+  ASSERT_TRUE(opt.Run(binary.data(), binary.size(), &optimized))
+      << before << "\n";
+
+  std::string disassembly;
+  tools.Disassemble(optimized.data(), optimized.size(), &disassembly);
+
+  EXPECT_EQ(after, disassembly)
+      << "Was expecting the result id of DebugScope to have been changed.";
+}
+
 }  // namespace
 }  // namespace opt
 }  // namespace spvtools