mirror of
https://github.com/RPCSX/SPIRV-Tools.git
synced 2025-03-03 00:46:52 +00:00
OpDecorate should not accept any number of literal operands.
This is a grammar fix. The Decoration operand of OpDecorate (and OpMemberDecorate) determines the remaining operands. Don't just allow any number of literal numbers as operands. (The OperandVariableLiterals operand class as the last member of the OpDecorate and OpMemberDecorate entries in in opcode.inc is an artifact of how the spec generates the opcode descriptions. It's not suitable for parsing those instructions.) Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/34
This commit is contained in:
David Neto
parent
36909c05a5
commit
39fa148234
@ -99,9 +99,18 @@ spv_operand_type_t convertOperandClassToType(SpvOp opcode,
|
||||
break;
|
||||
}
|
||||
} else if (operandClass == OperandVariableLiterals) {
|
||||
if (opcode == SpvOpConstant || opcode == SpvOpSpecConstant) {
|
||||
// The number type is determined by the type Id operand.
|
||||
return SPV_OPERAND_TYPE_TYPED_LITERAL_NUMBER;
|
||||
switch (opcode) {
|
||||
case SpvOpConstant:
|
||||
case SpvOpSpecConstant:
|
||||
// The number type is determined by the type Id operand.
|
||||
return SPV_OPERAND_TYPE_TYPED_LITERAL_NUMBER;
|
||||
case SpvOpDecorate:
|
||||
case SpvOpMemberDecorate:
|
||||
// The operand types at the end of the instruction are
|
||||
// determined instead by the decoration kind.
|
||||
return SPV_OPERAND_TYPE_NONE;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -555,6 +555,42 @@ INSTANTIATE_TEST_CASE_P(
|
||||
{0 /* this word does not belong*/}}),
|
||||
"Invalid instruction OpString starting at word 5: expected no more"
|
||||
" operands after 3 words, but stated word count is 4."},
|
||||
// Word count is too large. There are too many words after the string
|
||||
// literal. A linkage attribute decoration is the only case in SPIR-V
|
||||
// where a string operand is followed by another operand.
|
||||
{Concatenate({ExpectedHeaderForBound(2),
|
||||
{spvOpcodeMake(6, SpvOpDecorate), 1 /* target id */,
|
||||
uint32_t(SpvDecorationLinkageAttributes)},
|
||||
MakeVector("abc"),
|
||||
{uint32_t(SpvLinkageTypeImport),
|
||||
0 /* does not belong */}}),
|
||||
"Invalid instruction OpDecorate starting at word 5: expected no more"
|
||||
" operands after 5 words, but stated word count is 6."},
|
||||
// Same as the previous case, but with OpMemberDecorate.
|
||||
{Concatenate(
|
||||
{ExpectedHeaderForBound(2),
|
||||
{spvOpcodeMake(7, SpvOpMemberDecorate), 1 /* target id */,
|
||||
42 /* member index */, uint32_t(SpvDecorationLinkageAttributes)},
|
||||
MakeVector("abc"),
|
||||
{uint32_t(SpvLinkageTypeImport), 0 /* does not belong */}}),
|
||||
"Invalid instruction OpMemberDecorate starting at word 5: expected no"
|
||||
" more operands after 6 words, but stated word count is 7."},
|
||||
// Word count is too large. There should be no more words
|
||||
// after the RelaxedPrecision decoration.
|
||||
{Concatenate({ExpectedHeaderForBound(2),
|
||||
{spvOpcodeMake(4, SpvOpDecorate), 1 /* target id */,
|
||||
uint32_t(SpvDecorationRelaxedPrecision),
|
||||
0 /* does not belong */}}),
|
||||
"Invalid instruction OpDecorate starting at word 5: expected no"
|
||||
" more operands after 3 words, but stated word count is 4."},
|
||||
// Word count is too large. There should be only one word after
|
||||
// the SpecId decoration enum word.
|
||||
{Concatenate({ExpectedHeaderForBound(2),
|
||||
{spvOpcodeMake(5, SpvOpDecorate), 1 /* target id */,
|
||||
uint32_t(SpvDecorationSpecId), 42 /* the spec id */,
|
||||
0 /* does not belong */}}),
|
||||
"Invalid instruction OpDecorate starting at word 5: expected no"
|
||||
" more operands after 4 words, but stated word count is 5."},
|
||||
{Concatenate({ExpectedHeaderForBound(2),
|
||||
{spvOpcodeMake(2, SpvOpTypeVoid), 0}}),
|
||||
"Error: Result Id is 0"},
|
||||
|
||||
@ -52,10 +52,13 @@ TEST_P(OpDecorateSimpleTest, AnySimpleDecoration) {
|
||||
std::stringstream input;
|
||||
input << "OpDecorate %1 " << GetParam().name();
|
||||
for (auto operand : GetParam().operands()) input << " " << operand;
|
||||
input << std::endl;
|
||||
EXPECT_THAT(
|
||||
CompiledInstructions(input.str()),
|
||||
Eq(MakeInstruction(SpvOpDecorate, {1, uint32_t(GetParam().value())},
|
||||
GetParam().operands())));
|
||||
// Also check disassembly.
|
||||
EXPECT_THAT(EncodeAndDecodeSuccessfully(input.str()), Eq(input.str()));
|
||||
}
|
||||
|
||||
#define CASE(NAME) SpvDecoration##NAME, #NAME
|
||||
@ -111,6 +114,25 @@ TEST_F(OpDecorateSimpleTest, WrongDecoration) {
|
||||
Eq("Invalid decoration 'xxyyzz'."));
|
||||
}
|
||||
|
||||
TEST_F(OpDecorateSimpleTest, ExtraOperandsOnDecorationExpectingNone) {
|
||||
EXPECT_THAT(CompileFailure("OpDecorate %1 RelaxedPrecision 99"),
|
||||
Eq("Expected <opcode> or <result-id> at the beginning of an "
|
||||
"instruction, found '99'."));
|
||||
}
|
||||
|
||||
TEST_F(OpDecorateSimpleTest, ExtraOperandsOnDecorationExpectingOne) {
|
||||
EXPECT_THAT(CompileFailure("OpDecorate %1 SpecId 99 100"),
|
||||
Eq("Expected <opcode> or <result-id> at the beginning of an "
|
||||
"instruction, found '100'."));
|
||||
}
|
||||
|
||||
TEST_F(OpDecorateSimpleTest, ExtraOperandsOnDecorationExpectingTwo) {
|
||||
EXPECT_THAT(
|
||||
CompileFailure("OpDecorate %1 LinkageAttributes \"abc\" Import 42"),
|
||||
Eq("Expected <opcode> or <result-id> at the beginning of an "
|
||||
"instruction, found '42'."));
|
||||
}
|
||||
|
||||
// A single test case for an enum decoration.
|
||||
struct DecorateEnumCase {
|
||||
// Place the enum value first, so it's easier to read the binary dumps when
|
||||
@ -369,7 +391,100 @@ TEST_F(TextToBinaryTest, GroupMemberDecorateInvalidSecondTargetMemberNumber) {
|
||||
Eq("Invalid unsigned integer literal: %id2"));
|
||||
}
|
||||
|
||||
// TODO(dneto): OpMemberDecorate
|
||||
// Test OpMemberDecorate
|
||||
|
||||
using OpMemberDecorateSimpleTest = spvtest::TextToBinaryTestBase<
|
||||
::testing::TestWithParam<EnumCase<SpvDecoration>>>;
|
||||
|
||||
TEST_P(OpMemberDecorateSimpleTest, AnySimpleDecoration) {
|
||||
// This string should assemble, but should not validate.
|
||||
std::stringstream input;
|
||||
input << "OpMemberDecorate %1 42 " << GetParam().name();
|
||||
for (auto operand : GetParam().operands()) input << " " << operand;
|
||||
input << std::endl;
|
||||
EXPECT_THAT(CompiledInstructions(input.str()),
|
||||
Eq(MakeInstruction(SpvOpMemberDecorate,
|
||||
{1, 42, uint32_t(GetParam().value())},
|
||||
GetParam().operands())));
|
||||
// Also check disassembly.
|
||||
EXPECT_THAT(EncodeAndDecodeSuccessfully(input.str()), Eq(input.str()));
|
||||
}
|
||||
|
||||
#define CASE(NAME) SpvDecoration##NAME, #NAME
|
||||
INSTANTIATE_TEST_CASE_P(
|
||||
TextToBinaryDecorateSimple, OpMemberDecorateSimpleTest,
|
||||
::testing::ValuesIn(std::vector<EnumCase<SpvDecoration>>{
|
||||
// The operand literal values are arbitrarily chosen,
|
||||
// but there are the right number of them.
|
||||
{CASE(RelaxedPrecision), {}},
|
||||
{CASE(SpecId), {100}},
|
||||
{CASE(Block), {}},
|
||||
{CASE(BufferBlock), {}},
|
||||
{CASE(RowMajor), {}},
|
||||
{CASE(ColMajor), {}},
|
||||
{CASE(ArrayStride), {4}},
|
||||
{CASE(MatrixStride), {16}},
|
||||
{CASE(GLSLShared), {}},
|
||||
{CASE(GLSLPacked), {}},
|
||||
{CASE(CPacked), {}},
|
||||
// Placeholder line for enum value 12
|
||||
{CASE(NoPerspective), {}},
|
||||
{CASE(Flat), {}},
|
||||
{CASE(Patch), {}},
|
||||
{CASE(Centroid), {}},
|
||||
{CASE(Sample), {}},
|
||||
{CASE(Invariant), {}},
|
||||
{CASE(Restrict), {}},
|
||||
{CASE(Aliased), {}},
|
||||
{CASE(Volatile), {}},
|
||||
{CASE(Constant), {}},
|
||||
{CASE(Coherent), {}},
|
||||
{CASE(NonWritable), {}},
|
||||
{CASE(NonReadable), {}},
|
||||
{CASE(Uniform), {}},
|
||||
{CASE(SaturatedConversion), {}},
|
||||
{CASE(Stream), {2}},
|
||||
{CASE(Location), {6}},
|
||||
{CASE(Component), {3}},
|
||||
{CASE(Index), {14}},
|
||||
{CASE(Binding), {19}},
|
||||
{CASE(DescriptorSet), {7}},
|
||||
{CASE(Offset), {12}},
|
||||
{CASE(XfbBuffer), {1}},
|
||||
{CASE(XfbStride), {8}},
|
||||
{CASE(NoContraction), {}},
|
||||
{CASE(InputAttachmentIndex), {102}},
|
||||
{CASE(Alignment), {16}},
|
||||
}));
|
||||
#undef CASE
|
||||
|
||||
TEST_F(OpMemberDecorateSimpleTest, WrongDecoration) {
|
||||
EXPECT_THAT(CompileFailure("OpMemberDecorate %1 9 xxyyzz"),
|
||||
Eq("Invalid decoration 'xxyyzz'."));
|
||||
}
|
||||
|
||||
TEST_F(OpMemberDecorateSimpleTest, ExtraOperandsOnDecorationExpectingNone) {
|
||||
EXPECT_THAT(CompileFailure("OpMemberDecorate %1 12 RelaxedPrecision 99"),
|
||||
Eq("Expected <opcode> or <result-id> at the beginning of an "
|
||||
"instruction, found '99'."));
|
||||
}
|
||||
|
||||
TEST_F(OpMemberDecorateSimpleTest, ExtraOperandsOnDecorationExpectingOne) {
|
||||
EXPECT_THAT(CompileFailure("OpMemberDecorate %1 0 SpecId 99 100"),
|
||||
Eq("Expected <opcode> or <result-id> at the beginning of an "
|
||||
"instruction, found '100'."));
|
||||
}
|
||||
|
||||
TEST_F(OpMemberDecorateSimpleTest, ExtraOperandsOnDecorationExpectingTwo) {
|
||||
EXPECT_THAT(CompileFailure(
|
||||
"OpMemberDecorate %1 1 LinkageAttributes \"abc\" Import 42"),
|
||||
Eq("Expected <opcode> or <result-id> at the beginning of an "
|
||||
"instruction, found '42'."));
|
||||
}
|
||||
|
||||
// TODO(dneto): OpMemberDecorate cases for decorations with parameters which
|
||||
// are: not just lists of literal numbers.
|
||||
|
||||
// TODO(dneto): OpDecorationGroup
|
||||
// TODO(dneto): OpGroupDecorate
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user