From 1299436c8f16e084d11e04c70ca4189d8f12f555 Mon Sep 17 00:00:00 2001 From: Alastair Donaldson Date: Fri, 27 Nov 2020 16:31:04 +0000 Subject: [PATCH] Reject SPIR-V that applies void to OpUndef, OpCopyObject, OpPhi (#4036) Fixes #4035. --- source/opt/inline_exhaustive_pass.cpp | 5 - source/opt/inline_pass.cpp | 13 +-- source/val/validate_cfg.cpp | 14 ++- source/val/validate_composites.cpp | 4 + source/val/validate_misc.cpp | 4 + ...mation_flatten_conditional_branch_test.cpp | 2 - .../transformation_outline_function_test.cpp | 92 ------------------- test/opt/inline_opaque_test.cpp | 84 +++++++++-------- test/opt/inline_test.cpp | 63 +++---------- test/val/val_cfg_test.cpp | 36 +++++++- test/val/val_composites_test.cpp | 30 ++++++ test/val/val_id_test.cpp | 6 +- test/val/val_misc_test.cpp | 23 +++++ 13 files changed, 170 insertions(+), 206 deletions(-) diff --git a/source/opt/inline_exhaustive_pass.cpp b/source/opt/inline_exhaustive_pass.cpp index 24f4e736..f24f744d 100644 --- a/source/opt/inline_exhaustive_pass.cpp +++ b/source/opt/inline_exhaustive_pass.cpp @@ -38,11 +38,6 @@ Pass::Status InlineExhaustivePass::InlineExhaustive(Function* func) { if (newBlocks.size() > 1) UpdateSucceedingPhis(newBlocks); // Replace old calling block with new block(s). - // We need to kill the name and decorations for the call, which - // will be deleted. Other instructions in the block will be moved to - // newBlocks. We don't need to do anything with those. - context()->KillNamesAndDecorates(&*ii); - bi = bi.Erase(); for (auto& bb : newBlocks) { diff --git a/source/opt/inline_pass.cpp b/source/opt/inline_pass.cpp index eaf29aa0..88f395f0 100644 --- a/source/opt/inline_pass.cpp +++ b/source/opt/inline_pass.cpp @@ -619,14 +619,6 @@ bool InlinePass::GenInlineCode( assert(resId != 0); AddLoad(calleeTypeId, resId, returnVarId, &new_blk_ptr, call_inst_itr->dbg_line_inst(), call_inst_itr->GetDebugScope()); - } else { - // Even though it is very unlikely, it is possible that the result id of - // the void-function call is used, so we need to generate an instruction - // with that result id. - std::unique_ptr undef_inst( - new Instruction(context(), SpvOpUndef, call_inst_itr->type_id(), - call_inst_itr->result_id(), {})); - context()->AddGlobalValue(std::move(undef_inst)); } // Move instructions of original caller block after call instruction. @@ -645,6 +637,11 @@ bool InlinePass::GenInlineCode( for (auto& blk : *new_blocks) { id2block_[blk->id()] = &*blk; } + + // We need to kill the name and decorations for the call, which will be + // deleted. + context()->KillNamesAndDecorates(&*call_inst_itr); + return true; } diff --git a/source/val/validate_cfg.cpp b/source/val/validate_cfg.cpp index 45edd0c9..ca2ae1ae 100644 --- a/source/val/validate_cfg.cpp +++ b/source/val/validate_cfg.cpp @@ -48,11 +48,11 @@ spv_result_t ValidatePhi(ValidationState_t& _, const Instruction* inst) { "basic blocks."; } - const Instruction* type_inst = _.FindDef(inst->type_id()); - assert(type_inst); - - const SpvOp type_opcode = type_inst->opcode(); - if (type_opcode == SpvOpTypePointer && + if (_.IsVoidType(inst->type_id())) { + return _.diag(SPV_ERROR_INVALID_DATA, inst) + << "OpPhi must not have void result type"; + } + if (_.IsPointerType(inst->type_id()) && _.addressing_model() == SpvAddressingModelLogical) { if (!_.features().variable_pointers && !_.features().variable_pointers_storage_buffer) { @@ -62,6 +62,10 @@ spv_result_t ValidatePhi(ValidationState_t& _, const Instruction* inst) { } } + const Instruction* type_inst = _.FindDef(inst->type_id()); + assert(type_inst); + const SpvOp type_opcode = type_inst->opcode(); + if (!_.options()->before_hlsl_legalization) { if (type_opcode == SpvOpTypeSampledImage || (_.HasCapability(SpvCapabilityShader) && diff --git a/source/val/validate_composites.cpp b/source/val/validate_composites.cpp index eb8a3244..d5b978ff 100644 --- a/source/val/validate_composites.cpp +++ b/source/val/validate_composites.cpp @@ -437,6 +437,10 @@ spv_result_t ValidateCopyObject(ValidationState_t& _, const Instruction* inst) { return _.diag(SPV_ERROR_INVALID_DATA, inst) << "Expected Result Type and Operand type to be the same"; } + if (_.IsVoidType(result_type)) { + return _.diag(SPV_ERROR_INVALID_DATA, inst) + << "OpCopyObject cannot have void result type"; + } return SPV_SUCCESS; } diff --git a/source/val/validate_misc.cpp b/source/val/validate_misc.cpp index f0deedfa..8cccd4c5 100644 --- a/source/val/validate_misc.cpp +++ b/source/val/validate_misc.cpp @@ -26,6 +26,10 @@ namespace val { namespace { spv_result_t ValidateUndef(ValidationState_t& _, const Instruction* inst) { + if (_.IsVoidType(inst->type_id())) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << "Cannot create undefined values with void type"; + } if (_.HasCapability(SpvCapabilityShader) && _.ContainsLimitedUseIntOrFloatType(inst->type_id()) && !_.IsPointerType(inst->type_id())) { diff --git a/test/fuzz/transformation_flatten_conditional_branch_test.cpp b/test/fuzz/transformation_flatten_conditional_branch_test.cpp index 540275a8..1a0ff6aa 100644 --- a/test/fuzz/transformation_flatten_conditional_branch_test.cpp +++ b/test/fuzz/transformation_flatten_conditional_branch_test.cpp @@ -666,7 +666,6 @@ TEST(TransformationFlattenConditionalBranchTest, EdgeCases) { OpBranchConditional %5 %13 %12 %13 = OpLabel %14 = OpFunctionCall %3 %11 - %15 = OpCopyObject %3 %14 OpBranch %12 %12 = OpLabel OpReturn @@ -774,7 +773,6 @@ TEST(TransformationFlattenConditionalBranchTest, EdgeCases) { OpBranchConditional %5 %13 %12 %13 = OpLabel %14 = OpFunctionCall %3 %11 - %15 = OpCopyObject %3 %14 OpBranch %12 %12 = OpLabel OpReturn diff --git a/test/fuzz/transformation_outline_function_test.cpp b/test/fuzz/transformation_outline_function_test.cpp index 6c0bff4b..c567680c 100644 --- a/test/fuzz/transformation_outline_function_test.cpp +++ b/test/fuzz/transformation_outline_function_test.cpp @@ -2531,98 +2531,6 @@ TEST(TransformationOutlineFunctionTest, ExitBlockHeadsLoop) { ApplyAndCheckFreshIds(transformation, context.get(), &transformation_context); } -TEST(TransformationOutlineFunctionTest, SkipVoidOutputId) { - std::string shader = R"( - OpCapability Shader - %1 = OpExtInstImport "GLSL.std.450" - OpMemoryModel Logical GLSL450 - OpEntryPoint Fragment %6 "main" - OpExecutionMode %6 OriginUpperLeft - OpSource ESSL 310 - %2 = OpTypeVoid - %3 = OpTypeFunction %2 - %21 = OpTypeBool - %81 = OpConstantTrue %21 - %6 = OpFunction %2 None %3 - %7 = OpLabel - OpBranch %80 - %80 = OpLabel - %84 = OpFunctionCall %2 %87 - OpBranch %90 - %90 = OpLabel - %86 = OpPhi %2 %84 %80 - OpReturn - OpFunctionEnd - %87 = OpFunction %2 None %3 - %88 = OpLabel - OpReturn - OpFunctionEnd - )"; - - const auto env = SPV_ENV_UNIVERSAL_1_5; - const auto consumer = nullptr; - const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption); - spvtools::ValidatorOptions validator_options; - ASSERT_TRUE(fuzzerutil::IsValidAndWellFormed(context.get(), validator_options, - kConsoleMessageConsumer)); - TransformationContext transformation_context( - MakeUnique(context.get()), validator_options); - TransformationOutlineFunction transformation( - /*entry_block*/ 80, - /*exit_block*/ 80, - /*new_function_struct_return_type_id*/ 300, - /*new_function_type_id*/ 301, - /*new_function_id*/ 302, - /*new_function_region_entry_block*/ 304, - /*new_caller_result_id*/ 305, - /*new_callee_result_id*/ 306, - /*input_id_to_fresh_id*/ {}, - /*output_id_to_fresh_id*/ {{84, 307}}); - - ASSERT_TRUE( - transformation.IsApplicable(context.get(), transformation_context)); - ApplyAndCheckFreshIds(transformation, context.get(), &transformation_context); - ASSERT_TRUE(fuzzerutil::IsValidAndWellFormed(context.get(), validator_options, - kConsoleMessageConsumer)); - - std::string after_transformation = R"( - OpCapability Shader - %1 = OpExtInstImport "GLSL.std.450" - OpMemoryModel Logical GLSL450 - OpEntryPoint Fragment %6 "main" - OpExecutionMode %6 OriginUpperLeft - OpSource ESSL 310 - %2 = OpTypeVoid - %3 = OpTypeFunction %2 - %21 = OpTypeBool - %81 = OpConstantTrue %21 - %300 = OpTypeStruct - %301 = OpTypeFunction %300 - %6 = OpFunction %2 None %3 - %7 = OpLabel - OpBranch %80 - %80 = OpLabel - %305 = OpFunctionCall %300 %302 - %84 = OpUndef %2 - OpBranch %90 - %90 = OpLabel - %86 = OpPhi %2 %84 %80 - OpReturn - OpFunctionEnd - %87 = OpFunction %2 None %3 - %88 = OpLabel - OpReturn - OpFunctionEnd - %302 = OpFunction %300 None %301 - %304 = OpLabel - %307 = OpFunctionCall %2 %87 - %306 = OpCompositeConstruct %300 - OpReturnValue %306 - OpFunctionEnd - )"; - ASSERT_TRUE(IsEqual(env, after_transformation, context.get())); -} - TEST(TransformationOutlineFunctionTest, Miscellaneous1) { // This tests outlining of some non-trivial code, and also tests the way // overflow ids are used by the transformation. diff --git a/test/opt/inline_opaque_test.cpp b/test/opt/inline_opaque_test.cpp index 47e05333..8cb8925c 100644 --- a/test/opt/inline_opaque_test.cpp +++ b/test/opt/inline_opaque_test.cpp @@ -28,7 +28,7 @@ TEST_F(InlineOpaqueTest, InlineCallWithStructArgContainingSampledImage) { // Function with opaque argument is inlined. // TODO(greg-lunarg): Add HLSL code - const std::string predefs = + const std::string predefs_1 = R"(OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 @@ -47,22 +47,27 @@ OpName %sampler15 "sampler15" OpName %s0 "s0" OpName %texCoords "texCoords" OpName %param "param" -OpDecorate %sampler15 DescriptorSet 0 +)"; + + const std::string name = R"(OpName %return_value "return_value" +)"; + + const std::string predefs_2 = R"(OpDecorate %sampler15 DescriptorSet 0 %void = OpTypeVoid -%12 = OpTypeFunction %void +%13 = OpTypeFunction %void %float = OpTypeFloat 32 %v2float = OpTypeVector %float 2 %v4float = OpTypeVector %float 4 %_ptr_Output_v4float = OpTypePointer Output %v4float %outColor = OpVariable %_ptr_Output_v4float Output -%17 = OpTypeImage %float 2D 0 0 0 1 Unknown -%18 = OpTypeSampledImage %17 -%S_t = OpTypeStruct %v2float %v2float %18 +%18 = OpTypeImage %float 2D 0 0 0 1 Unknown +%19 = OpTypeSampledImage %18 +%S_t = OpTypeStruct %v2float %v2float %19 %_ptr_Function_S_t = OpTypePointer Function %S_t -%20 = OpTypeFunction %void %_ptr_Function_S_t -%_ptr_UniformConstant_18 = OpTypePointer UniformConstant %18 -%_ptr_Function_18 = OpTypePointer Function %18 -%sampler15 = OpVariable %_ptr_UniformConstant_18 UniformConstant +%21 = OpTypeFunction %void %_ptr_Function_S_t +%_ptr_UniformConstant_19 = OpTypePointer UniformConstant %19 +%_ptr_Function_19 = OpTypePointer Function %19 +%sampler15 = OpVariable %_ptr_UniformConstant_19 UniformConstant %int = OpTypeInt 32 1 %int_0 = OpConstant %int 0 %int_2 = OpConstant %int 2 @@ -72,39 +77,38 @@ OpDecorate %sampler15 DescriptorSet 0 )"; const std::string before = - R"(%main = OpFunction %void None %12 -%28 = OpLabel + R"(%main = OpFunction %void None %13 +%29 = OpLabel %s0 = OpVariable %_ptr_Function_S_t Function %param = OpVariable %_ptr_Function_S_t Function -%29 = OpLoad %v2float %texCoords -%30 = OpAccessChain %_ptr_Function_v2float %s0 %int_0 -OpStore %30 %29 -%31 = OpLoad %18 %sampler15 -%32 = OpAccessChain %_ptr_Function_18 %s0 %int_2 -OpStore %32 %31 -%33 = OpLoad %S_t %s0 -OpStore %param %33 -%34 = OpFunctionCall %void %foo_struct_S_t_vf2_vf21_ %param +%30 = OpLoad %v2float %texCoords +%31 = OpAccessChain %_ptr_Function_v2float %s0 %int_0 +OpStore %31 %30 +%32 = OpLoad %19 %sampler15 +%33 = OpAccessChain %_ptr_Function_19 %s0 %int_2 +OpStore %33 %32 +%34 = OpLoad %S_t %s0 +OpStore %param %34 +%return_value = OpFunctionCall %void %foo_struct_S_t_vf2_vf21_ %param OpReturn OpFunctionEnd )"; const std::string after = - R"(%34 = OpUndef %void -%main = OpFunction %void None %12 -%28 = OpLabel + R"(%main = OpFunction %void None %13 +%29 = OpLabel %s0 = OpVariable %_ptr_Function_S_t Function %param = OpVariable %_ptr_Function_S_t Function -%29 = OpLoad %v2float %texCoords -%30 = OpAccessChain %_ptr_Function_v2float %s0 %int_0 -OpStore %30 %29 -%31 = OpLoad %18 %sampler15 -%32 = OpAccessChain %_ptr_Function_18 %s0 %int_2 -OpStore %32 %31 -%33 = OpLoad %S_t %s0 -OpStore %param %33 -%42 = OpAccessChain %_ptr_Function_18 %param %int_2 -%43 = OpLoad %18 %42 +%30 = OpLoad %v2float %texCoords +%31 = OpAccessChain %_ptr_Function_v2float %s0 %int_0 +OpStore %31 %30 +%32 = OpLoad %19 %sampler15 +%33 = OpAccessChain %_ptr_Function_19 %s0 %int_2 +OpStore %33 %32 +%34 = OpLoad %S_t %s0 +OpStore %param %34 +%42 = OpAccessChain %_ptr_Function_19 %param %int_2 +%43 = OpLoad %19 %42 %44 = OpAccessChain %_ptr_Function_v2float %param %int_0 %45 = OpLoad %v2float %44 %46 = OpImageSampleImplicitLod %v4float %43 %45 @@ -114,11 +118,11 @@ OpFunctionEnd )"; const std::string post_defs = - R"(%foo_struct_S_t_vf2_vf21_ = OpFunction %void None %20 + R"(%foo_struct_S_t_vf2_vf21_ = OpFunction %void None %21 %s = OpFunctionParameter %_ptr_Function_S_t %35 = OpLabel -%36 = OpAccessChain %_ptr_Function_18 %s %int_2 -%37 = OpLoad %18 %36 +%36 = OpAccessChain %_ptr_Function_19 %s %int_2 +%37 = OpLoad %19 %36 %38 = OpAccessChain %_ptr_Function_v2float %s %int_0 %39 = OpLoad %v2float %38 %40 = OpImageSampleImplicitLod %v4float %37 %39 @@ -128,7 +132,8 @@ OpFunctionEnd )"; SinglePassRunAndCheck( - predefs + before + post_defs, predefs + after + post_defs, true, true); + predefs_1 + name + predefs_2 + before + post_defs, + predefs_1 + predefs_2 + after + post_defs, true, true); } TEST_F(InlineOpaqueTest, InlineOpaqueReturn) { @@ -290,8 +295,7 @@ OpFunctionEnd )"; const std::string after = - R"(%35 = OpUndef %void -%main2 = OpFunction %void None %13 + R"(%main2 = OpFunction %void None %13 %29 = OpLabel %s0 = OpVariable %_ptr_Function_S_t Function %param = OpVariable %_ptr_Function_S_t Function diff --git a/test/opt/inline_test.cpp b/test/opt/inline_test.cpp index c0ca6da7..d3ef09c3 100644 --- a/test/opt/inline_test.cpp +++ b/test/opt/inline_test.cpp @@ -381,7 +381,6 @@ TEST_F(InlineTest, InOutParameter) { const std::vector after = { // clang-format off - "%26 = OpUndef %void", "%main = OpFunction %void None %11", "%23 = OpLabel", "%b = OpVariable %_ptr_Function_v4float Function", @@ -1543,11 +1542,9 @@ OpReturn OpFunctionEnd )"; - const std::string undef = "%11 = OpUndef %void\n"; - - SinglePassRunAndCheck( - predefs + nonEntryFuncs + before, predefs + undef + nonEntryFuncs + after, - false, true); + SinglePassRunAndCheck(predefs + nonEntryFuncs + before, + predefs + nonEntryFuncs + after, + false, true); } TEST_F(InlineTest, MultiBlockLoopHeaderCallsMultiBlockCallee) { @@ -1622,10 +1619,9 @@ OpReturn OpFunctionEnd )"; - const std::string undef = "%20 = OpUndef %void\n"; - SinglePassRunAndCheck( - predefs + nonEntryFuncs + before, predefs + undef + nonEntryFuncs + after, - false, true); + SinglePassRunAndCheck(predefs + nonEntryFuncs + before, + predefs + nonEntryFuncs + after, + false, true); } TEST_F(InlineTest, SingleBlockLoopCallsMultiBlockCalleeHavingSelectionMerge) { @@ -1711,10 +1707,9 @@ OpBranchConditional %true %13 %16 OpReturn OpFunctionEnd )"; - const std::string undef = "%15 = OpUndef %void\n"; - SinglePassRunAndCheck( - predefs + nonEntryFuncs + before, predefs + undef + nonEntryFuncs + after, - false, true); + SinglePassRunAndCheck(predefs + nonEntryFuncs + before, + predefs + nonEntryFuncs + after, + false, true); } TEST_F(InlineTest, @@ -1793,10 +1788,9 @@ OpReturn OpFunctionEnd )"; - const std::string undef = "%20 = OpUndef %void\n"; - SinglePassRunAndCheck( - predefs + nonEntryFuncs + before, predefs + undef + nonEntryFuncs + after, - false, true); + SinglePassRunAndCheck(predefs + nonEntryFuncs + before, + predefs + nonEntryFuncs + after, + false, true); } TEST_F(InlineTest, NonInlinableCalleeWithSingleReturn) { @@ -2169,7 +2163,6 @@ OpName %foo "foo" OpName %foo_entry "foo_entry" %void = OpTypeVoid %void_fn = OpTypeFunction %void -%3 = OpUndef %void %foo = OpFunction %void None %void_fn %foo_entry = OpLabel OpReturn @@ -2475,7 +2468,6 @@ OpName %kill_ "kill(" %3 = OpTypeFunction %void %bool = OpTypeBool %true = OpConstantTrue %bool -%16 = OpUndef %void %main = OpFunction %void None %3 %5 = OpLabel OpKill @@ -2573,7 +2565,6 @@ OpName %kill_ "kill(" %3 = OpTypeFunction %void %bool = OpTypeBool %true = OpConstantTrue %bool -%16 = OpUndef %void %main = OpFunction %void None %3 %5 = OpLabel OpTerminateInvocation @@ -2801,7 +2792,6 @@ OpFunctionEnd %uint_0 = OpConstant %uint 0 %false = OpConstantFalse %bool %_ptr_Function_bool = OpTypePointer Function %bool -%11 = OpUndef %void %foo_ = OpFunction %void None %4 %7 = OpLabel %18 = OpVariable %_ptr_Function_bool Function %false @@ -3890,35 +3880,6 @@ OpFunctionEnd SinglePassRunAndMatch(text, true); } -TEST_F(InlineTest, UsingVoidFunctionResult) { - const std::string text = R"( -; CHECK: [[undef:%\w+]] = OpUndef %void -; CHECK: OpFunction -; CHECK: OpCopyObject %void [[undef]] -; CHECK: OpFunctionEnd - OpCapability Shader - %1 = OpExtInstImport "GLSL.std.450" - OpMemoryModel Logical GLSL450 - OpEntryPoint Fragment %4 "main" - OpExecutionMode %4 OriginUpperLeft - OpSource ESSL 320 - %2 = OpTypeVoid - %3 = OpTypeFunction %2 - %4 = OpFunction %2 None %3 - %5 = OpLabel - %8 = OpFunctionCall %2 %6 - %9 = OpCopyObject %2 %8 - OpReturn - OpFunctionEnd - %6 = OpFunction %2 None %3 - %7 = OpLabel - OpReturn - OpFunctionEnd -)"; - - SinglePassRunAndMatch(text, true); -} - // TODO(greg-lunarg): Add tests to verify handling of these cases: // // Empty modules diff --git a/test/val/val_cfg_test.cpp b/test/val/val_cfg_test.cpp index b4d1c289..7a268ebb 100644 --- a/test/val/val_cfg_test.cpp +++ b/test/val/val_cfg_test.cpp @@ -496,10 +496,10 @@ TEST_P(ValidateCFG, BranchTargetFirstBlockBadSinceEntryBlock) { TEST_P(ValidateCFG, BranchTargetFirstBlockBadSinceValue) { Block entry("entry"); - entry.SetBody("%undef = OpUndef %voidt\n"); + entry.SetBody("%undef = OpUndef %boolt\n"); Block bad("bad"); Block end("end", SpvOpReturn); - Block badvalue("undef"); // This referenes the OpUndef. + Block badvalue("undef"); // This references the OpUndef. std::string str = GetDefaultHeader(GetParam()) + nameOps("entry", "bad", std::make_pair("func", "Main")) + types_consts() + @@ -4598,6 +4598,38 @@ TEST_F(ValidateCFG, PhiInstructionWithDuplicateIncomingEdges) { EXPECT_THAT(getDiagnosticString(), HasSubstr("multiple times.")); } +TEST_F(ValidateCFG, PhiOnVoid) { + const std::string text = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %4 "main" + OpExecutionMode %4 OriginUpperLeft + OpSource ESSL 320 + OpName %4 "main" + OpName %6 "foo(" + %2 = OpTypeVoid + %3 = OpTypeFunction %2 + %4 = OpFunction %2 None %3 + %5 = OpLabel + %8 = OpFunctionCall %2 %6 + OpBranch %20 + %20 = OpLabel + %21 = OpPhi %2 %8 %20 + OpReturn + OpFunctionEnd + %6 = OpFunction %2 None %3 + %7 = OpLabel + OpReturn + OpFunctionEnd +)"; + + CompileSuccessfully(text); + EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("OpPhi must not have void result type")); +} + } // namespace } // namespace val } // namespace spvtools diff --git a/test/val/val_composites_test.cpp b/test/val/val_composites_test.cpp index e9705621..bf7caa9f 100644 --- a/test/val/val_composites_test.cpp +++ b/test/val/val_composites_test.cpp @@ -1994,6 +1994,36 @@ OpFunctionEnd HasSubstr("Cannot transpose matrices of 16-bit floats")); } +TEST_F(ValidateComposites, CopyObjectVoid) { + const std::string spirv = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %4 "main" + OpExecutionMode %4 OriginUpperLeft + OpSource ESSL 320 + OpName %4 "main" + OpName %6 "foo(" + %2 = OpTypeVoid + %3 = OpTypeFunction %2 + %4 = OpFunction %2 None %3 + %5 = OpLabel + %8 = OpFunctionCall %2 %6 + %20 = OpCopyObject %2 %8 + OpReturn + OpFunctionEnd + %6 = OpFunction %2 None %3 + %7 = OpLabel + OpReturn + OpFunctionEnd +)"; + + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("OpCopyObject cannot have void result type")); +} + } // namespace } // namespace val } // namespace spvtools diff --git a/test/val/val_id_test.cpp b/test/val/val_id_test.cpp index adea5639..069e8f20 100644 --- a/test/val/val_id_test.cpp +++ b/test/val/val_id_test.cpp @@ -2749,9 +2749,13 @@ TEST_F(ValidateIdWithMessage, OpStoreObjectGood) { %6 = OpVariable %3 Uniform %7 = OpFunction %1 None %4 %8 = OpLabel -%9 = OpUndef %1 +%9 = OpFunctionCall %1 %10 OpStore %6 %9 OpReturn + OpFunctionEnd +%10 = OpFunction %1 None %4 +%11 = OpLabel + OpReturn OpFunctionEnd)"; CompileSuccessfully(spirv.c_str()); EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); diff --git a/test/val/val_misc_test.cpp b/test/val/val_misc_test.cpp index 93954844..e181f8d1 100644 --- a/test/val/val_misc_test.cpp +++ b/test/val/val_misc_test.cpp @@ -226,6 +226,29 @@ OpFunctionEnd)"; EXPECT_THAT(getDiagnosticString(), HasSubstr("Scope must be Subgroup or Device")); } + +TEST_F(ValidateMisc, UndefVoid) { + const std::string spirv = R"( + OpCapability Shader + %1 = OpExtInstImport "GLSL.std.450" + OpMemoryModel Logical GLSL450 + OpEntryPoint Fragment %4 "main" + OpExecutionMode %4 OriginUpperLeft + OpSource ESSL 320 + %2 = OpTypeVoid + %10 = OpUndef %2 + %3 = OpTypeFunction %2 + %4 = OpFunction %2 None %3 + %5 = OpLabel + OpReturn + OpFunctionEnd +)"; + + CompileSuccessfully(spirv); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions()); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("Cannot create undefined values with void type")); +} } // namespace } // namespace val } // namespace spvtools