diff --git a/source/assembly_grammar.cpp b/source/assembly_grammar.cpp index 79f18eee3..4f5942ab4 100644 --- a/source/assembly_grammar.cpp +++ b/source/assembly_grammar.cpp @@ -62,9 +62,9 @@ spv_result_t spvTextParseMaskOperand(spv_target_env env, end = std::find(begin, text_end, separator); spv_operand_desc entry = nullptr; - if (spvOperandTableNameLookup(env, operandTable, type, begin, end - begin, - &entry)) { - return SPV_ERROR_INVALID_TEXT; + if (auto error = spvOperandTableNameLookup(env, operandTable, type, begin, + end - begin, &entry)) { + return error; } value |= entry->value; diff --git a/source/operand.cpp b/source/operand.cpp index 6d83e81e2..0c255a352 100644 --- a/source/operand.cpp +++ b/source/operand.cpp @@ -72,12 +72,16 @@ spv_result_t spvOperandTableNameLookup(spv_target_env env, // Note that the second rule assumes the extension enabling this operand // is indeed requested in the SPIR-V code; checking that should be // validator's work. - if (((version >= entry.minVersion && version <= entry.lastVersion) || - entry.numExtensions > 0u || entry.numCapabilities > 0u) && - nameLength == strlen(entry.name) && + if (nameLength == strlen(entry.name) && !strncmp(entry.name, name, nameLength)) { - *pEntry = &entry; - return SPV_SUCCESS; + if ((version >= entry.minVersion && version <= entry.lastVersion) || + entry.numExtensions > 0u || entry.numCapabilities > 0u) { + *pEntry = &entry; + return SPV_SUCCESS; + } else { + // if there is no extension/capability then the version is wrong + return SPV_ERROR_WRONG_VERSION; + } } } } diff --git a/source/text.cpp b/source/text.cpp index 415c059d4..97d00cbe1 100644 --- a/source/text.cpp +++ b/source/text.cpp @@ -403,9 +403,10 @@ spv_result_t spvTextEncodeOperand(const spvtools::AssemblyGrammar& grammar, case SPV_OPERAND_TYPE_DEBUG_INFO_FLAGS: case SPV_OPERAND_TYPE_CLDEBUG100_DEBUG_INFO_FLAGS: { uint32_t value; - if (grammar.parseMaskOperand(type, textValue, &value)) { - return context->diagnostic() << "Invalid " << spvOperandTypeStr(type) - << " operand '" << textValue << "'."; + if (auto error = grammar.parseMaskOperand(type, textValue, &value)) { + return context->diagnostic(error) + << "Invalid " << spvOperandTypeStr(type) << " operand '" + << textValue << "'."; } if (auto error = context->binaryEncodeU32(value, pInst)) return error; // Prepare to parse the operands for this logical operand. @@ -769,8 +770,8 @@ spv_result_t spvTextToBinaryInternal(const spvtools::AssemblyGrammar& grammar, instructions.push_back({}); spv_instruction_t& inst = instructions.back(); - if (spvTextEncodeOpcode(grammar, &context, &inst)) { - return SPV_ERROR_INVALID_TEXT; + if (auto error = spvTextEncodeOpcode(grammar, &context, &inst)) { + return error; } if (context.advance()) break; diff --git a/test/val/val_fixtures.h b/test/val/val_fixtures.h index acbe0e577..98d8d32a9 100644 --- a/test/val/val_fixtures.h +++ b/test/val/val_fixtures.h @@ -40,8 +40,10 @@ class ValidateBase : public ::testing::Test, // Assembles the given SPIR-V text, checks that it fails to assemble, // and returns resulting diagnostic. No internal state is updated. + // Setting the desired_result to SPV_SUCCESS is used to allow all results std::string CompileFailure(std::string code, - spv_target_env env = SPV_ENV_UNIVERSAL_1_0); + spv_target_env env = SPV_ENV_UNIVERSAL_1_0, + spv_result_t desired_result = SPV_SUCCESS); // Checks that 'code' is valid SPIR-V text representation and stores the // binary version for further method calls. @@ -108,11 +110,17 @@ void ValidateBase::TearDown() { template std::string ValidateBase::CompileFailure(std::string code, - spv_target_env env) { + spv_target_env env, + spv_result_t desired_result) { spv_diagnostic diagnostic = nullptr; - EXPECT_NE(SPV_SUCCESS, - spvTextToBinary(ScopedContext(env).context, code.c_str(), - code.size(), &binary_, &diagnostic)); + spv_result_t actual_result = + spvTextToBinary(ScopedContext(env).context, code.c_str(), code.size(), + &binary_, &diagnostic); + EXPECT_NE(SPV_SUCCESS, actual_result); + // optional check for exact result + if (desired_result != SPV_SUCCESS) { + EXPECT_EQ(actual_result, desired_result); + } std::string result(diagnostic->error); spvDiagnosticDestroy(diagnostic); return result; diff --git a/test/val/val_image_test.cpp b/test/val/val_image_test.cpp index 87bec2239..d8f6b4498 100644 --- a/test/val/val_image_test.cpp +++ b/test/val/val_image_test.cpp @@ -5540,7 +5540,8 @@ TEST_F(ValidateImage, SignExtendV13Bad) { )"; EXPECT_THAT(CompileFailure(GenerateShaderCode(body, "", "Fragment", "", - SPV_ENV_UNIVERSAL_1_3)), + SPV_ENV_UNIVERSAL_1_3), + SPV_ENV_UNIVERSAL_1_3, SPV_ERROR_WRONG_VERSION), HasSubstr("Invalid image operand 'SignExtend'")); } @@ -5551,7 +5552,8 @@ TEST_F(ValidateImage, ZeroExtendV13Bad) { )"; EXPECT_THAT(CompileFailure(GenerateShaderCode(body, "", "Fragment", "", - SPV_ENV_UNIVERSAL_1_3)), + SPV_ENV_UNIVERSAL_1_3), + SPV_ENV_UNIVERSAL_1_3, SPV_ERROR_WRONG_VERSION), HasSubstr("Invalid image operand 'ZeroExtend'")); }