spirv-fuzz: Clamp statically out-of-bounds accesses in code donation (#3315)

It has been resolved that statically out-of-bounds accesses are not
invalid in SPIR-V (they lead to undefind behaviour at runtime but
should not cause a module to be rejected during validation).  This
change tolerates such accesses in donated code, clamping them in-bound
as part of making a function live-safe.
This commit is contained in:
Alastair Donaldson 2020-04-27 14:24:54 +01:00 committed by GitHub
parent b74199a22d
commit 88faf63ad3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 123 additions and 22 deletions

View File

@ -810,8 +810,10 @@ bool TransformationAddFunction::TryToClampAccessChainIndices(
->GetType(index_type_inst->result_id())
->AsInteger();
if (index_inst->opcode() != SpvOpConstant) {
// The index is non-constant so we need to clamp it.
if (index_inst->opcode() != SpvOpConstant ||
index_inst->GetSingleWordInOperand(0) >= bound) {
// The index is either non-constant or an out-of-bounds constant, so we
// need to clamp it.
assert(should_be_composite_type->opcode() != SpvOpTypeStruct &&
"Access chain indices into structures are required to be "
"constants.");
@ -864,21 +866,6 @@ bool TransformationAddFunction::TryToClampAccessChainIndices(
access_chain_inst->SetInOperand(index, {select_id});
fuzzerutil::UpdateModuleIdBound(ir_context, compare_id);
fuzzerutil::UpdateModuleIdBound(ir_context, select_id);
} else {
// TODO(afd): At present the SPIR-V spec is not clear on whether
// statically out-of-bounds indices mean that a module is invalid (so
// that it should be rejected by the validator), or that such accesses
// yield undefined results. Via the following assertion, we assume that
// functions added to the module do not feature statically out-of-bounds
// accesses.
// Assert that the index is smaller (unsigned) than this value.
// Return false if it is not (to keep compilers happy).
if (index_inst->GetSingleWordInOperand(0) >= bound) {
assert(false &&
"The function has a statically out-of-bounds access; "
"this should not occur.");
return false;
}
}
should_be_composite_type =
FollowCompositeIndex(ir_context, *should_be_composite_type, index_id);

View File

@ -1862,7 +1862,7 @@ TEST(TransformationAddFunctionTest,
const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
ASSERT_TRUE(IsValid(env, context.get()));
// Make a sequence of instruction messages corresponding to function %8 in
// Make a sequence of instruction messages corresponding to function %6 in
// |donor|.
std::vector<protobufs::Instruction> instructions =
GetInstructionsForFunction(env, consumer, donor, 6);
@ -2020,7 +2020,7 @@ TEST(TransformationAddFunctionTest,
const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
ASSERT_TRUE(IsValid(env, context.get()));
// Make a sequence of instruction messages corresponding to function %8 in
// Make a sequence of instruction messages corresponding to function %6 in
// |donor|.
std::vector<protobufs::Instruction> instructions =
GetInstructionsForFunction(env, consumer, donor, 6);
@ -2176,7 +2176,7 @@ TEST(TransformationAddFunctionTest, LoopLimitersHeaderIsBackEdgeBlock) {
const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
ASSERT_TRUE(IsValid(env, context.get()));
// Make a sequence of instruction messages corresponding to function %8 in
// Make a sequence of instruction messages corresponding to function %6 in
// |donor|.
std::vector<protobufs::Instruction> instructions =
GetInstructionsForFunction(env, consumer, donor, 6);
@ -2324,7 +2324,7 @@ TEST(TransformationAddFunctionTest, InfiniteLoop1) {
const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
ASSERT_TRUE(IsValid(env, context.get()));
// Make a sequence of instruction messages corresponding to function %8 in
// Make a sequence of instruction messages corresponding to function %6 in
// |donor|.
std::vector<protobufs::Instruction> instructions =
GetInstructionsForFunction(env, consumer, donor, 6);
@ -2482,7 +2482,7 @@ TEST(TransformationAddFunctionTest, UnreachableContinueConstruct) {
const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
ASSERT_TRUE(IsValid(env, context.get()));
// Make a sequence of instruction messages corresponding to function %8 in
// Make a sequence of instruction messages corresponding to function %6 in
// |donor|.
std::vector<protobufs::Instruction> instructions =
GetInstructionsForFunction(env, consumer, donor, 6);
@ -2925,6 +2925,120 @@ TEST(TransformationAddFunctionTest, LoopLimitersAndOpPhi2) {
ASSERT_TRUE(IsEqual(env, expected, context.get()));
}
TEST(TransformationAddFunctionTest, StaticallyOutOfBoundsArrayAccess) {
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
%8 = OpTypeInt 32 1
%9 = OpTypeInt 32 0
%10 = OpConstant %9 3
%11 = OpTypeArray %8 %10
%12 = OpTypePointer Private %11
%13 = OpVariable %12 Private
%14 = OpConstant %8 3
%20 = OpConstant %8 2
%15 = OpConstant %8 1
%21 = OpTypeBool
%16 = OpTypePointer Private %8
%4 = OpFunction %2 None %3
%5 = OpLabel
OpReturn
OpFunctionEnd
)";
std::string donor = 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
%8 = OpTypeInt 32 1
%9 = OpTypeInt 32 0
%10 = OpConstant %9 3
%11 = OpTypeArray %8 %10
%12 = OpTypePointer Private %11
%13 = OpVariable %12 Private
%14 = OpConstant %8 3
%15 = OpConstant %8 1
%16 = OpTypePointer Private %8
%4 = OpFunction %2 None %3
%5 = OpLabel
OpReturn
OpFunctionEnd
%6 = OpFunction %2 None %3
%7 = OpLabel
%17 = OpAccessChain %16 %13 %14
OpStore %17 %15
OpReturn
OpFunctionEnd
)";
const auto env = SPV_ENV_UNIVERSAL_1_4;
const auto consumer = nullptr;
FactManager fact_manager;
spvtools::ValidatorOptions validator_options;
TransformationContext transformation_context(&fact_manager,
validator_options);
const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
ASSERT_TRUE(IsValid(env, context.get()));
// Make a sequence of instruction messages corresponding to function %6 in
// |donor|.
std::vector<protobufs::Instruction> instructions =
GetInstructionsForFunction(env, consumer, donor, 6);
TransformationAddFunction add_livesafe_function(
instructions, 0, 0, {}, 0, {MakeAccessClampingInfo(17, {{100, 101}})});
ASSERT_TRUE(add_livesafe_function.IsApplicable(context.get(),
transformation_context));
add_livesafe_function.Apply(context.get(), &transformation_context);
ASSERT_TRUE(IsValid(env, context.get()));
std::string expected = 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
%8 = OpTypeInt 32 1
%9 = OpTypeInt 32 0
%10 = OpConstant %9 3
%11 = OpTypeArray %8 %10
%12 = OpTypePointer Private %11
%13 = OpVariable %12 Private
%14 = OpConstant %8 3
%20 = OpConstant %8 2
%15 = OpConstant %8 1
%21 = OpTypeBool
%16 = OpTypePointer Private %8
%4 = OpFunction %2 None %3
%5 = OpLabel
OpReturn
OpFunctionEnd
%6 = OpFunction %2 None %3
%7 = OpLabel
%100 = OpULessThanEqual %21 %14 %20
%101 = OpSelect %8 %100 %14 %20
%17 = OpAccessChain %16 %13 %101
OpStore %17 %15
OpReturn
OpFunctionEnd
)";
ASSERT_TRUE(IsEqual(env, expected, context.get()));
}
} // namespace
} // namespace fuzz
} // namespace spvtools