[tint][ir] Check result and operand Types for nullness
Fixes: 355409306
Change-Id: I03b13eb1cf2aea0e4f41b58b9d5bd59344c4a0bc
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/200059
Reviewed-by: James Price <jrprice@google.com>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: James Price <jrprice@google.com>
diff --git a/src/tint/lang/core/ir/validator.cc b/src/tint/lang/core/ir/validator.cc
index 01e6a56..a932edc 100644
--- a/src/tint/lang/core/ir/validator.cc
+++ b/src/tint/lang/core/ir/validator.cc
@@ -297,26 +297,27 @@
/// @returns the styled name for the given block
StyledText NameOf(const Block* block);
- /// Checks the given result is not null
+ /// Checks the given result is not null and its type is not null
/// @param inst the instruction
/// @param idx the result index
/// @returns true if the result is not null
- bool CheckResultNotNull(const ir::Instruction* inst, size_t idx);
+ bool CheckResult(const Instruction* inst, size_t idx);
/// Checks the number of results for @p inst are exactly equal to @p count and that none of
- /// them are null.
+ /// them are null. Also checks that the types for the results are not null
/// @param inst the instruction
/// @param count the number of results to check
/// @returns true if the results count is as expected and none are null
bool CheckResults(const ir::Instruction* inst, size_t count);
- /// Checks the given operand is not null
+ /// Checks the given operand is not null and its type is not null
/// @param inst the instruction
/// @param idx the operand index
/// @returns true if the operand is not null
- bool CheckOperandNotNull(const ir::Instruction* inst, size_t idx);
+ bool CheckOperand(const Instruction* inst, size_t idx);
- /// Checks the number of operands provided to @p inst and that none of them are null.
+ /// Checks the number of operands provided to @p inst and that none of them are null. Also
+ /// checks that the types for the operands are not null
/// @param inst the instruction
/// @param min_count the minimum number of operands to expect
/// @param max_count the maximum number of operands to expect, if not set, than only the minimum
@@ -327,7 +328,7 @@
std::optional<size_t> max_count);
/// Checks the number of operands for @p inst are exactly equal to @p count and that none of
- /// them are null.
+ /// them are null. Also checks that the types for the operands are not null
/// @param inst the instruction
/// @param count the number of operands to check
/// @returns true if the operands count is as expected and none are null
@@ -791,12 +792,18 @@
<< Disassemble().NameOf(block);
}
-bool Validator::CheckResultNotNull(const Instruction* inst, size_t idx) {
+bool Validator::CheckResult(const Instruction* inst, size_t idx) {
auto* result = inst->Result(idx);
if (TINT_UNLIKELY(result == nullptr)) {
AddResultError(inst, idx) << "result is undefined";
return false;
}
+
+ if (TINT_UNLIKELY(result->Type() == nullptr)) {
+ AddResultError(inst, idx) << "result type is undefined";
+ return false;
+ }
+
return true;
}
@@ -809,19 +816,27 @@
bool passed = true;
for (size_t i = 0; i < count; i++) {
- if (TINT_UNLIKELY(!CheckResultNotNull(inst, i))) {
+ if (TINT_UNLIKELY(!CheckResult(inst, i))) {
passed = false;
}
}
return passed;
}
-bool Validator::CheckOperandNotNull(const Instruction* inst, size_t idx) {
+bool Validator::CheckOperand(const Instruction* inst, size_t idx) {
auto* operand = inst->Operand(idx);
if (TINT_UNLIKELY(operand == nullptr)) {
AddError(inst, idx) << "operand is undefined";
return false;
}
+
+ // ir::Function does not have a meaningful type, so does not override the default Type()
+ // behaviour.
+ if (TINT_UNLIKELY(!operand->Is<ir::Function>() && operand->Type() == nullptr)) {
+ AddError(inst, idx) << "operand type is undefined";
+ return false;
+ }
+
return true;
}
@@ -847,7 +862,7 @@
bool passed = true;
for (size_t i = 0; i < inst->Operands().Length(); i++) {
- if (TINT_UNLIKELY(!CheckOperandNotNull(inst, i))) {
+ if (TINT_UNLIKELY(!CheckOperand(inst, i))) {
passed = false;
}
}
@@ -863,7 +878,7 @@
bool passed = true;
for (size_t i = 0; i < count; i++) {
- if (TINT_UNLIKELY(!CheckOperandNotNull(inst, i))) {
+ if (TINT_UNLIKELY(!CheckOperand(inst, i))) {
passed = false;
}
}
@@ -1139,16 +1154,15 @@
// Check that initializer and result type match
if (var->Initializer()) {
+ if (!CheckOperand(var, ir::Var::kInitializerOperandOffset)) {
+ return;
+ }
+
if (var->Initializer()->Type() != var->Result(0)->Type()->UnwrapPtrOrRef()) {
AddError(var) << "initializer type "
- << style::Type(var->Initializer()->Type()
- ? var->Initializer()->Type()->FriendlyName()
- : "undef")
+ << style::Type(var->Initializer()->Type()->FriendlyName())
<< " does not match store type "
- << style::Type(
- var->Result(0)->Type()
- ? var->Result(0)->Type()->UnwrapPtrOrRef()->FriendlyName()
- : "undef");
+ << style::Type(var->Result(0)->Type()->UnwrapPtrOrRef()->FriendlyName());
return;
}
}
@@ -1830,14 +1844,7 @@
}
auto* value_type = from->Type();
auto* store_type = mv->StoreType();
-
- if (!store_type) {
- AddError(s, Store::kToOperandOffset) << "store type must not be null";
- }
- if (!value_type) {
- AddError(s, Store::kFromOperandOffset) << "value type must not be null";
- }
- if (store_type && value_type && value_type != store_type) {
+ if (value_type != store_type) {
AddError(s, Store::kFromOperandOffset)
<< "value type " << style::Type(value_type->FriendlyName())
<< " does not match store type " << style::Type(store_type->FriendlyName());
diff --git a/src/tint/lang/core/ir/validator_test.cc b/src/tint/lang/core/ir/validator_test.cc
index 0e92fb2..538738d 100644
--- a/src/tint/lang/core/ir/validator_test.cc
+++ b/src/tint/lang/core/ir/validator_test.cc
@@ -2302,6 +2302,51 @@
)");
}
+TEST_F(IR_ValidatorTest, Var_Init_NullType) {
+ auto* f = b.Function("my_func", ty.void_());
+
+ b.Append(f->Block(), [&] {
+ auto* i = b.Var<function, f32>("i");
+ i->SetInitializer(b.Constant(0_f));
+ auto* load = b.Load(i);
+ auto* load_ret = load->Result(0);
+ auto* j = b.Var<function, f32>("j");
+ j->SetInitializer(load_ret);
+ load_ret->SetType(nullptr);
+ b.Return(f);
+ });
+
+ auto res = ir::Validate(mod);
+ ASSERT_NE(res, Success);
+ EXPECT_EQ(res.Failure().reason.Str(),
+ R"(:4:5 error: load: result type is undefined
+ %3:null = load %i
+ ^^^^^^^
+
+:2:3 note: in block
+ $B1: {
+ ^^^
+
+:5:46 error: var: operand type is undefined
+ %j:ptr<function, f32, read_write> = var, %3
+ ^^
+
+:2:3 note: in block
+ $B1: {
+ ^^^
+
+note: # Disassembly
+%my_func = func():void {
+ $B1: {
+ %i:ptr<function, f32, read_write> = var, 0.0f
+ %3:null = load %i
+ %j:ptr<function, f32, read_write> = var, %3
+ ret
+ }
+}
+)");
+}
+
TEST_F(IR_ValidatorTest, Var_HandleMissingBindingPoint) {
auto* v = b.Var(ty.ptr<handle, i32>());
mod.root_block->Append(v);
@@ -5691,7 +5736,7 @@
$B1: {
^^^
-:3:11 error: store: store target operand is not a memory view
+:3:11 error: store: operand type is undefined
store %2, 42u
^^
@@ -5724,7 +5769,7 @@
auto res = ir::Validate(mod);
ASSERT_NE(res, Success);
EXPECT_EQ(res.Failure().reason.Str(),
- R"(:5:15 error: store: value type must not be null
+ R"(:5:15 error: store: operand type is undefined
store %2, %3
^^
