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.
diff --git a/source/opt/ir_loader.cpp b/source/opt/ir_loader.cpp
index 70e5144..e443ebb 100644
--- a/source/opt/ir_loader.cpp
+++ b/source/opt/ir_loader.cpp
@@ -41,6 +41,7 @@
++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 @@
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 @@
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;
}
}
diff --git a/source/opt/module.h b/source/opt/module.h
index 2c96f02..d609b60 100644
--- a/source/opt/module.h
+++ b/source/opt/module.h
@@ -49,7 +49,7 @@
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 @@
// 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 @@
// 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 @@
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();
diff --git a/source/opt/optimizer.cpp b/source/opt/optimizer.cpp
index 8726ff9..c2a21f2 100644
--- a/source/opt/optimizer.cpp
+++ b/source/opt/optimizer.cpp
@@ -583,10 +583,12 @@
#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);
diff --git a/test/opt/optimizer_test.cpp b/test/opt/optimizer_test.cpp
index 945aa78..de79536 100644
--- a/test/opt/optimizer_test.cpp
+++ b/test/opt/optimizer_test.cpp
@@ -762,6 +762,124 @@
<< "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
