AuroraMiddleware/SPIRV-Tools
1
0
Fork 0
mirror of https://github.com/KhronosGroup/SPIRV-Tools synced 2024-10-18 19:20:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix major bug in validate_builtins

Browse Source 
Fixed an early return in the loop, resulting in only one decoration
being checked.
This commit is contained in:
Andrey Tuganov 2018-04-03 16:07:55 -04:00 committed by David Neto
parent da332cf332
commit 691eed92cb
3 changed files with 218 additions and 130 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
source/val/validation_state.h
View File

@ -306,8 +306,7 @@ class ValidationState_t {
}
// Returns const pointer to the internal decoration container.
const std::unordered_map<uint32_t, std::vector<Decoration>>& id_decorations()
const {
const std::map<uint32_t, std::vector<Decoration>>& id_decorations() const {
return id_decorations_;
}
@ -528,7 +527,7 @@ class ValidationState_t {
std::unordered_map<uint32_t, uint32_t> struct_nesting_depth_;
/// Stores the list of decorations for a given <id>
std::unordered_map<uint32_t, std::vector<Decoration>> id_decorations_;
std::map<uint32_t, std::vector<Decoration>> id_decorations_;
/// Stores type declarations which need to be unique (i.e. non-aggregates),
/// in the form [opcode, operand words], result_id is not stored.

269
source/validate_builtins.cpp
View File

@ -112,10 +112,15 @@ class BuiltInsValidator {
private:
// Goes through all decorations in the module, if decoration is BuiltIn
// validates the instruction defining the decorated id. Also seeds
// id_to_at_reference_checks_ with decorated ids.
// calls ValidateSingleBuiltInAtDefinition().
spv_result_t ValidateBuiltInsAtDefinition();
// Validates the instruction defining an id with built-in decoration.
// Can be called multiple times for the same id, if multiple built-ins are
// specified. Seeds id_to_at_reference_checks_ with decorated ids if needed.
spv_result_t ValidateSingleBuiltInAtDefinition(const Decoration& decoration,
const Instruction& inst);
// The following section contains functions which are called when id defined
// by |inst| is decorated with BuiltIn |decoration|.
// Most functions are specific to a single built-in and have naming scheme:
@ -2245,139 +2250,149 @@ spv_result_t BuiltInsValidator::ValidateWorkgroupSizeAtReference(
return SPV_SUCCESS;
}
spv_result_t BuiltInsValidator::ValidateSingleBuiltInAtDefinition(
const Decoration& decoration, const Instruction& inst) {
const SpvBuiltIn label = SpvBuiltIn(decoration.params()[0]);
// If you are adding a new BuiltIn enum, please register it here.
// If the newly added enum has validation rules associated with it
// consider leaving a TODO and/or creating an issue.
switch (label) {
case SpvBuiltInClipDistance:
case SpvBuiltInCullDistance: {
return ValidateClipOrCullDistanceAtDefinition(decoration, inst);
}
case SpvBuiltInFragCoord: {
return ValidateFragCoordAtDefinition(decoration, inst);
}
case SpvBuiltInFragDepth: {
return ValidateFragDepthAtDefinition(decoration, inst);
}
case SpvBuiltInFrontFacing: {
return ValidateFrontFacingAtDefinition(decoration, inst);
}
case SpvBuiltInGlobalInvocationId:
case SpvBuiltInLocalInvocationId:
case SpvBuiltInNumWorkgroups:
case SpvBuiltInWorkgroupId: {
return ValidateComputeShaderI32Vec3InputAtDefinition(decoration, inst);
}
case SpvBuiltInHelperInvocation: {
return ValidateHelperInvocationAtDefinition(decoration, inst);
}
case SpvBuiltInInvocationId: {
return ValidateInvocationIdAtDefinition(decoration, inst);
}
case SpvBuiltInInstanceIndex: {
return ValidateInstanceIndexAtDefinition(decoration, inst);
}
case SpvBuiltInLayer:
case SpvBuiltInViewportIndex: {
return ValidateLayerOrViewportIndexAtDefinition(decoration, inst);
}
case SpvBuiltInPatchVertices: {
return ValidatePatchVerticesAtDefinition(decoration, inst);
}
case SpvBuiltInPointCoord: {
return ValidatePointCoordAtDefinition(decoration, inst);
}
case SpvBuiltInPointSize: {
return ValidatePointSizeAtDefinition(decoration, inst);
}
case SpvBuiltInPosition: {
return ValidatePositionAtDefinition(decoration, inst);
}
case SpvBuiltInPrimitiveId: {
return ValidatePrimitiveIdAtDefinition(decoration, inst);
}
case SpvBuiltInSampleId: {
return ValidateSampleIdAtDefinition(decoration, inst);
}
case SpvBuiltInSampleMask: {
return ValidateSampleMaskAtDefinition(decoration, inst);
}
case SpvBuiltInSamplePosition: {
return ValidateSamplePositionAtDefinition(decoration, inst);
}
case SpvBuiltInTessCoord: {
return ValidateTessCoordAtDefinition(decoration, inst);
}
case SpvBuiltInTessLevelOuter: {
return ValidateTessLevelOuterAtDefinition(decoration, inst);
}
case SpvBuiltInTessLevelInner: {
return ValidateTessLevelInnerAtDefinition(decoration, inst);
}
case SpvBuiltInVertexIndex: {
return ValidateVertexIndexAtDefinition(decoration, inst);
}
case SpvBuiltInWorkgroupSize: {
return ValidateWorkgroupSizeAtDefinition(decoration, inst);
}
case SpvBuiltInVertexId:
case SpvBuiltInInstanceId:
case SpvBuiltInLocalInvocationIndex:
case SpvBuiltInWorkDim:
case SpvBuiltInGlobalSize:
case SpvBuiltInEnqueuedWorkgroupSize:
case SpvBuiltInGlobalOffset:
case SpvBuiltInGlobalLinearId:
case SpvBuiltInSubgroupSize:
case SpvBuiltInSubgroupMaxSize:
case SpvBuiltInNumSubgroups:
case SpvBuiltInNumEnqueuedSubgroups:
case SpvBuiltInSubgroupId:
case SpvBuiltInSubgroupLocalInvocationId:
case SpvBuiltInSubgroupEqMaskKHR:
case SpvBuiltInSubgroupGeMaskKHR:
case SpvBuiltInSubgroupGtMaskKHR:
case SpvBuiltInSubgroupLeMaskKHR:
case SpvBuiltInSubgroupLtMaskKHR:
case SpvBuiltInBaseVertex:
case SpvBuiltInBaseInstance:
case SpvBuiltInDrawIndex:
case SpvBuiltInDeviceIndex:
case SpvBuiltInViewIndex:
case SpvBuiltInBaryCoordNoPerspAMD:
case SpvBuiltInBaryCoordNoPerspCentroidAMD:
case SpvBuiltInBaryCoordNoPerspSampleAMD:
case SpvBuiltInBaryCoordSmoothAMD:
case SpvBuiltInBaryCoordSmoothCentroidAMD:
case SpvBuiltInBaryCoordSmoothSampleAMD:
case SpvBuiltInBaryCoordPullModelAMD:
case SpvBuiltInFragStencilRefEXT:
case SpvBuiltInViewportMaskNV:
case SpvBuiltInSecondaryPositionNV:
case SpvBuiltInSecondaryViewportMaskNV:
case SpvBuiltInPositionPerViewNV:
case SpvBuiltInViewportMaskPerViewNV:
case SpvBuiltInFullyCoveredEXT:
case SpvBuiltInMax: {
// No validation rules (for the moment).
break;
}
}
return SPV_SUCCESS;
}
spv_result_t BuiltInsValidator::ValidateBuiltInsAtDefinition() {
for (const auto& kv : _.id_decorations()) {
const uint32_t id = kv.first;
const Instruction* inst = nullptr;
const auto& decorations = kv.second;
if (decorations.empty()) {
continue;
}
const Instruction* inst = _.FindDef(id);
assert(inst);
for (const auto& decoration : kv.second) {
if (decoration.dec_type() != SpvDecorationBuiltIn) {
continue;
}
if (!inst) {
inst = _.FindDef(id);
assert(inst);
}
const SpvBuiltIn label = SpvBuiltIn(decoration.params()[0]);
// If you are adding a new BuiltIn enum, please register it here.
// If the newly added enum has validation rules associated with it
// consider leaving a TODO and/or creating an issue.
switch (label) {
case SpvBuiltInClipDistance:
case SpvBuiltInCullDistance: {
return ValidateClipOrCullDistanceAtDefinition(decoration, *inst);
}
case SpvBuiltInFragCoord: {
return ValidateFragCoordAtDefinition(decoration, *inst);
}
case SpvBuiltInFragDepth: {
return ValidateFragDepthAtDefinition(decoration, *inst);
}
case SpvBuiltInFrontFacing: {
return ValidateFrontFacingAtDefinition(decoration, *inst);
}
case SpvBuiltInGlobalInvocationId:
case SpvBuiltInLocalInvocationId:
case SpvBuiltInNumWorkgroups:
case SpvBuiltInWorkgroupId: {
return ValidateComputeShaderI32Vec3InputAtDefinition(decoration,
*inst);
}
case SpvBuiltInHelperInvocation: {
return ValidateHelperInvocationAtDefinition(decoration, *inst);
}
case SpvBuiltInInvocationId: {
return ValidateInvocationIdAtDefinition(decoration, *inst);
}
case SpvBuiltInInstanceIndex: {
return ValidateInstanceIndexAtDefinition(decoration, *inst);
}
case SpvBuiltInLayer:
case SpvBuiltInViewportIndex: {
return ValidateLayerOrViewportIndexAtDefinition(decoration, *inst);
}
case SpvBuiltInPatchVertices: {
return ValidatePatchVerticesAtDefinition(decoration, *inst);
}
case SpvBuiltInPointCoord: {
return ValidatePointCoordAtDefinition(decoration, *inst);
}
case SpvBuiltInPointSize: {
return ValidatePointSizeAtDefinition(decoration, *inst);
}
case SpvBuiltInPosition: {
return ValidatePositionAtDefinition(decoration, *inst);
}
case SpvBuiltInPrimitiveId: {
return ValidatePrimitiveIdAtDefinition(decoration, *inst);
}
case SpvBuiltInSampleId: {
return ValidateSampleIdAtDefinition(decoration, *inst);
}
case SpvBuiltInSampleMask: {
return ValidateSampleMaskAtDefinition(decoration, *inst);
}
case SpvBuiltInSamplePosition: {
return ValidateSamplePositionAtDefinition(decoration, *inst);
}
case SpvBuiltInTessCoord: {
return ValidateTessCoordAtDefinition(decoration, *inst);
}
case SpvBuiltInTessLevelOuter: {
return ValidateTessLevelOuterAtDefinition(decoration, *inst);
}
case SpvBuiltInTessLevelInner: {
return ValidateTessLevelInnerAtDefinition(decoration, *inst);
}
case SpvBuiltInVertexIndex: {
return ValidateVertexIndexAtDefinition(decoration, *inst);
}
case SpvBuiltInWorkgroupSize: {
return ValidateWorkgroupSizeAtDefinition(decoration, *inst);
}
case SpvBuiltInVertexId:
case SpvBuiltInInstanceId:
case SpvBuiltInLocalInvocationIndex:
case SpvBuiltInWorkDim:
case SpvBuiltInGlobalSize:
case SpvBuiltInEnqueuedWorkgroupSize:
case SpvBuiltInGlobalOffset:
case SpvBuiltInGlobalLinearId:
case SpvBuiltInSubgroupSize:
case SpvBuiltInSubgroupMaxSize:
case SpvBuiltInNumSubgroups:
case SpvBuiltInNumEnqueuedSubgroups:
case SpvBuiltInSubgroupId:
case SpvBuiltInSubgroupLocalInvocationId:
case SpvBuiltInSubgroupEqMaskKHR:
case SpvBuiltInSubgroupGeMaskKHR:
case SpvBuiltInSubgroupGtMaskKHR:
case SpvBuiltInSubgroupLeMaskKHR:
case SpvBuiltInSubgroupLtMaskKHR:
case SpvBuiltInBaseVertex:
case SpvBuiltInBaseInstance:
case SpvBuiltInDrawIndex:
case SpvBuiltInDeviceIndex:
case SpvBuiltInViewIndex:
case SpvBuiltInBaryCoordNoPerspAMD:
case SpvBuiltInBaryCoordNoPerspCentroidAMD:
case SpvBuiltInBaryCoordNoPerspSampleAMD:
case SpvBuiltInBaryCoordSmoothAMD:
case SpvBuiltInBaryCoordSmoothCentroidAMD:
case SpvBuiltInBaryCoordSmoothSampleAMD:
case SpvBuiltInBaryCoordPullModelAMD:
case SpvBuiltInFragStencilRefEXT:
case SpvBuiltInViewportMaskNV:
case SpvBuiltInSecondaryPositionNV:
case SpvBuiltInSecondaryViewportMaskNV:
case SpvBuiltInPositionPerViewNV:
case SpvBuiltInViewportMaskPerViewNV:
case SpvBuiltInFullyCoveredEXT:
case SpvBuiltInMax: {
// No validation rules (for the moment).
break;
}
if (spv_result_t error =
ValidateSingleBuiltInAtDefinition(decoration, *inst)) {
return error;
}
}
}

74
test/val/val_builtins_test.cpp
View File

@ -1638,6 +1638,80 @@ OpStore %output_pos %pos
ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_0));
}
TEST_F(ValidateBuiltIns, TwoBuiltInsFirstFails) {
CodeGenerator generator = GetDefaultShaderCodeGenerator();
generator.before_types_ = R"(
OpMemberDecorate %input_type 0 BuiltIn FragCoord
OpMemberDecorate %output_type 0 BuiltIn Position
)";
generator.after_types_ = R"(
%input_type = OpTypeStruct %f32vec4
%input_ptr = OpTypePointer Input %input_type
%input = OpVariable %input_ptr Input
%input_f32vec4_ptr = OpTypePointer Input %f32vec4
%output_type = OpTypeStruct %f32vec4
%output_ptr = OpTypePointer Output %output_type
%output = OpVariable %output_ptr Output
%output_f32vec4_ptr = OpTypePointer Output %f32vec4
)";
EntryPoint entry_point;
entry_point.name = "main";
entry_point.execution_model = "Geometry";
entry_point.body = R"(
%input_pos = OpAccessChain %input_f32vec4_ptr %input %u32_0
%output_pos = OpAccessChain %output_f32vec4_ptr %output %u32_0
%pos = OpLoad %f32vec4 %input_pos
OpStore %output_pos %pos
)";
generator.entry_points_.push_back(std::move(entry_point));
CompileSuccessfully(generator.Build(), SPV_ENV_VULKAN_1_0);
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_0));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Vulkan spec allows BuiltIn FragCoord to be used only "
"with Fragment execution model"));
}
TEST_F(ValidateBuiltIns, TwoBuiltInsSecondFails) {
CodeGenerator generator = GetDefaultShaderCodeGenerator();
generator.before_types_ = R"(
OpMemberDecorate %input_type 0 BuiltIn Position
OpMemberDecorate %output_type 0 BuiltIn FragCoord
)";
generator.after_types_ = R"(
%input_type = OpTypeStruct %f32vec4
%input_ptr = OpTypePointer Input %input_type
%input = OpVariable %input_ptr Input
%input_f32vec4_ptr = OpTypePointer Input %f32vec4
%output_type = OpTypeStruct %f32vec4
%output_ptr = OpTypePointer Output %output_type
%output = OpVariable %output_ptr Output
%output_f32vec4_ptr = OpTypePointer Output %f32vec4
)";
EntryPoint entry_point;
entry_point.name = "main";
entry_point.execution_model = "Geometry";
entry_point.body = R"(
%input_pos = OpAccessChain %input_f32vec4_ptr %input %u32_0
%output_pos = OpAccessChain %output_f32vec4_ptr %output %u32_0
%pos = OpLoad %f32vec4 %input_pos
OpStore %output_pos %pos
)";
generator.entry_points_.push_back(std::move(entry_point));
CompileSuccessfully(generator.Build(), SPV_ENV_VULKAN_1_0);
ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_0));
EXPECT_THAT(getDiagnosticString(),
HasSubstr("Vulkan spec allows BuiltIn FragCoord to be only used "
"for variables with Input storage class"));
}
TEST_F(ValidateBuiltIns, VertexPositionVariableSuccess) {
CodeGenerator generator = GetDefaultShaderCodeGenerator();
generator.before_types_ = R"(