mirror of
https://github.com/KhronosGroup/SPIRV-Tools
synced 2024-12-24 00:40:14 +00:00
spirv-fuzz: do not allow a dead break to target an unreachable block (#2917)
Because dominance information becomes a bit unreliable when blocks are unreachable, this change makes it so that the 'dead break' transformation will not introduce a break to an unreachable block. Fixes #2907.
This commit is contained in:
parent
27238bcca0
commit
84b1976061
@ -300,6 +300,13 @@ bool NewEdgeRespectsUseDefDominance(opt::IRContext* context,
|
||||
return true;
|
||||
}
|
||||
|
||||
bool BlockIsReachableInItsFunction(opt::IRContext* context,
|
||||
opt::BasicBlock* bb) {
|
||||
auto enclosing_function = bb->GetParent();
|
||||
return context->GetDominatorAnalysis(enclosing_function)
|
||||
->Dominates(enclosing_function->entry().get(), bb);
|
||||
}
|
||||
|
||||
} // namespace fuzzerutil
|
||||
|
||||
} // namespace fuzz
|
||||
|
@ -82,6 +82,11 @@ bool NewEdgeRespectsUseDefDominance(opt::IRContext* context,
|
||||
opt::BasicBlock* bb_from,
|
||||
opt::BasicBlock* bb_to);
|
||||
|
||||
// Returns true if and only if there is a path to |bb| from the entry block of
|
||||
// the function that contains |bb|.
|
||||
bool BlockIsReachableInItsFunction(opt::IRContext* context,
|
||||
opt::BasicBlock* bb);
|
||||
|
||||
} // namespace fuzzerutil
|
||||
|
||||
} // namespace fuzz
|
||||
|
@ -138,6 +138,13 @@ bool TransformationAddDeadBreak::IsApplicable(
|
||||
return false;
|
||||
}
|
||||
|
||||
if (!fuzzerutil::BlockIsReachableInItsFunction(context, bb_to)) {
|
||||
// If the target of the break is unreachable, we conservatively do not
|
||||
// allow adding a dead break, to avoid the compilations that arise due to
|
||||
// the lack of sensible dominance information for unreachable blocks.
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check that |message_.from_block| ends with an unconditional branch.
|
||||
if (bb_from->terminator()->opcode() != SpvOpBranch) {
|
||||
// The block associated with the id does not end with an unconditional
|
||||
|
@ -85,10 +85,9 @@ bool TransformationAddDeadContinue::IsApplicable(
|
||||
|
||||
auto continue_block = context->cfg()->block(loop_header)->ContinueBlockId();
|
||||
|
||||
if (!context->GetDominatorAnalysis(bb_from->GetParent())
|
||||
->Dominates(loop_header, continue_block)) {
|
||||
// If the loop's continue block is not dominated by the loop header, the
|
||||
// continue block is unreachable. In that case, we conservatively do not
|
||||
if (!fuzzerutil::BlockIsReachableInItsFunction(
|
||||
context, context->cfg()->block(continue_block))) {
|
||||
// If the loop's continue block is unreachable, we conservatively do not
|
||||
// allow adding a dead continue, to avoid the compilations that arise due to
|
||||
// the lack of sensible dominance information for unreachable blocks.
|
||||
return false;
|
||||
|
@ -2559,6 +2559,55 @@ TEST(TransformationAddDeadBreakTest, RespectDominanceRules8) {
|
||||
ASSERT_FALSE(bad_transformation.IsApplicable(context.get(), fact_manager));
|
||||
}
|
||||
|
||||
TEST(TransformationAddDeadBreakTest,
|
||||
BreakWouldDisobeyDominanceBlockOrderingRules) {
|
||||
std::string shader = R"(
|
||||
OpCapability Shader
|
||||
%1 = OpExtInstImport "GLSL.std.450"
|
||||
OpMemoryModel Logical GLSL450
|
||||
OpEntryPoint Fragment %4 "main"
|
||||
OpExecutionMode %4 OriginUpperLeft
|
||||
OpSource ESSL 310
|
||||
%2 = OpTypeVoid
|
||||
%3 = OpTypeFunction %2
|
||||
%6 = OpTypeBool
|
||||
%9 = OpConstantTrue %6
|
||||
%4 = OpFunction %2 None %3
|
||||
%5 = OpLabel
|
||||
OpBranch %10
|
||||
%10 = OpLabel
|
||||
OpLoopMerge %16 %15 None
|
||||
OpBranch %11
|
||||
%11 = OpLabel
|
||||
OpSelectionMerge %14 None
|
||||
OpBranchConditional %9 %12 %13
|
||||
%14 = OpLabel
|
||||
OpBranch %15
|
||||
%12 = OpLabel
|
||||
OpBranch %16
|
||||
%13 = OpLabel
|
||||
OpBranch %16
|
||||
%15 = OpLabel
|
||||
OpBranch %10
|
||||
%16 = OpLabel
|
||||
OpReturn
|
||||
OpFunctionEnd
|
||||
)";
|
||||
|
||||
const auto env = SPV_ENV_UNIVERSAL_1_3;
|
||||
const auto consumer = nullptr;
|
||||
const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
|
||||
ASSERT_TRUE(IsValid(env, context.get()));
|
||||
|
||||
FactManager fact_manager;
|
||||
|
||||
// Bad because 14 comes before 12 in the module, and 14 has no predecessors.
|
||||
// This means that an edge from 12 to 14 will lead to 12 dominating 14, which
|
||||
// is illegal if 12 appears after 14.
|
||||
auto bad_transformation = TransformationAddDeadBreak(12, 14, true, {});
|
||||
ASSERT_FALSE(bad_transformation.IsApplicable(context.get(), fact_manager));
|
||||
}
|
||||
|
||||
} // namespace
|
||||
} // namespace fuzz
|
||||
} // namespace spvtools
|
||||
|
Loading…
Reference in New Issue
Block a user