[val, opt] Disallow extra NSDI operands without option

include/spirv-tools/libspirv.h

@@ -784,6 +784,11 @@ SPIRV_TOOLS_EXPORT void spvValidatorOptionsSetAllowOffsetTextureOperand(
 SPIRV_TOOLS_EXPORT void spvValidatorOptionsSetAllowVulkan32BitBitwise(
     spv_validator_options options, bool val);
 
+// Records whether or not the validator should bypass version checks for
+// NonSemantic.Shader.DebugInfo instructions.
+SPIRV_TOOLS_EXPORT void spvValidatorOptionsSetAllowUnknownNsdiVersion(
+    spv_validator_options options, bool val);
+
 // Whether friendly names should be used in validation error messages.
 SPIRV_TOOLS_EXPORT void spvValidatorOptionsSetFriendlyNames(
     spv_validator_options options, bool val);

include/spirv-tools/libspirv.hpp

@@ -138,6 +138,12 @@ class SPIRV_TOOLS_EXPORT ValidatorOptions {
     spvValidatorOptionsSetAllowVulkan32BitBitwise(options_, val);
   }
 
+  // Records whether or not the validator should bypass version checks for
+  // NonSemantic.Shader.DebugInfo instructions.
+  void SetAllowUnknownNsdiVersion(bool val) {
+    spvValidatorOptionsSetAllowUnknownNsdiVersion(options_, val);
+  }
+
   // Records whether or not the validator should relax the rules on pointer
   // usage in logical addressing mode.
   //

source/spirv_validator_options.cpp

@@ -136,6 +136,11 @@ void spvValidatorOptionsSetAllowVulkan32BitBitwise(
   options->allow_vulkan_32_bit_bitwise = val;
 }
 
+void spvValidatorOptionsSetAllowUnknownNsdiVersion(
+    spv_validator_options options, bool val) {
+  options->allow_unknown_nsdi_version = val;
+}
+
 void spvValidatorOptionsSetFriendlyNames(spv_validator_options options,
                                          bool val) {
   options->use_friendly_names = val;

source/spirv_validator_options.h

@@ -50,6 +50,7 @@ struct spv_validator_options_t {
         allow_localsizeid(false),
         allow_offset_texture_operand(false),
         allow_vulkan_32_bit_bitwise(false),
+        allow_unknown_nsdi_version(false),
         before_hlsl_legalization(false),
         use_friendly_names(true) {}
 
@@ -64,6 +65,7 @@ struct spv_validator_options_t {
   bool allow_localsizeid;
   bool allow_offset_texture_operand;
   bool allow_vulkan_32_bit_bitwise;
+  bool allow_unknown_nsdi_version;
   bool before_hlsl_legalization;
   bool use_friendly_names;
 };

source/val/validate_extensions.cpp

@@ -3513,6 +3513,33 @@ spv_result_t ValidateExtInstDebugInfo(ValidationState_t& _,
 
   // Handle any non-common NonSemanticShaderDebugInfo instructions.
   if (vulkanDebugInfo) {
+    if (!_.options()->allow_unknown_nsdi_version) {
+      if (nsdi_version > kNSDIKnownVersion) {
+        return _.diag(SPV_ERROR_INVALID_DATA, inst)
+               << GetExtInstName(_, inst) << ": "
+               << "using an unknown version. Latest known version is "
+               << kNSDIKnownVersion;
+      }
+
+      const ExtInstDesc* desc = nullptr;
+      if (LookupExtInst(ext_inst_type, ext_inst_index, &desc) == SPV_SUCCESS &&
+          desc) {
+        auto op_type = desc->operands().back();
+        if (!spvOperandIsVariable(op_type)) {
+          size_t max_operands = desc->operands().size();
+          assert(inst->operands().size() >= 4);
+          size_t num_ext_operands = inst->operands().size() - 4;
+          if (num_ext_operands > max_operands) {
+            return _.diag(SPV_ERROR_INVALID_DATA, inst)
+                   << GetExtInstName(_, inst) << ": "
+                   << "incorrect number of operands: expected at most "
+                   << max_operands << " operands, but found"
+                   << num_ext_operands;
+          }
+        }
+      }
+    }
+
     const NonSemanticShaderDebugInfoInstructions ext_inst_key =
         NonSemanticShaderDebugInfoInstructions(ext_inst_index);
     switch (ext_inst_key) {

test/tools/opt/flags.py

@@ -51,6 +51,56 @@ class TestHelpFlag(expect.ReturnCodeIsZero, expect.StdoutMatch):
   expected_stdout = re.compile(r'.*The SPIR-V binary is read from <input>')
 
 
+@inside_spirv_testsuite('SpirvOptFlags')
+class TestAllowUnknownNsdiVersion(expect.ValidObjectFile1_6):
+  """Tests that spirv-opt accepts --allow-unknown-nsdi-version."""
+
+  shader = placeholder.FileSPIRVShader("""
+         OpCapability Shader
+         OpExtension "SPV_KHR_non_semantic_info"
+    %1 = OpExtInstImport "NonSemantic.Shader.DebugInfo.9999"
+         OpMemoryModel Logical GLSL450
+         OpEntryPoint Vertex %4 "main"
+  %src = OpString "simple.hlsl"
+ %code = OpString "int main() {}"
+         OpName %4 "main"
+    %2 = OpTypeVoid
+    %3 = OpTypeFunction %2
+%dbg_src = OpExtInst %2 %1 DebugSource %src %code
+    %4 = OpFunction %2 None %3
+    %5 = OpLabel
+         OpReturn
+         OpFunctionEnd""", '.spvasm')
+  output = placeholder.TempFileName('output.spv')
+  spirv_args = [shader, '-o', output, '-O', '--allow-unknown-nsdi-version']
+  expected_object_filenames = (output)
+
+
+@inside_spirv_testsuite('SpirvOptFlags')
+class TestAllowUnknownNsdiVersionNegative(expect.ErrorMessageSubstr):
+  """Tests that spirv-opt fails without --allow-unknown-nsdi-version."""
+
+  shader = placeholder.FileSPIRVShader("""
+         OpCapability Shader
+         OpExtension "SPV_KHR_non_semantic_info"
+    %1 = OpExtInstImport "NonSemantic.Shader.DebugInfo.9999"
+         OpMemoryModel Logical GLSL450
+         OpEntryPoint Vertex %4 "main"
+  %src = OpString "simple.hlsl"
+ %code = OpString "int main() {}"
+         OpName %4 "main"
+    %2 = OpTypeVoid
+    %3 = OpTypeFunction %2
+%dbg_src = OpExtInst %2 %1 DebugSource %src %code
+    %4 = OpFunction %2 None %3
+    %5 = OpLabel
+         OpReturn
+         OpFunctionEnd""", '.spvasm')
+  output = placeholder.TempFileName('output.spv')
+  spirv_args = [shader, '-o', output, '-O']
+  expected_error_substr = 'using an unknown version. Latest known version is'
+
+
 @inside_spirv_testsuite('SpirvOptFlags')
 class TestValidPassFlags(expect.ValidObjectFile1_6,
                          expect.ExecutedListOfPasses):

test/val/val_ext_inst_debug_test.cpp

@@ -5849,11 +5849,32 @@ TEST_F(ValidateVulkan100DebugInfo, DebugTypeBasicExtraOperand) {
 %float_info = OpExtInst %void %DbgExt DebugTypeBasic %float_name %u32_32 %u32_3 %u32_0 %u32_1
 )";
 
+  spvValidatorOptionsSetAllowUnknownNsdiVersion(getValidatorOptions(), true);
   CompileSuccessfully(GenerateShaderCodeForDebugInfo(
       src, "", dbg_inst_header, "", shader_extension_9999, "Vertex"));
   ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
 }
 
+TEST_F(ValidateVulkan100DebugInfo, DebugTypeBasicExtraOperandFail) {
+  const std::string src = R"(
+%src = OpString "simple.hlsl"
+%code = OpString "int main() {}"
+%float_name = OpString "float"
+)";
+
+  const std::string dbg_inst_header = R"(
+%dbg_src = OpExtInst %void %DbgExt DebugSource %src %code
+%comp_unit = OpExtInst %void %DbgExt DebugCompilationUnit %u32_2 %u32_4 %dbg_src %u32_5
+%float_info = OpExtInst %void %DbgExt DebugTypeBasic %float_name %u32_32 %u32_3 %u32_0 %u32_1
+)";
+
+  CompileSuccessfully(GenerateShaderCodeForDebugInfo(
+      src, "", dbg_inst_header, "", shader_extension_9999, "Vertex"));
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("using an unknown version. Latest known version is"));
+}
+
 TEST_F(ValidateVulkan100DebugInfo, UnknownInstructionAccepted) {
   // Opcode 20000 is not defined in NSDI 100 (highest known opcode is 108).
   // Both the text assembler and binary decoder handle it via the VARIABLE_ID
@@ -5890,6 +5911,7 @@ TEST_F(ValidateVulkan100DebugInfo, DebugTypeBasicTwoExtraOperands) {
 %float_info = OpExtInst %void %DbgExt DebugTypeBasic %float_name %u32_32 %u32_3 %u32_0 %u32_1 %u32_2
 )";
 
+  spvValidatorOptionsSetAllowUnknownNsdiVersion(getValidatorOptions(), true);
   CompileSuccessfully(GenerateShaderCodeForDebugInfo(
       src, "", dbg_inst_header, "", shader_extension_9999, "Vertex"));
   ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
@@ -5909,6 +5931,7 @@ TEST_F(ValidateVulkan100DebugInfo, DebugSourceExtraOperand) {
 %comp_unit = OpExtInst %void %DbgExt DebugCompilationUnit %u32_2 %u32_4 %dbg_src %u32_5
 )";
 
+  spvValidatorOptionsSetAllowUnknownNsdiVersion(getValidatorOptions(), true);
   CompileSuccessfully(GenerateShaderCodeForDebugInfo(
       src, "", dbg_inst_header, "", shader_extension_100, "Vertex"));
   ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
@@ -5969,6 +5992,7 @@ TEST_F(ValidateVulkan100DebugInfo, DebugNoScopeExtraOperandInBody) {
 %no_scope = OpExtInst %void %DbgExt DebugNoScope %u32_0
 )";
 
+  spvValidatorOptionsSetAllowUnknownNsdiVersion(getValidatorOptions(), true);
   CompileSuccessfully(GenerateShaderCodeForDebugInfo(
       src, "", dbg_inst_header, body, shader_extension_100, "Vertex"));
   ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());

tools/opt/opt.cpp

@@ -97,6 +97,10 @@ NOTE: The optimizer is a work in progress.
 Options (in lexicographical order):)",
       program, program);
   printf(R"(
+  --allow-unknown-nsdi-version
+               Forwards this option to the validator.  See the validator help
+               for details.)");
+  printf(R"(
   --amd-ext-to-khr
                Replaces the extensions VK_AMD_shader_ballot, VK_AMD_gcn_shader,
                and VK_AMD_shader_trinary_minmax with equivalent code using core
@@ -830,6 +834,8 @@ OptStatus ParseFlags(int argc, const char** argv,
         optimizer->SetValidateAfterAll(true);
       } else if (0 == strcmp(cur_arg, "--before-hlsl-legalization")) {
         validator_options->SetBeforeHlslLegalization(true);
+      } else if (0 == strcmp(cur_arg, "--allow-unknown-nsdi-version")) {
+        validator_options->SetAllowUnknownNsdiVersion(true);
       } else if (0 == strcmp(cur_arg, "--relax-logical-pointer")) {
         validator_options->SetRelaxLogicalPointer(true);
       } else if (0 == strcmp(cur_arg, "--relax-block-layout")) {

tools/val/val.cpp

@@ -74,6 +74,8 @@ NOTE: The validator is a work in progress.
                                    be allowed by the target environment.
   --allow-vulkan-32-bit-bitwise    Allow use of non-32 bit for the Base operand where it would otherwise
                                    not be allowed by the target environment.
+  --allow-unknown-nsdi-version     Allow use of NonSemantic.Shader.DebugInfo instructions with a version
+                                   number higher than the latest known version.
   --before-hlsl-legalization       Allows code patterns that are intended to be
                                    fixed by spirv-opt's legalization passes.
   --version                        Display validator version information.
@@ -220,6 +222,8 @@ int main(int argc, char** argv) {
         options.SetAllowOffsetTextureOperand(true);
       } else if (0 == strcmp(cur_arg, "--allow-vulkan-32-bit-bitwise")) {
         options.SetAllowVulkan32BitBitwise(true);
+      } else if (0 == strcmp(cur_arg, "--allow-unknown-nsdi-version")) {
+        options.SetAllowUnknownNsdiVersion(true);
       } else if (0 == strcmp(cur_arg, "--relax-struct-store")) {
         options.SetRelaxStructStore(true);
       } else if (0 == cur_arg[1]) {