diff options
author | Steven Perron <stevenperron@google.com> | 2022-07-05 21:14:29 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-07-05 21:14:29 +0300 |
commit | d5a3bfcf2ffd154a244e7ffae54dd1766d98efa4 (patch) | |
tree | e66681a4e2a25a2de672013ed784b2e842d34824 | |
parent | 6803cc51268a56ec6d6b622fb91fac012cb589ef (diff) |
Avoid undefined behaviour when getting debug opcode (#4842)
If the `instruction` operand in an extended instruction instruction is
too large, it causes undefined behaviour when that value is cast to the
enum for the corresponding set. This is done with the
NonSemanticDebug100 instruction set. We need to avoid the undefined
behaviour.
Fixes #4727
-rw-r--r-- | source/opt/instruction.cpp | 8 | ||||
-rw-r--r-- | test/opt/instruction_test.cpp | 39 |
2 files changed, 45 insertions, 2 deletions
diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp index 418f1213a..6a8daea31 100644 --- a/source/opt/instruction.cpp +++ b/source/opt/instruction.cpp @@ -693,8 +693,12 @@ NonSemanticShaderDebugInfo100Instructions Instruction::GetShader100DebugOpcode() return NonSemanticShaderDebugInfo100InstructionsMax; } - return NonSemanticShaderDebugInfo100Instructions( - GetSingleWordInOperand(kExtInstInstructionInIdx)); + uint32_t opcode = GetSingleWordInOperand(kExtInstInstructionInIdx); + if (opcode >= NonSemanticShaderDebugInfo100InstructionsMax) { + return NonSemanticShaderDebugInfo100InstructionsMax; + } + + return NonSemanticShaderDebugInfo100Instructions(opcode); } CommonDebugInfoInstructions Instruction::GetCommonDebugOpcode() const { diff --git a/test/opt/instruction_test.cpp b/test/opt/instruction_test.cpp index 2a48134d9..dd749ab48 100644 --- a/test/opt/instruction_test.cpp +++ b/test/opt/instruction_test.cpp @@ -1525,6 +1525,45 @@ OpFunctionEnd EXPECT_EQ(false, inst->IsVulkanStorageTexelBuffer()); } +TEST_F(DescriptorTypeTest, GetShader100DebugOpcode) { + const std::string text = R"( + OpCapability Shader + %1 = OpExtInstImport "NonSemantic.Shader.DebugInfo.100" + %2 = OpString "ps.hlsl" + %3 = OpString "#line 1 \"ps.hlsl\"" + %void = OpTypeVoid + %5 = OpExtInst %void %1 DebugExpression + %6 = OpExtInst %void %1 DebugSource %2 %3 +)"; + + SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS); + std::unique_ptr<IRContext> context = + BuildModule(SPV_ENV_UNIVERSAL_1_2, nullptr, text); + Instruction* debug_expression = context->get_def_use_mgr()->GetDef(5); + EXPECT_EQ(debug_expression->GetShader100DebugOpcode(), + NonSemanticShaderDebugInfo100DebugExpression); + Instruction* debug_source = context->get_def_use_mgr()->GetDef(6); + EXPECT_EQ(debug_source->GetShader100DebugOpcode(), + NonSemanticShaderDebugInfo100DebugSource); + + // Test that an opcode larger than the max will return Max. This instruction + // cannot be in the assembly above because the assembler expects the string + // for the opcode, so we cannot use an arbitrary number. However, a binary + // file could have an arbitrary number. + std::unique_ptr<Instruction> past_max(debug_expression->Clone(context.get())); + const uint32_t kExtInstOpcodeInIndex = 1; + uint32_t large_opcode = NonSemanticShaderDebugInfo100InstructionsMax + 2; + past_max->SetInOperand(kExtInstOpcodeInIndex, {large_opcode}); + EXPECT_EQ(past_max->GetShader100DebugOpcode(), + NonSemanticShaderDebugInfo100InstructionsMax); + + // Test that an opcode without a value in the enum, but less than Max returns + // the same value. + uint32_t opcode = NonSemanticShaderDebugInfo100InstructionsMax - 2; + past_max->SetInOperand(kExtInstOpcodeInIndex, {opcode}); + EXPECT_EQ(past_max->GetShader100DebugOpcode(), opcode); +} + } // namespace } // namespace opt } // namespace spvtools |