spirv-fuzz: Ensure that donated variables are always initialized (#3181)
This change ensures that global and local variables donated from other
modules are always initialized at their declaration in the module
being transformed. This is to help limit issues related to undefined
behaviour that might arise due to accessing uninitialized memory.
The change also introduces some helper functions in fuzzer_util to
make it easier to find the pointee types of pointer types.
diff --git a/source/fuzz/fuzzer_pass_donate_modules.cpp b/source/fuzz/fuzzer_pass_donate_modules.cpp
index 83cb18c..e820f25 100644
--- a/source/fuzz/fuzzer_pass_donate_modules.cpp
+++ b/source/fuzz/fuzzer_pass_donate_modules.cpp
@@ -402,14 +402,22 @@
// way they wish, and pass them as pointer parameters to functions
// without worrying about whether their data might get modified.
new_result_id = GetFuzzerContext()->GetFreshId();
+ uint32_t remapped_pointer_type =
+ original_id_to_donated_id->at(type_or_value.type_id());
+ uint32_t initializer_id;
+ if (type_or_value.NumInOperands() == 1) {
+ // The variable did not have an initializer; initialize it to zero.
+ // This is to limit problems associated with uninitialized data.
+ initializer_id = FindOrCreateZeroConstant(
+ fuzzerutil::GetPointeeTypeIdFromPointerType(
+ GetIRContext(), remapped_pointer_type));
+ } else {
+ // The variable already had an initializer; use its remapped id.
+ initializer_id = original_id_to_donated_id->at(
+ type_or_value.GetSingleWordInOperand(1));
+ }
ApplyTransformation(TransformationAddGlobalVariable(
- new_result_id,
- original_id_to_donated_id->at(type_or_value.type_id()),
- type_or_value.NumInOperands() == 1
- ? 0
- : original_id_to_donated_id->at(
- type_or_value.GetSingleWordInOperand(1)),
- true));
+ new_result_id, remapped_pointer_type, initializer_id, true));
} break;
case SpvOpUndef: {
// It is fine to have multiple Undef instructions of the same type, so
@@ -473,53 +481,65 @@
});
// Consider every instruction of the donor function.
- function_to_donate->ForEachInst(
- [&donated_instructions,
- &original_id_to_donated_id](const opt::Instruction* instruction) {
- // Get the instruction's input operands into donation-ready form,
- // remapping any id uses in the process.
- opt::Instruction::OperandList input_operands;
+ function_to_donate->ForEachInst([this, &donated_instructions,
+ &original_id_to_donated_id](
+ const opt::Instruction* instruction) {
+ // Get the instruction's input operands into donation-ready form,
+ // remapping any id uses in the process.
+ opt::Instruction::OperandList input_operands;
- // Consider each input operand in turn.
- for (uint32_t in_operand_index = 0;
- in_operand_index < instruction->NumInOperands();
- in_operand_index++) {
- std::vector<uint32_t> operand_data;
- const opt::Operand& in_operand =
- instruction->GetInOperand(in_operand_index);
- switch (in_operand.type) {
- case SPV_OPERAND_TYPE_ID:
- case SPV_OPERAND_TYPE_TYPE_ID:
- case SPV_OPERAND_TYPE_RESULT_ID:
- case SPV_OPERAND_TYPE_MEMORY_SEMANTICS_ID:
- case SPV_OPERAND_TYPE_SCOPE_ID:
- // This is an id operand - it consists of a single word of data,
- // which needs to be remapped so that it is replaced with the
- // donated form of the id.
- operand_data.push_back(
- original_id_to_donated_id->at(in_operand.words[0]));
- break;
- default:
- // For non-id operands, we just add each of the data words.
- for (auto word : in_operand.words) {
- operand_data.push_back(word);
- }
- break;
+ // Consider each input operand in turn.
+ for (uint32_t in_operand_index = 0;
+ in_operand_index < instruction->NumInOperands();
+ in_operand_index++) {
+ std::vector<uint32_t> operand_data;
+ const opt::Operand& in_operand =
+ instruction->GetInOperand(in_operand_index);
+ switch (in_operand.type) {
+ case SPV_OPERAND_TYPE_ID:
+ case SPV_OPERAND_TYPE_TYPE_ID:
+ case SPV_OPERAND_TYPE_RESULT_ID:
+ case SPV_OPERAND_TYPE_MEMORY_SEMANTICS_ID:
+ case SPV_OPERAND_TYPE_SCOPE_ID:
+ // This is an id operand - it consists of a single word of data,
+ // which needs to be remapped so that it is replaced with the
+ // donated form of the id.
+ operand_data.push_back(
+ original_id_to_donated_id->at(in_operand.words[0]));
+ break;
+ default:
+ // For non-id operands, we just add each of the data words.
+ for (auto word : in_operand.words) {
+ operand_data.push_back(word);
}
- input_operands.push_back({in_operand.type, operand_data});
- }
- // Remap the result type and result id (if present) of the
- // instruction, and turn it into a protobuf message.
- donated_instructions.push_back(MakeInstructionMessage(
- instruction->opcode(),
- instruction->type_id()
- ? original_id_to_donated_id->at(instruction->type_id())
- : 0,
- instruction->result_id()
- ? original_id_to_donated_id->at(instruction->result_id())
- : 0,
- input_operands));
- });
+ break;
+ }
+ input_operands.push_back({in_operand.type, operand_data});
+ }
+
+ if (instruction->opcode() == SpvOpVariable &&
+ instruction->NumInOperands() == 1) {
+ // This is an uninitialized local variable. Initialize it to zero.
+ input_operands.push_back(
+ {SPV_OPERAND_TYPE_ID,
+ {FindOrCreateZeroConstant(
+ fuzzerutil::GetPointeeTypeIdFromPointerType(
+ GetIRContext(),
+ original_id_to_donated_id->at(instruction->type_id())))}});
+ }
+
+ // Remap the result type and result id (if present) of the
+ // instruction, and turn it into a protobuf message.
+ donated_instructions.push_back(MakeInstructionMessage(
+ instruction->opcode(),
+ instruction->type_id()
+ ? original_id_to_donated_id->at(instruction->type_id())
+ : 0,
+ instruction->result_id()
+ ? original_id_to_donated_id->at(instruction->result_id())
+ : 0,
+ input_operands));
+ });
if (make_livesafe) {
// Various types and constants must be in place for a function to be made
diff --git a/source/fuzz/fuzzer_util.cpp b/source/fuzz/fuzzer_util.cpp
index 1af3fc8..cb90143 100644
--- a/source/fuzz/fuzzer_util.cpp
+++ b/source/fuzz/fuzzer_util.cpp
@@ -471,6 +471,36 @@
return found_parameter;
}
+uint32_t GetTypeId(opt::IRContext* context, uint32_t result_id) {
+ return context->get_def_use_mgr()->GetDef(result_id)->type_id();
+}
+
+uint32_t GetPointeeTypeIdFromPointerType(opt::Instruction* pointer_type_inst) {
+ assert(pointer_type_inst && pointer_type_inst->opcode() == SpvOpTypePointer &&
+ "Precondition: |pointer_type_inst| must be OpTypePointer.");
+ return pointer_type_inst->GetSingleWordInOperand(1);
+}
+
+uint32_t GetPointeeTypeIdFromPointerType(opt::IRContext* context,
+ uint32_t pointer_type_id) {
+ return GetPointeeTypeIdFromPointerType(
+ context->get_def_use_mgr()->GetDef(pointer_type_id));
+}
+
+SpvStorageClass GetStorageClassFromPointerType(
+ opt::Instruction* pointer_type_inst) {
+ assert(pointer_type_inst && pointer_type_inst->opcode() == SpvOpTypePointer &&
+ "Precondition: |pointer_type_inst| must be OpTypePointer.");
+ return static_cast<SpvStorageClass>(
+ pointer_type_inst->GetSingleWordInOperand(0));
+}
+
+SpvStorageClass GetStorageClassFromPointerType(opt::IRContext* context,
+ uint32_t pointer_type_id) {
+ return GetStorageClassFromPointerType(
+ context->get_def_use_mgr()->GetDef(pointer_type_id));
+}
+
} // namespace fuzzerutil
} // namespace fuzz
diff --git a/source/fuzz/fuzzer_util.h b/source/fuzz/fuzzer_util.h
index 3e9c691..292cd9f 100644
--- a/source/fuzz/fuzzer_util.h
+++ b/source/fuzz/fuzzer_util.h
@@ -158,6 +158,29 @@
bool InstructionIsFunctionParameter(opt::Instruction* instruction,
opt::Function* function);
+// Returns the type id of the instruction defined by |result_id|, or 0 if there
+// is no such result id.
+uint32_t GetTypeId(opt::IRContext* context, uint32_t result_id);
+
+// Given |pointer_type_inst|, which must be an OpTypePointer instruction,
+// returns the id of the associated pointee type.
+uint32_t GetPointeeTypeIdFromPointerType(opt::Instruction* pointer_type_inst);
+
+// Given |pointer_type_id|, which must be the id of a pointer type, returns the
+// id of the associated pointee type.
+uint32_t GetPointeeTypeIdFromPointerType(opt::IRContext* context,
+ uint32_t pointer_type_id);
+
+// Given |pointer_type_inst|, which must be an OpTypePointer instruction,
+// returns the associated storage class.
+SpvStorageClass GetStorageClassFromPointerType(
+ opt::Instruction* pointer_type_inst);
+
+// Given |pointer_type_id|, which must be the id of a pointer type, returns the
+// associated storage class.
+SpvStorageClass GetStorageClassFromPointerType(opt::IRContext* context,
+ uint32_t pointer_type_id);
+
} // namespace fuzzerutil
} // namespace fuzz
diff --git a/source/fuzz/protobufs/spvtoolsfuzz.proto b/source/fuzz/protobufs/spvtoolsfuzz.proto
index 08af3e0..7cd59ac 100644
--- a/source/fuzz/protobufs/spvtoolsfuzz.proto
+++ b/source/fuzz/protobufs/spvtoolsfuzz.proto
@@ -505,7 +505,7 @@
// The type of the global variable
uint32 type_id = 2;
- // Optional initializer; 0 if there is no initializer
+ // Initial value of the variable
uint32 initializer_id = 3;
// True if and only if the behaviour of the module should not depend on the
diff --git a/source/fuzz/transformation_add_global_variable.cpp b/source/fuzz/transformation_add_global_variable.cpp
index 7af5888..e4f9f7a 100644
--- a/source/fuzz/transformation_add_global_variable.cpp
+++ b/source/fuzz/transformation_add_global_variable.cpp
@@ -53,21 +53,19 @@
if (pointer_type->storage_class() != SpvStorageClassPrivate) {
return false;
}
- if (message_.initializer_id()) {
- // The initializer id must be the id of a constant. Check this with the
- // constant manager.
- auto constant_id = context->get_constant_mgr()->GetConstantsFromIds(
- {message_.initializer_id()});
- if (constant_id.empty()) {
- return false;
- }
- assert(constant_id.size() == 1 &&
- "We asked for the constant associated with a single id; we should "
- "get a single constant.");
- // The type of the constant must match the pointee type of the pointer.
- if (pointer_type->pointee_type() != constant_id[0]->type()) {
- return false;
- }
+ // The initializer id must be the id of a constant. Check this with the
+ // constant manager.
+ auto constant_id = context->get_constant_mgr()->GetConstantsFromIds(
+ {message_.initializer_id()});
+ if (constant_id.empty()) {
+ return false;
+ }
+ assert(constant_id.size() == 1 &&
+ "We asked for the constant associated with a single id; we should "
+ "get a single constant.");
+ // The type of the constant must match the pointee type of the pointer.
+ if (pointer_type->pointee_type() != constant_id[0]->type()) {
+ return false;
}
return true;
}
diff --git a/source/fuzz/transformation_load.cpp b/source/fuzz/transformation_load.cpp
index ab6b8ac..4cba37d 100644
--- a/source/fuzz/transformation_load.cpp
+++ b/source/fuzz/transformation_load.cpp
@@ -82,11 +82,8 @@
void TransformationLoad::Apply(opt::IRContext* context,
spvtools::fuzz::FactManager* /*unused*/) const {
- uint32_t result_type = context->get_def_use_mgr()
- ->GetDef(context->get_def_use_mgr()
- ->GetDef(message_.pointer_id())
- ->type_id())
- ->GetSingleWordInOperand(1);
+ uint32_t result_type = fuzzerutil::GetPointeeTypeIdFromPointerType(
+ context, fuzzerutil::GetTypeId(context, message_.pointer_id()));
fuzzerutil::UpdateModuleIdBound(context, message_.fresh_id());
FindInstruction(message_.instruction_to_insert_before(), context)
->InsertBefore(MakeUnique<opt::Instruction>(
diff --git a/test/fuzz/fuzzer_pass_donate_modules_test.cpp b/test/fuzz/fuzzer_pass_donate_modules_test.cpp
index 0d202b7..dc7ba3a 100644
--- a/test/fuzz/fuzzer_pass_donate_modules_test.cpp
+++ b/test/fuzz/fuzzer_pass_donate_modules_test.cpp
@@ -314,14 +314,17 @@
%100 = OpTypePointer Function %6
%101 = OpTypeStruct %6
%102 = OpTypePointer Private %101
- %103 = OpVariable %102 Private
- %104 = OpConstant %12 0
- %105 = OpTypePointer Private %6
- %106 = OpTypePointer Function %12
- %107 = OpTypeStruct %12
- %108 = OpTypePointer Private %107
- %109 = OpVariable %108 Private
- %110 = OpTypePointer Private %12
+ %104 = OpConstant %6 0
+ %105 = OpConstantComposite %101 %104
+ %103 = OpVariable %102 Private %105
+ %106 = OpConstant %12 0
+ %107 = OpTypePointer Private %6
+ %108 = OpTypePointer Function %12
+ %109 = OpTypeStruct %12
+ %110 = OpTypePointer Private %109
+ %112 = OpConstantComposite %109 %13
+ %111 = OpVariable %110 Private %112
+ %113 = OpTypePointer Private %12
%4 = OpFunction %2 None %3
%5 = OpLabel
%8 = OpVariable %7 Function
@@ -334,16 +337,16 @@
OpStore %18 %24
OpReturn
OpFunctionEnd
- %111 = OpFunction %2 None %3
- %112 = OpLabel
- %113 = OpVariable %100 Function
- %114 = OpVariable %106 Function
- %115 = OpAccessChain %105 %103 %104
- %116 = OpLoad %6 %115
- OpStore %113 %116
- %117 = OpAccessChain %110 %109 %104
- %118 = OpLoad %12 %117
- OpStore %114 %118
+ %114 = OpFunction %2 None %3
+ %115 = OpLabel
+ %116 = OpVariable %100 Function %104
+ %117 = OpVariable %108 Function %13
+ %118 = OpAccessChain %107 %103 %106
+ %119 = OpLoad %6 %118
+ OpStore %116 %119
+ %120 = OpAccessChain %113 %111 %106
+ %121 = OpLoad %12 %120
+ OpStore %117 %121
OpReturn
OpFunctionEnd
)";
@@ -420,19 +423,21 @@
%10 = OpTypePointer Input %7
%11 = OpVariable %10 Input
%100 = OpTypePointer Private %7
- %101 = OpVariable %100 Private
- %102 = OpTypePointer Private %7
- %103 = OpVariable %102 Private
+ %102 = OpConstant %6 0
+ %103 = OpConstantComposite %7 %102 %102 %102 %102
+ %101 = OpVariable %100 Private %103
+ %104 = OpTypePointer Private %7
+ %105 = OpVariable %104 Private %103
%4 = OpFunction %2 None %3
%5 = OpLabel
%12 = OpLoad %7 %11
OpStore %9 %12
OpReturn
OpFunctionEnd
- %104 = OpFunction %2 None %3
- %105 = OpLabel
- %106 = OpLoad %7 %103
- OpStore %101 %106
+ %106 = OpFunction %2 None %3
+ %107 = OpLabel
+ %108 = OpLoad %7 %105
+ OpStore %101 %108
OpReturn
OpFunctionEnd
)";
diff --git a/test/fuzz/transformation_add_global_variable_test.cpp b/test/fuzz/transformation_add_global_variable_test.cpp
index d43a2ae..619f068 100644
--- a/test/fuzz/transformation_add_global_variable_test.cpp
+++ b/test/fuzz/transformation_add_global_variable_test.cpp
@@ -30,8 +30,10 @@
%2 = OpTypeVoid
%3 = OpTypeFunction %2
%6 = OpTypeFloat 32
+ %40 = OpConstant %6 0
%7 = OpTypeInt 32 1
%8 = OpTypeVector %6 2
+ %41 = OpConstantComposite %8 %40 %40
%9 = OpTypePointer Function %6
%10 = OpTypePointer Private %6
%20 = OpTypePointer Uniform %6
@@ -104,13 +106,13 @@
TransformationAddGlobalVariable transformations[] = {
// %100 = OpVariable %12 Private
- TransformationAddGlobalVariable(100, 12, 0, true),
+ TransformationAddGlobalVariable(100, 12, 16, true),
// %101 = OpVariable %10 Private
- TransformationAddGlobalVariable(101, 10, 0, false),
+ TransformationAddGlobalVariable(101, 10, 40, false),
// %102 = OpVariable %13 Private
- TransformationAddGlobalVariable(102, 13, 0, true),
+ TransformationAddGlobalVariable(102, 13, 41, true),
// %103 = OpVariable %12 Private %16
TransformationAddGlobalVariable(103, 12, 16, false),
@@ -144,8 +146,10 @@
%2 = OpTypeVoid
%3 = OpTypeFunction %2
%6 = OpTypeFloat 32
+ %40 = OpConstant %6 0
%7 = OpTypeInt 32 1
%8 = OpTypeVector %6 2
+ %41 = OpConstantComposite %8 %40 %40
%9 = OpTypePointer Function %6
%10 = OpTypePointer Private %6
%20 = OpTypePointer Uniform %6
@@ -160,9 +164,9 @@
%19 = OpTypePointer Private %18
%21 = OpConstantTrue %18
%22 = OpConstantFalse %18
- %100 = OpVariable %12 Private
- %101 = OpVariable %10 Private
- %102 = OpVariable %13 Private
+ %100 = OpVariable %12 Private %16
+ %101 = OpVariable %10 Private %40
+ %102 = OpVariable %13 Private %41
%103 = OpVariable %12 Private %16
%104 = OpVariable %19 Private %21
%105 = OpVariable %19 Private %22
@@ -222,7 +226,7 @@
TransformationAddGlobalVariable transformations[] = {
// %100 = OpVariable %12 Private
- TransformationAddGlobalVariable(100, 12, 0, true),
+ TransformationAddGlobalVariable(100, 12, 16, true),
// %101 = OpVariable %12 Private %16
TransformationAddGlobalVariable(101, 12, 16, false),
@@ -265,7 +269,7 @@
%18 = OpTypeBool
%19 = OpTypePointer Private %18
%21 = OpConstantTrue %18
- %100 = OpVariable %12 Private
+ %100 = OpVariable %12 Private %16
%101 = OpVariable %12 Private %16
%102 = OpVariable %19 Private %21
%4 = OpFunction %2 None %3
