Reject SPIR-V that applies void to OpUndef, OpCopyObject, OpPhi (#4036)

Fixes #4035.
This commit is contained in:
Alastair Donaldson 2020-11-27 16:31:04 +00:00 коммит произвёл GitHub
Родитель 2c458414c0
Коммит 1299436c8f
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
13 изменённых файлов: 170 добавлений и 206 удалений

Просмотреть файл

@ -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) {

Просмотреть файл

@ -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<Instruction> 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;
}

Просмотреть файл

@ -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) &&

Просмотреть файл

@ -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;
}

Просмотреть файл

@ -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())) {

Просмотреть файл

@ -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

Просмотреть файл

@ -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<FactManager>(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.

Просмотреть файл

@ -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<InlineOpaquePass>(
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

Просмотреть файл

@ -381,7 +381,6 @@ TEST_F(InlineTest, InOutParameter) {
const std::vector<const char*> 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<InlineExhaustivePass>(
predefs + nonEntryFuncs + before, predefs + undef + nonEntryFuncs + after,
false, true);
SinglePassRunAndCheck<InlineExhaustivePass>(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<InlineExhaustivePass>(
predefs + nonEntryFuncs + before, predefs + undef + nonEntryFuncs + after,
false, true);
SinglePassRunAndCheck<InlineExhaustivePass>(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<InlineExhaustivePass>(
predefs + nonEntryFuncs + before, predefs + undef + nonEntryFuncs + after,
false, true);
SinglePassRunAndCheck<InlineExhaustivePass>(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<InlineExhaustivePass>(
predefs + nonEntryFuncs + before, predefs + undef + nonEntryFuncs + after,
false, true);
SinglePassRunAndCheck<InlineExhaustivePass>(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<InlineExhaustivePass>(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<InlineExhaustivePass>(text, true);
}
// TODO(greg-lunarg): Add tests to verify handling of these cases:
//
// Empty modules

Просмотреть файл

@ -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

Просмотреть файл

@ -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

Просмотреть файл

@ -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());

Просмотреть файл

@ -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