diff --git a/include/spirv-tools/libspirv.h b/include/spirv-tools/libspirv.h index 0c5b129f67..b6d94e1233 100644 --- a/include/spirv-tools/libspirv.h +++ b/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); diff --git a/include/spirv-tools/libspirv.hpp b/include/spirv-tools/libspirv.hpp index 1b3ed86a80..96ee642f10 100644 --- a/include/spirv-tools/libspirv.hpp +++ b/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. // diff --git a/source/spirv_validator_options.cpp b/source/spirv_validator_options.cpp index a9591f6b35..81dc8b3054 100644 --- a/source/spirv_validator_options.cpp +++ b/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; diff --git a/source/spirv_validator_options.h b/source/spirv_validator_options.h index 9f0c125436..1695649898 100644 --- a/source/spirv_validator_options.h +++ b/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; }; diff --git a/source/val/validate_extensions.cpp b/source/val/validate_extensions.cpp index 5b6a1d0127..69714a6108 100644 --- a/source/val/validate_extensions.cpp +++ b/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) { diff --git a/test/tools/opt/flags.py b/test/tools/opt/flags.py index 2c7d7cad9e..3c607be5de 100644 --- a/test/tools/opt/flags.py +++ b/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 ') +@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): diff --git a/test/val/val_ext_inst_debug_test.cpp b/test/val/val_ext_inst_debug_test.cpp index 514e41a8bd..4d53d77caa 100644 --- a/test/val/val_ext_inst_debug_test.cpp +++ b/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()); diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index 19852e6e9c..1d619a238b 100644 --- a/tools/opt/opt.cpp +++ b/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")) { diff --git a/tools/val/val.cpp b/tools/val/val.cpp index 377fd0b1a2..264ae9fab2 100644 --- a/tools/val/val.cpp +++ b/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]) {