spirv-fuzz: Tighten checks on null and undef pointers (#4367)
Adaps the transformations that add OpConstantUndef and OpConstantNull
to a module so that pointer undefs are not allowed, and null pointers
are only allowed if suitable capabilities are present.
Fixes #4357.
diff --git a/source/fuzz/fuzzer_util.cpp b/source/fuzz/fuzzer_util.cpp
index eaa6225..0e718e1 100644
--- a/source/fuzz/fuzzer_util.cpp
+++ b/source/fuzz/fuzzer_util.cpp
@@ -787,11 +787,38 @@
return absolute_index - inst.NumOperands() + inst.NumInOperands();
}
-bool IsNullConstantSupported(const opt::analysis::Type& type) {
- return type.AsBool() || type.AsInteger() || type.AsFloat() ||
- type.AsMatrix() || type.AsVector() || type.AsArray() ||
- type.AsStruct() || type.AsPointer() || type.AsEvent() ||
- type.AsDeviceEvent() || type.AsReserveId() || type.AsQueue();
+bool IsNullConstantSupported(opt::IRContext* ir_context,
+ const opt::Instruction& type_inst) {
+ switch (type_inst.opcode()) {
+ case SpvOpTypeArray:
+ case SpvOpTypeBool:
+ case SpvOpTypeDeviceEvent:
+ case SpvOpTypeEvent:
+ case SpvOpTypeFloat:
+ case SpvOpTypeInt:
+ case SpvOpTypeMatrix:
+ case SpvOpTypeQueue:
+ case SpvOpTypeReserveId:
+ case SpvOpTypeVector:
+ case SpvOpTypeStruct:
+ return true;
+ case SpvOpTypePointer:
+ // Null pointers are allowed if the VariablePointers capability is
+ // enabled, or if the VariablePointersStorageBuffer capability is enabled
+ // and the pointer type has StorageBuffer as its storage class.
+ if (ir_context->get_feature_mgr()->HasCapability(
+ SpvCapabilityVariablePointers)) {
+ return true;
+ }
+ if (ir_context->get_feature_mgr()->HasCapability(
+ SpvCapabilityVariablePointersStorageBuffer)) {
+ return type_inst.GetSingleWordInOperand(0) ==
+ SpvStorageClassStorageBuffer;
+ }
+ return false;
+ default:
+ return false;
+ }
}
bool GlobalVariablesMustBeDeclaredInEntryPointInterfaces(
diff --git a/source/fuzz/fuzzer_util.h b/source/fuzz/fuzzer_util.h
index b956507..a394661 100644
--- a/source/fuzz/fuzzer_util.h
+++ b/source/fuzz/fuzzer_util.h
@@ -273,8 +273,10 @@
uint32_t absolute_index);
// Returns true if and only if |type| is one of the types for which it is legal
-// to have an OpConstantNull value.
-bool IsNullConstantSupported(const opt::analysis::Type& type);
+// to have an OpConstantNull value. This may depend on the capabilities declared
+// in |context|.
+bool IsNullConstantSupported(opt::IRContext* context,
+ const opt::Instruction& type);
// Returns true if and only if the SPIR-V version being used requires that
// global variables accessed in the static call graph of an entry point need
diff --git a/source/fuzz/transformation_add_constant_null.cpp b/source/fuzz/transformation_add_constant_null.cpp
index 32544e6..c0f7367 100644
--- a/source/fuzz/transformation_add_constant_null.cpp
+++ b/source/fuzz/transformation_add_constant_null.cpp
@@ -35,14 +35,14 @@
if (!fuzzerutil::IsFreshId(context, message_.fresh_id())) {
return false;
}
- auto type = context->get_type_mgr()->GetType(message_.type_id());
+ auto type = context->get_def_use_mgr()->GetDef(message_.type_id());
// The type must exist.
if (!type) {
return false;
}
// The type must be one of the types for which null constants are allowed,
// according to the SPIR-V spec.
- return fuzzerutil::IsNullConstantSupported(*type);
+ return fuzzerutil::IsNullConstantSupported(context, *type);
}
void TransformationAddConstantNull::Apply(
diff --git a/source/fuzz/transformation_add_global_undef.cpp b/source/fuzz/transformation_add_global_undef.cpp
index eb390ea..ec0574a 100644
--- a/source/fuzz/transformation_add_global_undef.cpp
+++ b/source/fuzz/transformation_add_global_undef.cpp
@@ -15,6 +15,7 @@
#include "source/fuzz/transformation_add_global_undef.h"
#include "source/fuzz/fuzzer_util.h"
+#include "source/opt/reflect.h"
namespace spvtools {
namespace fuzz {
@@ -35,9 +36,11 @@
if (!fuzzerutil::IsFreshId(ir_context, message_.fresh_id())) {
return false;
}
- auto type = ir_context->get_type_mgr()->GetType(message_.type_id());
- // The type must exist, and must not be a function type.
- return type && !type->AsFunction();
+ auto type = ir_context->get_def_use_mgr()->GetDef(message_.type_id());
+ // The type must exist, and must not be a function or pointer type.
+ return type != nullptr && opt::IsTypeInst(type->opcode()) &&
+ type->opcode() != SpvOpTypeFunction &&
+ type->opcode() != SpvOpTypePointer;
}
void TransformationAddGlobalUndef::Apply(
diff --git a/source/fuzz/transformation_add_global_undef.h b/source/fuzz/transformation_add_global_undef.h
index 37542c3..fff1ad9 100644
--- a/source/fuzz/transformation_add_global_undef.h
+++ b/source/fuzz/transformation_add_global_undef.h
@@ -31,7 +31,7 @@
TransformationAddGlobalUndef(uint32_t fresh_id, uint32_t type_id);
// - |message_.fresh_id| must be fresh
- // - |message_.type_id| must be the id of a non-function type
+ // - |message_.type_id| must be the id of a non-function, non-pointer type
bool IsApplicable(
opt::IRContext* ir_context,
const TransformationContext& transformation_context) const override;
diff --git a/test/fuzz/fuzzer_pass_donate_modules_test.cpp b/test/fuzz/fuzzer_pass_donate_modules_test.cpp
index f11885d..a8e4ace 100644
--- a/test/fuzz/fuzzer_pass_donate_modules_test.cpp
+++ b/test/fuzz/fuzzer_pass_donate_modules_test.cpp
@@ -531,6 +531,7 @@
std::string recipient_shader = R"(
OpCapability Shader
OpCapability ImageQuery
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main"
@@ -548,6 +549,7 @@
std::string donor_shader = R"(
OpCapability Shader
OpCapability ImageQuery
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main"
diff --git a/test/fuzz/fuzzerutil_test.cpp b/test/fuzz/fuzzerutil_test.cpp
index 7ff9b4d..6771c40 100644
--- a/test/fuzz/fuzzerutil_test.cpp
+++ b/test/fuzz/fuzzerutil_test.cpp
@@ -196,8 +196,6 @@
%50 = OpTypePointer Private %7
%34 = OpTypeBool
%35 = OpConstantFalse %34
- %60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24
@@ -507,8 +505,6 @@
%50 = OpTypePointer Private %7
%34 = OpTypeBool
%35 = OpConstantFalse %34
- %60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24
@@ -799,8 +795,6 @@
%50 = OpTypePointer Private %7
%34 = OpTypeBool
%35 = OpConstantFalse %34
- %60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24
@@ -894,8 +888,6 @@
%50 = OpTypePointer Private %7
%34 = OpTypeBool
%35 = OpConstantFalse %34
- %60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24
@@ -1165,8 +1157,6 @@
%50 = OpTypePointer Private %7
%34 = OpTypeBool
%35 = OpConstantFalse %34
- %60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24
@@ -1261,8 +1251,6 @@
%50 = OpTypePointer Private %7
%34 = OpTypeBool
%35 = OpConstantFalse %34
- %60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24
@@ -1362,8 +1350,6 @@
%50 = OpTypePointer Private %7
%34 = OpTypeBool
%35 = OpConstantFalse %34
- %60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24
diff --git a/test/fuzz/transformation_access_chain_test.cpp b/test/fuzz/transformation_access_chain_test.cpp
index e791981..bddcf5f 100644
--- a/test/fuzz/transformation_access_chain_test.cpp
+++ b/test/fuzz/transformation_access_chain_test.cpp
@@ -26,6 +26,7 @@
TEST(TransformationAccessChainTest, BasicTest) {
std::string shader = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main" %48 %54
@@ -63,7 +64,6 @@
%85 = OpConstant %10 5
%52 = OpTypeArray %50 %51
%53 = OpTypePointer Private %52
- %45 = OpUndef %9
%46 = OpConstantNull %9
%47 = OpTypePointer Private %8
%48 = OpVariable %47 Private
@@ -204,15 +204,6 @@
#ifndef NDEBUG
// Bad: pointer is null
ASSERT_DEATH(
- TransformationAccessChain(100, 45, {80},
- MakeInstructionDescriptor(24, SpvOpLoad, 0))
- .IsApplicable(context.get(), transformation_context),
- "Access chains should not be created from null/undefined pointers");
-#endif
-
-#ifndef NDEBUG
- // Bad: pointer is undef
- ASSERT_DEATH(
TransformationAccessChain(100, 46, {80},
MakeInstructionDescriptor(24, SpvOpLoad, 0))
.IsApplicable(context.get(), transformation_context),
@@ -331,6 +322,7 @@
std::string after_transformation = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main" %48 %54
@@ -368,7 +360,6 @@
%85 = OpConstant %10 5
%52 = OpTypeArray %50 %51
%53 = OpTypePointer Private %52
- %45 = OpUndef %9
%46 = OpConstantNull %9
%47 = OpTypePointer Private %8
%48 = OpVariable %47 Private
diff --git a/test/fuzz/transformation_add_copy_memory_test.cpp b/test/fuzz/transformation_add_copy_memory_test.cpp
index 642a556..ff8ac72 100644
--- a/test/fuzz/transformation_add_copy_memory_test.cpp
+++ b/test/fuzz/transformation_add_copy_memory_test.cpp
@@ -26,6 +26,7 @@
TEST(TransformationAddCopyMemoryTest, BasicTest) {
std::string shader = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main"
@@ -64,7 +65,6 @@
%67 = OpTypePointer Function %66
%83 = OpTypePointer Private %66
%86 = OpVariable %79 Private %20
- %87 = OpUndef %79
%88 = OpConstantNull %79
%4 = OpFunction %2 None %3
%5 = OpLabel
@@ -182,12 +182,6 @@
90, 40, SpvStorageClassPrivate, 0)
.IsApplicable(context.get(), transformation_context));
- // Source instruction is OpUndef.
- ASSERT_FALSE(
- TransformationAddCopyMemory(MakeInstructionDescriptor(41, SpvOpLoad, 0),
- 90, 87, SpvStorageClassPrivate, 0)
- .IsApplicable(context.get(), transformation_context));
-
// Source instruction is OpConstantNull.
ASSERT_FALSE(
TransformationAddCopyMemory(MakeInstructionDescriptor(41, SpvOpLoad, 0),
@@ -255,6 +249,7 @@
std::string expected = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main"
@@ -293,7 +288,6 @@
%67 = OpTypePointer Function %66
%83 = OpTypePointer Private %66
%86 = OpVariable %79 Private %20
- %87 = OpUndef %79
%88 = OpConstantNull %79
%90 = OpVariable %79 Private %20
%92 = OpVariable %78 Private %25
diff --git a/test/fuzz/transformation_add_synonym_test.cpp b/test/fuzz/transformation_add_synonym_test.cpp
index 314b003..ffcf1c9 100644
--- a/test/fuzz/transformation_add_synonym_test.cpp
+++ b/test/fuzz/transformation_add_synonym_test.cpp
@@ -1242,9 +1242,10 @@
ASSERT_TRUE(IsEqual(env, after_transformation, context.get()));
}
-TEST(TransformationAddSynonymTest, DoNotCopyNullOrUndefPointers) {
+TEST(TransformationAddSynonymTest, DoNotCopyNullPointers) {
std::string shader = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main"
@@ -1255,7 +1256,6 @@
%6 = OpTypeInt 32 1
%7 = OpTypePointer Function %6
%8 = OpConstantNull %7
- %9 = OpUndef %7
%4 = OpFunction %2 None %3
%5 = OpLabel
OpReturn
@@ -1275,12 +1275,6 @@
8, protobufs::TransformationAddSynonym::COPY_OBJECT, 100,
MakeInstructionDescriptor(5, SpvOpReturn, 0))
.IsApplicable(context.get(), transformation_context));
-
- // Illegal to copy an OpUndef of pointer type.
- ASSERT_FALSE(TransformationAddSynonym(
- 9, protobufs::TransformationAddSynonym::COPY_OBJECT, 100,
- MakeInstructionDescriptor(5, SpvOpReturn, 0))
- .IsApplicable(context.get(), transformation_context));
}
TEST(TransformationAddSynonymTest, PropagateIrrelevantPointeeFact) {
diff --git a/test/fuzz/transformation_load_test.cpp b/test/fuzz/transformation_load_test.cpp
index a03ffdd..3b6587f 100644
--- a/test/fuzz/transformation_load_test.cpp
+++ b/test/fuzz/transformation_load_test.cpp
@@ -26,6 +26,7 @@
TEST(TransformationLoadTest, BasicTest) {
std::string shader = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main"
@@ -49,7 +50,6 @@
%34 = OpTypeBool
%35 = OpConstantFalse %34
%60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%4 = OpFunction %2 None %3
@@ -171,10 +171,6 @@
100, 60, MakeInstructionDescriptor(38, SpvOpAccessChain, 0))
.IsApplicable(context.get(), transformation_context));
- // Bad: attempt to load from undefined pointer
- ASSERT_FALSE(TransformationLoad(
- 100, 61, MakeInstructionDescriptor(38, SpvOpAccessChain, 0))
- .IsApplicable(context.get(), transformation_context));
// Bad: %40 is not available at the program point
ASSERT_FALSE(
TransformationLoad(100, 40, MakeInstructionDescriptor(37, SpvOpReturn, 0))
@@ -231,6 +227,7 @@
std::string after_transformation = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main"
@@ -254,7 +251,6 @@
%34 = OpTypeBool
%35 = OpConstantFalse %34
%60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%4 = OpFunction %2 None %3
diff --git a/test/fuzz/transformation_mutate_pointer_test.cpp b/test/fuzz/transformation_mutate_pointer_test.cpp
index ae42310..e869efa 100644
--- a/test/fuzz/transformation_mutate_pointer_test.cpp
+++ b/test/fuzz/transformation_mutate_pointer_test.cpp
@@ -26,6 +26,7 @@
TEST(TransformationMutatePointerTest, BasicTest) {
std::string shader = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main"
@@ -60,7 +61,6 @@
%23 = OpTypePointer Output %6
%24 = OpVariable %23 Output
%27 = OpTypeFunction %2 %13
- %32 = OpUndef %16
%33 = OpConstantNull %16
%4 = OpFunction %2 None %3
%5 = OpLabel
@@ -110,10 +110,6 @@
ASSERT_FALSE(TransformationMutatePointer(11, 70, insert_before)
.IsApplicable(context.get(), transformation_context));
- // |pointer_id| is a result id of OpUndef.
- ASSERT_FALSE(TransformationMutatePointer(32, 70, insert_before)
- .IsApplicable(context.get(), transformation_context));
-
// |pointer_id| is a result id of OpConstantNull.
ASSERT_FALSE(TransformationMutatePointer(33, 70, insert_before)
.IsApplicable(context.get(), transformation_context));
@@ -163,6 +159,7 @@
std::string after_transformation = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main"
@@ -197,7 +194,6 @@
%23 = OpTypePointer Output %6
%24 = OpVariable %23 Output
%27 = OpTypeFunction %2 %13
- %32 = OpUndef %16
%33 = OpConstantNull %16
%4 = OpFunction %2 None %3
%5 = OpLabel
diff --git a/test/fuzz/transformation_push_id_through_variable_test.cpp b/test/fuzz/transformation_push_id_through_variable_test.cpp
index 7487cab..b0fff58 100644
--- a/test/fuzz/transformation_push_id_through_variable_test.cpp
+++ b/test/fuzz/transformation_push_id_through_variable_test.cpp
@@ -26,6 +26,7 @@
TEST(TransformationPushIdThroughVariableTest, IsApplicable) {
std::string reference_shader = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main" %92 %52 %53
@@ -50,7 +51,6 @@
%34 = OpTypeBool
%35 = OpConstantFalse %34
%60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24
@@ -257,6 +257,7 @@
TEST(TransformationPushIdThroughVariableTest, Apply) {
std::string reference_shader = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main" %92 %52 %53
@@ -281,7 +282,6 @@
%34 = OpTypeBool
%35 = OpConstantFalse %34
%60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24
@@ -419,6 +419,7 @@
std::string variant_shader = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main" %92 %52 %53 %109 %111
@@ -443,7 +444,6 @@
%34 = OpTypeBool
%35 = OpConstantFalse %34
%60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24
@@ -519,6 +519,7 @@
TEST(TransformationPushIdThroughVariableTest, AddSynonymsForRelevantIds) {
std::string reference_shader = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main" %92 %52 %53
@@ -543,7 +544,6 @@
%34 = OpTypeBool
%35 = OpConstantFalse %34
%60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24
@@ -620,6 +620,7 @@
TEST(TransformationPushIdThroughVariableTest, DontAddSynonymsForIrrelevantIds) {
std::string reference_shader = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main" %92 %52 %53
@@ -644,7 +645,6 @@
%34 = OpTypeBool
%35 = OpConstantFalse %34
%60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24
diff --git a/test/fuzz/transformation_store_test.cpp b/test/fuzz/transformation_store_test.cpp
index 93257d0..ec24a73 100644
--- a/test/fuzz/transformation_store_test.cpp
+++ b/test/fuzz/transformation_store_test.cpp
@@ -26,6 +26,7 @@
TEST(TransformationStoreTest, BasicTest) {
std::string shader = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main" %92 %52 %53
@@ -50,7 +51,6 @@
%34 = OpTypeBool
%35 = OpConstantFalse %34
%60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24
@@ -290,6 +290,7 @@
std::string after_transformation = R"(
OpCapability Shader
+ OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main" %92 %52 %53
@@ -314,7 +315,6 @@
%34 = OpTypeBool
%35 = OpConstantFalse %34
%60 = OpConstantNull %50
- %61 = OpUndef %51
%52 = OpVariable %50 Private
%53 = OpVariable %51 Private
%80 = OpConstantComposite %8 %21 %24