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.

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.

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
   }
 }

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

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,

test/val/val_limits_test.cpp

@@ -79,6 +79,70 @@ TEST_F(ValidateLimits, IdEqualToBoundBad) {
       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"(

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;

tools/val/val.cpp

@@ -45,6 +45,7 @@ Options:
   --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