diff: Fix crash in OpString matching (#5988)

Fixes #5971
KhronosGroup · Feb 10, 2025 · f95b07a · f95b07a
1 parent 114920c
commit f95b07a
Show file tree

Hide file tree

Showing 5 changed files with 261 additions and 0 deletions.
diff --git a/source/diff/diff.cpp b/source/diff/diff.cpp
@@ -198,6 +198,10 @@ struct IdInstructions {
         forward_pointer_map_(module->IdBound()) {
     // Map ids from all sections to instructions that define them.
     MapIdsToInstruction(module->ext_inst_imports());
+    MapIdsToInstruction(module->debugs1());
+    MapIdsToInstruction(module->debugs2());
+    MapIdsToInstruction(module->debugs3());
+    MapIdsToInstruction(module->ext_inst_debuginfo());
     MapIdsToInstruction(module->types_values());
     for (const opt::Function& function : *module) {
       function.ForEachInst(

diff --git a/test/diff/diff_files/diff_test_files_autogen.cmake b/test/diff/diff_files/diff_test_files_autogen.cmake
@@ -44,5 +44,6 @@ list(APPEND DIFF_TEST_FILES
 "diff_files/spec_constant_composite_autogen.cpp"
 "diff_files/spec_constant_op_autogen.cpp"
 "diff_files/spec_constant_specid_autogen.cpp"
+"diff_files/string_in_ext_inst_autogen.cpp"
 "diff_files/unrelated_shaders_autogen.cpp"
 )
diff --git a/test/diff/diff_files/string_in_ext_inst_autogen.cpp b/test/diff/diff_files/string_in_ext_inst_autogen.cpp
@@ -0,0 +1,205 @@
+// GENERATED FILE - DO NOT EDIT.
+// Generated by generate_tests.py
+//
+// Copyright (c) 2022 Google LLC.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "../diff_test_utils.h"
+
+#include "gtest/gtest.h"
+
+namespace spvtools {
+namespace diff {
+namespace {
+
+// Tests a diff where the an OpString is used only as parameter of OpExtInst.
+constexpr char kSrc[] = R"(               OpCapability Shader
+               OpExtension "SPV_KHR_non_semantic_info"
+          %1 = OpExtInstImport "GLSL.std.450"
+         %12 = OpExtInstImport "NonSemantic.DebugPrintf"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint GLCompute %main "main"
+               OpExecutionMode %main LocalSize 1 1 1
+         %10 = OpString "unsigned == %u"
+               OpSource GLSL 450
+               OpSourceExtension "GL_EXT_debug_printf"
+               OpName %main "main"
+               OpName %foo "foo"
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+       %uint = OpTypeInt 32 0
+%_ptr_Function_uint = OpTypePointer Function %uint
+   %uint_127 = OpConstant %uint 127
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+        %foo = OpVariable %_ptr_Function_uint Function
+               OpStore %foo %uint_127
+         %11 = OpLoad %uint %foo
+         %13 = OpExtInst %void %12 1 %10 %11
+               OpReturn
+               OpFunctionEnd)";
+constexpr char kDst[] = R"(               OpCapability Shader
+               OpExtension "SPV_KHR_non_semantic_info"
+          %1 = OpExtInstImport "GLSL.std.450"
+         %12 = OpExtInstImport "NonSemantic.DebugPrintf"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint GLCompute %main "main"
+               OpExecutionMode %main LocalSize 1 1 1
+         %10 = OpString "signed == %d"
+               OpSource GLSL 450
+               OpSourceExtension "GL_EXT_debug_printf"
+               OpName %main "main"
+               OpName %foo "foo"
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+       %uint = OpTypeInt 32 0
+%_ptr_Function_uint = OpTypePointer Function %uint
+   %uint_127 = OpConstant %uint 127
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+        %foo = OpVariable %_ptr_Function_uint Function
+               OpStore %foo %uint_127
+         %11 = OpLoad %uint %foo
+         %13 = OpExtInst %void %12 1 %10 %11
+               OpReturn
+               OpFunctionEnd
+)";
+
+TEST(DiffTest, StringInExtInst) {
+  constexpr char kDiff[] = R"( ; SPIR-V
+ ; Version: 1.6
+ ; Generator: Khronos SPIR-V Tools Assembler; 0
+-; Bound: 14
++; Bound: 15
+ ; Schema: 0
+ OpCapability Shader
+ OpExtension "SPV_KHR_non_semantic_info"
+ %1 = OpExtInstImport "GLSL.std.450"
+ %12 = OpExtInstImport "NonSemantic.DebugPrintf"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint GLCompute %2 "main"
+ OpExecutionMode %2 LocalSize 1 1 1
+-%10 = OpString "unsigned == %u"
++%14 = OpString "signed == %d"
+ OpSource GLSL 450
+ OpSourceExtension "GL_EXT_debug_printf"
+ OpName %2 "main"
+ OpName %4 "foo"
+ %6 = OpTypeVoid
+ %3 = OpTypeFunction %6
+ %7 = OpTypeInt 32 0
+ %8 = OpTypePointer Function %7
+ %9 = OpConstant %7 127
+ %2 = OpFunction %6 None %3
+ %5 = OpLabel
+ %4 = OpVariable %8 Function
+ OpStore %4 %9
+ %11 = OpLoad %7 %4
+-%13 = OpExtInst %6 %12 1 %10 %11
++%13 = OpExtInst %6 %12 1 %14 %11
+ OpReturn
+ OpFunctionEnd
+)";
+  Options options;
+  DoStringDiffTest(kSrc, kDst, kDiff, options);
+}
+
+TEST(DiffTest, StringInExtInstNoDebug) {
+  constexpr char kSrcNoDebug[] = R"(               OpCapability Shader
+               OpExtension "SPV_KHR_non_semantic_info"
+          %1 = OpExtInstImport "GLSL.std.450"
+         %12 = OpExtInstImport "NonSemantic.DebugPrintf"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint GLCompute %main "main"
+               OpExecutionMode %main LocalSize 1 1 1
+         %10 = OpString "unsigned == %u"
+               OpSource GLSL 450
+               OpSourceExtension "GL_EXT_debug_printf"
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+       %uint = OpTypeInt 32 0
+%_ptr_Function_uint = OpTypePointer Function %uint
+   %uint_127 = OpConstant %uint 127
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+        %foo = OpVariable %_ptr_Function_uint Function
+               OpStore %foo %uint_127
+         %11 = OpLoad %uint %foo
+         %13 = OpExtInst %void %12 1 %10 %11
+               OpReturn
+               OpFunctionEnd
+)";
+  constexpr char kDstNoDebug[] = R"(               OpCapability Shader
+               OpExtension "SPV_KHR_non_semantic_info"
+          %1 = OpExtInstImport "GLSL.std.450"
+         %12 = OpExtInstImport "NonSemantic.DebugPrintf"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint GLCompute %main "main"
+               OpExecutionMode %main LocalSize 1 1 1
+         %10 = OpString "signed == %d"
+               OpSource GLSL 450
+               OpSourceExtension "GL_EXT_debug_printf"
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+       %uint = OpTypeInt 32 0
+%_ptr_Function_uint = OpTypePointer Function %uint
+   %uint_127 = OpConstant %uint 127
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+        %foo = OpVariable %_ptr_Function_uint Function
+               OpStore %foo %uint_127
+         %11 = OpLoad %uint %foo
+         %13 = OpExtInst %void %12 1 %10 %11
+               OpReturn
+               OpFunctionEnd
+)";
+  constexpr char kDiff[] = R"( ; SPIR-V
+ ; Version: 1.6
+ ; Generator: Khronos SPIR-V Tools Assembler; 0
+-; Bound: 14
++; Bound: 15
+ ; Schema: 0
+ OpCapability Shader
+ OpExtension "SPV_KHR_non_semantic_info"
+ %1 = OpExtInstImport "GLSL.std.450"
+ %12 = OpExtInstImport "NonSemantic.DebugPrintf"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint GLCompute %2 "main"
+ OpExecutionMode %2 LocalSize 1 1 1
+-%10 = OpString "unsigned == %u"
++%14 = OpString "signed == %d"
+ OpSource GLSL 450
+ OpSourceExtension "GL_EXT_debug_printf"
+ %4 = OpTypeVoid
+ %3 = OpTypeFunction %4
+ %6 = OpTypeInt 32 0
+ %7 = OpTypePointer Function %6
+ %8 = OpConstant %6 127
+ %2 = OpFunction %4 None %3
+ %5 = OpLabel
+ %9 = OpVariable %7 Function
+ OpStore %9 %8
+ %11 = OpLoad %6 %9
+-%13 = OpExtInst %4 %12 1 %10 %11
++%13 = OpExtInst %4 %12 1 %14 %11
+ OpReturn
+ OpFunctionEnd
+)";
+  Options options;
+  DoStringDiffTest(kSrcNoDebug, kDstNoDebug, kDiff, options);
+}
+
+}  // namespace
+}  // namespace diff
+}  // namespace spvtools
diff --git a/test/diff/diff_files/string_in_ext_inst_dst.spvasm b/test/diff/diff_files/string_in_ext_inst_dst.spvasm
@@ -0,0 +1,25 @@
+               OpCapability Shader
+               OpExtension "SPV_KHR_non_semantic_info"
+          %1 = OpExtInstImport "GLSL.std.450"
+         %12 = OpExtInstImport "NonSemantic.DebugPrintf"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint GLCompute %main "main"
+               OpExecutionMode %main LocalSize 1 1 1
+         %10 = OpString "signed == %d"
+               OpSource GLSL 450
+               OpSourceExtension "GL_EXT_debug_printf"
+               OpName %main "main"
+               OpName %foo "foo"
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+       %uint = OpTypeInt 32 0
+%_ptr_Function_uint = OpTypePointer Function %uint
+   %uint_127 = OpConstant %uint 127
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+        %foo = OpVariable %_ptr_Function_uint Function
+               OpStore %foo %uint_127
+         %11 = OpLoad %uint %foo
+         %13 = OpExtInst %void %12 1 %10 %11
+               OpReturn
+               OpFunctionEnd
diff --git a/test/diff/diff_files/string_in_ext_inst_src.spvasm b/test/diff/diff_files/string_in_ext_inst_src.spvasm
@@ -0,0 +1,26 @@
+;; Tests a diff where the an OpString is used only as parameter of OpExtInst.
+               OpCapability Shader
+               OpExtension "SPV_KHR_non_semantic_info"
+          %1 = OpExtInstImport "GLSL.std.450"
+         %12 = OpExtInstImport "NonSemantic.DebugPrintf"
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint GLCompute %main "main"
+               OpExecutionMode %main LocalSize 1 1 1
+         %10 = OpString "unsigned == %u"
+               OpSource GLSL 450
+               OpSourceExtension "GL_EXT_debug_printf"
+               OpName %main "main"
+               OpName %foo "foo"
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+       %uint = OpTypeInt 32 0
+%_ptr_Function_uint = OpTypePointer Function %uint
+   %uint_127 = OpConstant %uint 127
+       %main = OpFunction %void None %3
+          %5 = OpLabel
+        %foo = OpVariable %_ptr_Function_uint Function
+               OpStore %foo %uint_127
+         %11 = OpLoad %uint %foo
+         %13 = OpExtInst %void %12 1 %10 %11
+               OpReturn
+               OpFunctionEnd