diff --git a/include/spirv-tools/libspirv.h b/include/spirv-tools/libspirv.h index e202f6fff..050fecc86 100644 --- a/include/spirv-tools/libspirv.h +++ b/include/spirv-tools/libspirv.h @@ -435,6 +435,7 @@ typedef enum { 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 0c0625364..6d9afab7e 100644 --- a/source/spirv_validator_options.cpp +++ b/source/spirv_validator_options.cpp @@ -37,6 +37,8 @@ bool spvParseUniversalLimitsOptions(const char* s, spv_validator_limit* type) { *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 @@ void spvValidatorOptionsSetUniversalLimit(spv_validator_options options, 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 d264a7e0b..ba1c8daeb 100644 --- a/source/spirv_validator_options.h +++ b/source/spirv_validator_options.h @@ -32,6 +32,7 @@ struct validator_universal_limits_t { 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 98088c512..22367909c 100644 --- a/source/val/validate.cpp +++ b/source/val/validate.cpp @@ -226,6 +226,12 @@ spv_result_t ValidateBinaryUsingContextAndValidationState( << 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 55bf1e5f1..61ab15c88 100644 --- a/test/val/val_limits_test.cpp +++ b/test/val/val_limits_test.cpp @@ -79,6 +79,70 @@ TEST_F(ValidateLimits, IdEqualToBoundBad) { HasSubstr("Result '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::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 e024a1b69..119bad9c5 100644 --- a/tools/opt/opt.cpp +++ b/tools/opt/opt.cpp @@ -557,6 +557,8 @@ OptStatus ParseFlags(int argc, const char** argv, 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 @@ int main(int argc, const char** argv) { 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 172dd121d..2f4f626ea 100644 --- a/tools/val/val.cpp +++ b/tools/val/val.cpp @@ -45,6 +45,7 @@ Options: --max-function-args --max-control-flow-nesting-depth --max-access-chain-indexes + --max-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