From 04b4a204aa501992ae85a207be1e200e195c98e1 Mon Sep 17 00:00:00 2001 From: alan-baker Date: Thu, 30 Jan 2025 15:46:00 -0500 Subject: [PATCH] Improve the instruction diagnostic for some access chain errors (#5978) Fixes #3263 * The checks relating to structure indexing used the index disassembly instead of the access chain making it difficult to determine where the actual error occurred --- source/val/validate_memory.cpp | 15 ++++++++------- test/val/val_id_test.cpp | 9 ++++----- test/val/val_memory_test.cpp | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/source/val/validate_memory.cpp b/source/val/validate_memory.cpp index 440320fd..ba45fa84 100644 --- a/source/val/validate_memory.cpp +++ b/source/val/validate_memory.cpp @@ -1581,9 +1581,10 @@ spv_result_t ValidateAccessChain(ValidationState_t& _, // index: the index must be an OpConstant. int64_t cur_index; if (!_.EvalConstantValInt64(cur_word, &cur_index)) { - return _.diag(SPV_ERROR_INVALID_ID, cur_word_instr) - << "The passed to " << instr_name - << " to index into a " + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "The passed to " << instr_name << " to index " + << _.getIdName(cur_word) + << " into a " "structure must be an OpConstant."; } @@ -1592,10 +1593,10 @@ spv_result_t ValidateAccessChain(ValidationState_t& _, const int64_t num_struct_members = static_cast(type_pointee->words().size() - 2); if (cur_index >= num_struct_members || cur_index < 0) { - return _.diag(SPV_ERROR_INVALID_ID, cur_word_instr) - << "Index is out of bounds: " << instr_name - << " cannot find index " << cur_index - << " into the structure " + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "Index " << _.getIdName(cur_word) + << " is out of bounds: " << instr_name << " cannot find index " + << cur_index << " into the structure " << _.getIdName(type_pointee->id()) << ". This structure has " << num_struct_members << " members. Largest valid index is " << num_struct_members - 1 << "."; diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp index 63de09f1..b4c9d78a 100644 --- a/test/val/val_id_test.cpp +++ b/test/val/val_id_test.cpp @@ -4320,12 +4320,11 @@ TEST_P(AccessChainInstructionTest, AccessChainStructIndexNotConstantBad) { OpReturn OpFunctionEnd )"; - const std::string expected_err = - "The passed to " + instr + - " to index into a structure must be an OpConstant."; CompileSuccessfully(spirv); EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); - EXPECT_THAT(getDiagnosticString(), HasSubstr(expected_err)); + EXPECT_THAT(getDiagnosticString(), HasSubstr("The passed to " + instr)); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("into a structure must be an OpConstant")); } // Invalid: Indexing up to a vec4 granularity, but result type expected float. @@ -4379,7 +4378,7 @@ TEST_P(AccessChainInstructionTest, AccessChainStructIndexOutOfBoundBad) { OpReturn OpFunctionEnd )"; - const std::string expected_err = "Index is out of bounds: " + instr + + const std::string expected_err = "is out of bounds: " + instr + " cannot find index 3 into the structure " " '25[%_struct_25]'. This structure " "has 3 members. Largest valid index is 2."; diff --git a/test/val/val_memory_test.cpp b/test/val/val_memory_test.cpp index e4cd470e..8855b0ef 100644 --- a/test/val/val_memory_test.cpp +++ b/test/val/val_memory_test.cpp @@ -5673,7 +5673,7 @@ OpFunctionEnd CompileSuccessfully(spirv); EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); - EXPECT_THAT(getDiagnosticString(), HasSubstr("Index is out of bounds")); + EXPECT_THAT(getDiagnosticString(), HasSubstr("is out of bounds")); EXPECT_THAT(getDiagnosticString(), HasSubstr("cannot find index -224")); } @@ -5701,7 +5701,7 @@ OpFunctionEnd CompileSuccessfully(spirv); EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); - EXPECT_THAT(getDiagnosticString(), HasSubstr("Index is out of bounds")); + EXPECT_THAT(getDiagnosticString(), HasSubstr("is out of bounds")); EXPECT_THAT(getDiagnosticString(), HasSubstr("cannot find index -224")); } -- 2.11.4.GIT