Skip to content

Commit

Permalink
Avoid capability checks on ID values. They are not literals
Browse files Browse the repository at this point in the history
Works around issue 248 by weakening the test:
KhronosGroup#248

The validator should try to track (32-bit) constant values, and then
for capability checks on IDs, check the referenced value, not the
raw ID number.
  • Loading branch information
dneto0 committed Jun 29, 2016
1 parent f760d11 commit e461cbe
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 0 deletions.
5 changes: 5 additions & 0 deletions source/validate_instruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

#include "diagnostic.h"
#include "opcode.h"
#include "operand.h"
#include "spirv_definition.h"
#include "val/Function.h"
#include "val/ValidationState.h"
Expand Down Expand Up @@ -109,6 +110,10 @@ spv_result_t CapCheck(ValidationState_t& _,
}
}
}
} else if (spvIsIdType(operand.type)) {
// TODO(dneto): Check the value referenced by this Id, if we can compute
// it. For now, just punt, to fix issue 248:
// https://github.com/KhronosGroup/SPIRV-Tools/issues/248
} else {
// Check the operand word as a whole.
const auto caps = RequiredCapabilities(_.grammar(), operand.type, word);
Expand Down
35 changes: 35 additions & 0 deletions test/Validate.Capability.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1153,4 +1153,39 @@ TEST_P(ValidateCapabilityV11, Capability) {
<< test_code;
}

TEST_F(ValidateCapability, SemanticsIdIsAnIdNotALiteral) {
// From https://github.com/KhronosGroup/SPIRV-Tools/issues/248
// The validator was interpreting the memory semantics ID number
// as the value to be checked rather than an ID that references
// another value to be checked.
// In this case a raw ID of 64 was mistaken to mean a literal
// semantic value of UniformMemory, which would require the Shader
// capability.
const char str[] = R"(
OpCapability Kernel
OpMemoryModel Logical OpenCL
; %i32 has ID 1
%i32 = OpTypeInt 32 1
%tf = OpTypeFunction %i32
%pi32 = OpTypePointer CrossWorkgroup %i32
%var = OpVariable %pi32 CrossWorkgroup
%c = OpConstant %i32 100
%scope = OpConstant %i32 1 ; Device scope
; Fake an instruction with 64 as the result id.
; !64 = OpConstantNull %i32
!0x3002e !1 !64
%f = OpFunction %i32 None %tf
%l = OpLabel
%result = OpAtomicIAdd %i32 %var %scope !64 %c
OpReturnValue %result
OpFunctionEnd
)";

CompileSuccessfully(str);
ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
}

} // namespace anonymous

0 comments on commit e461cbe

Please sign in to comment.