Validate the id bound. (#2031) * Validate the id bound. Validates that the id bound for the module is not larger than the max id bound. Also adds an option to set the max id bound. Allows the optimizer option to set the max id bound to also set the id bound for the validation run done by the optimizer. Fixes #2030.
diff --git a/include/spirv-tools/libspirv.h b/include/spirv-tools/libspirv.h index e202f6f..050fecc 100644 --- a/include/spirv-tools/libspirv.h +++ b/include/spirv-tools/libspirv.h
@@ -435,6 +435,7 @@ spv_validator_limit_max_function_args, spv_validator_limit_max_control_flow_nesting_depth, spv_validator_limit_max_access_chain_indexes, + spv_validator_limit_max_id_bound, } spv_validator_limit; // Returns a string describing the given SPIR-V target environment.
diff --git a/source/spirv_validator_options.cpp b/source/spirv_validator_options.cpp index 0c06253..6d9afab 100644 --- a/source/spirv_validator_options.cpp +++ b/source/spirv_validator_options.cpp
@@ -37,6 +37,8 @@ *type = spv_validator_limit_max_control_flow_nesting_depth; } else if (match("--max-access-chain-indexes")) { *type = spv_validator_limit_max_access_chain_indexes; + } else if (match("--max-id-bound")) { + *type = spv_validator_limit_max_id_bound; } else { // The command line option for this validator limit has not been added. // Therefore we return false. @@ -73,6 +75,7 @@ max_control_flow_nesting_depth) LIMIT(spv_validator_limit_max_access_chain_indexes, max_access_chain_indexes) + LIMIT(spv_validator_limit_max_id_bound, max_id_bound) #undef LIMIT } }
diff --git a/source/spirv_validator_options.h b/source/spirv_validator_options.h index d264a7e..ba1c8da 100644 --- a/source/spirv_validator_options.h +++ b/source/spirv_validator_options.h
@@ -32,6 +32,7 @@ uint32_t max_function_args{255}; uint32_t max_control_flow_nesting_depth{1023}; uint32_t max_access_chain_indexes{255}; + uint32_t max_id_bound{0x3FFFFF}; }; // Manages command line options passed to the SPIR-V Validator. New struct
diff --git a/source/val/validate.cpp b/source/val/validate.cpp index 98088c5..2236790 100644 --- a/source/val/validate.cpp +++ b/source/val/validate.cpp
@@ -226,6 +226,12 @@ << spvTargetEnvDescription(context.target_env) << "."; } + if (header.bound > vstate->options()->universal_limits_.max_id_bound) { + return DiagnosticStream(position, context.consumer, "", + SPV_ERROR_INVALID_BINARY) + << "Invalid SPIR-V. The id bound is larger than the max id bound " + << vstate->options()->universal_limits_.max_id_bound << "."; + } // Look for OpExtension instructions and register extensions. spvBinaryParse(&context, vstate, words, num_words, /* parsed_header = */ nullptr, ProcessExtensions,
diff --git a/test/val/val_limits_test.cpp b/test/val/val_limits_test.cpp index 55bf1e5..61ab15c 100644 --- a/test/val/val_limits_test.cpp +++ b/test/val/val_limits_test.cpp
@@ -79,6 +79,70 @@ HasSubstr("Result <id> '64' must be less than the ID bound '64'.")); } +TEST_F(ValidateLimits, IdBoundTooBigDeaultLimit) { + std::string str = header; + + CompileSuccessfully(str); + + // The largest ID used in this program is 64. Let's overwrite the ID bound in + // the header to be 64. This should result in an error because all IDs must + // satisfy: 0 < id < bound. + OverwriteAssembledBinary(3, 0x4FFFFF); + + ASSERT_EQ(SPV_ERROR_INVALID_BINARY, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("Invalid SPIR-V. The id bound is larger than the max " + "id bound 4194303.")); +} + +TEST_F(ValidateLimits, IdBoundAtSetLimit) { + std::string str = header; + + CompileSuccessfully(str); + + // The largest ID used in this program is 64. Let's overwrite the ID bound in + // the header to be 64. This should result in an error because all IDs must + // satisfy: 0 < id < bound. + uint32_t id_bound = 0x4FFFFF; + + OverwriteAssembledBinary(3, id_bound); + getValidatorOptions()->universal_limits_.max_id_bound = id_bound; + + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + +TEST_F(ValidateLimits, IdBoundJustAboveSetLimit) { + std::string str = header; + + CompileSuccessfully(str); + + // The largest ID used in this program is 64. Let's overwrite the ID bound in + // the header to be 64. This should result in an error because all IDs must + // satisfy: 0 < id < bound. + uint32_t id_bound = 5242878; + + OverwriteAssembledBinary(3, id_bound); + getValidatorOptions()->universal_limits_.max_id_bound = id_bound - 1; + + ASSERT_EQ(SPV_ERROR_INVALID_BINARY, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("Invalid SPIR-V. The id bound is larger than the max " + "id bound 5242877.")); +} + +TEST_F(ValidateLimits, IdBoundAtInMaxLimit) { + std::string str = header; + + CompileSuccessfully(str); + + uint32_t id_bound = std::numeric_limits<uint32_t>::max(); + + OverwriteAssembledBinary(3, id_bound); + getValidatorOptions()->universal_limits_.max_id_bound = id_bound; + + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions()); +} + TEST_F(ValidateLimits, StructNumMembersGood) { std::ostringstream spirv; spirv << header << R"(
diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index e024a1b..119bad9 100644 --- a/tools/opt/opt.cpp +++ b/tools/opt/opt.cpp
@@ -557,6 +557,8 @@ return {OPT_STOP, 1}; } optimizer_options->set_max_id_bound(max_id_bound); + validator_options->SetUniversalLimit(spv_validator_limit_max_id_bound, + max_id_bound); } else { // Some passes used to accept the form '--pass arg', canonicalize them // to '--pass=arg'. @@ -593,16 +595,16 @@ const char* out_file = nullptr; spv_target_env target_env = kDefaultEnvironment; - spvtools::ValidatorOptions validator_options; - spvtools::OptimizerOptions optimizer_options; - optimizer_options.set_validator_options(validator_options); spvtools::Optimizer optimizer(target_env); optimizer.SetMessageConsumer(spvtools::utils::CLIMessageConsumer); + spvtools::ValidatorOptions validator_options; + spvtools::OptimizerOptions optimizer_options; OptStatus status = ParseFlags(argc, argv, &optimizer, &in_file, &out_file, &validator_options, &optimizer_options); + optimizer_options.set_validator_options(validator_options); if (status.action == OPT_STOP) { return status.code;
diff --git a/tools/val/val.cpp b/tools/val/val.cpp index 172dd12..2f4f626 100644 --- a/tools/val/val.cpp +++ b/tools/val/val.cpp
@@ -45,6 +45,7 @@ --max-function-args <maximum number arguments allowed per function> --max-control-flow-nesting-depth <maximum Control Flow nesting depth allowed> --max-access-chain-indexes <maximum number of indexes allowed to use for Access Chain instructions> + --max-id-bound <maximum value for the id bound> --relax-logical-pointer Allow allocating an object of a pointer type and returning a pointer value from a function in logical addressing mode --relax-block-layout Enable VK_HR_relaxed_block_layout when checking standard