From c0949703b1264c33df45584efba50a8444b53022 Mon Sep 17 00:00:00 2001 From: Adam Van Ymeren Date: Wed, 15 Feb 2017 13:31:07 -0500 Subject: [PATCH] Fixes issue #548 Add validation checks for the reserved OpImageSparseSampleProj* opcodes. --- CHANGES | 2 + source/validate_instruction.cpp | 18 +++++++++ test/val/CMakeLists.txt | 5 +++ test/val/val_fixtures.cpp | 1 + test/val/val_instructions_test.cpp | 82 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 108 insertions(+) create mode 100644 test/val/val_instructions_test.cpp diff --git a/CHANGES b/CHANGES index 73b461e..11aa9e1 100644 --- a/CHANGES +++ b/CHANGES @@ -11,6 +11,8 @@ v2016.7-dev 2017-01-06 nested control construct. #551: If a merge block is reachable, it must be *strictly* dominated by its header. + #548: Validator: Error when the reserved OpImageSparseSampleProj* opcodes + are used. v2016.6 2016-12-13 - Published the C++ interface for assembling, disassembling, validation, and diff --git a/source/validate_instruction.cpp b/source/validate_instruction.cpp index e75bbb8..e81bfd0 100644 --- a/source/validate_instruction.cpp +++ b/source/validate_instruction.cpp @@ -132,6 +132,23 @@ spv_result_t CapCheck(ValidationState_t& _, return SPV_SUCCESS; } +// Checks that the instruction is not reserved for future use. +spv_result_t ReservedCheck(ValidationState_t& _, + const spv_parsed_instruction_t* inst) { + const SpvOp opcode = static_cast(inst->opcode); + switch (opcode) { + case SpvOpImageSparseSampleProjImplicitLod: + case SpvOpImageSparseSampleProjExplicitLod: + case SpvOpImageSparseSampleProjDrefImplicitLod: + case SpvOpImageSparseSampleProjDrefExplicitLod: + return _.diag(SPV_ERROR_INVALID_VALUE) + << spvOpcodeString(opcode) + << " is reserved for future use."; + default: + return SPV_SUCCESS; + } +} + // Checks that the Resuld is within the valid bound. spv_result_t LimitCheckIdBound(ValidationState_t& _, const spv_parsed_instruction_t* inst) { @@ -365,6 +382,7 @@ spv_result_t InstructionPass(ValidationState_t& _, if (auto error = LimitCheckIdBound(_, inst)) return error; if (auto error = LimitCheckStruct(_, inst)) return error; if (auto error = LimitCheckSwitch(_, inst)) return error; + if (auto error = ReservedCheck(_, inst)) return error; // All instruction checks have passed. return SPV_SUCCESS; diff --git a/test/val/CMakeLists.txt b/test/val/CMakeLists.txt index 80b0260..8c41185 100644 --- a/test/val/CMakeLists.txt +++ b/test/val/CMakeLists.txt @@ -87,3 +87,8 @@ add_spvtools_unittest(TARGET val_decoration LIBS ${SPIRV_TOOLS} ) +add_spvtools_unittest(TARGET val_instructions + SRCS val_instructions_test.cpp + ${VAL_TEST_COMMON_SRCS} + LIBS ${SPIRV_TOOLS} +) diff --git a/test/val/val_fixtures.cpp b/test/val/val_fixtures.cpp index b017433..24c6ec3 100644 --- a/test/val/val_fixtures.cpp +++ b/test/val/val_fixtures.cpp @@ -95,4 +95,5 @@ template class spvtest::ValidateBase< std::tuple, std::function>>>; template class spvtest::ValidateBase; +template class spvtest::ValidateBase>; } diff --git a/test/val/val_instructions_test.cpp b/test/val/val_instructions_test.cpp new file mode 100644 index 0000000..56c25ba --- /dev/null +++ b/test/val/val_instructions_test.cpp @@ -0,0 +1,82 @@ +// Copyright (c) 2017 Google Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Validation tests for illegal instructions + +#include +#include +#include + +#include "gmock/gmock.h" +#include "val_fixtures.h" + +using ::testing::HasSubstr; + +using ImageSampleTestParameter = std::pair; + +const std::string& Opcode(const ImageSampleTestParameter& p) { return p.first; } +const std::string& Operands(const ImageSampleTestParameter& p) { + return p.second; +} + +using ValidateIns = spvtest::ValidateBase; + +namespace { + +TEST_P(ValidateIns, Reserved) { + const auto& param = GetParam(); + + std::string str = R"( + OpCapability Shader + OpCapability SparseResidency + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %main "main" %img +%void = OpTypeVoid +%int = OpTypeInt 32 0 +%float = OpTypeFloat 32 +%fnt = OpTypeFunction %void +%coord = OpConstantNull %float +%lod = OpConstantNull %float +%dref = OpConstantNull %float +%imgt = OpTypeImage %float 2D 0 0 0 0 Rgba32f +%sampledt = OpTypeSampledImage %imgt +%sampledp = OpTypePointer Uniform %sampledt +%img = OpVariable %sampledp Input +%main = OpFunction %void None %fnt +%label = OpLabel +%sample = Op)" + Opcode(param) + + " " + Operands(param) + R"( + OpReturn + OpFunctionEnd +)"; + + CompileSuccessfully(str); + EXPECT_EQ(SPV_ERROR_INVALID_VALUE, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr(Opcode(param) + " is reserved for future use.")); +} + +#define CASE(NAME, ARGS) \ + { "ImageSparseSampleProj" #NAME, ARGS } + +INSTANTIATE_TEST_CASE_P( + OpImageSparseSampleProj, ValidateIns, + ::testing::ValuesIn(std::vector{ + CASE(ImplicitLod, "%float %img %coord"), + CASE(ExplicitLod, "%float %img %coord Lod %lod"), + CASE(DrefImplicitLod, "%float %img %coord %dref"), + CASE(DrefExplicitLod, "%float %img %coord %dref Lod %lod"), + }), ); +#undef CASE +} -- 2.7.4