Skip to content

Commit d0dfa74

Browse files
ben-claytonben-clayton
committed
[tint][ir] Add more validation
Check that: * Instruction::Block() match their parent block * UserCall::Target() is actually part of the module Change-Id: I200853dd292d9076a352a44df170580fcd4cd902 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/153121 Reviewed-by: James Price <[email protected]> Kokoro: Kokoro <[email protected]>
1 parent 5021631 commit d0dfa74

File tree

3 files changed

+147
-12
lines changed

3 files changed

+147
-12
lines changed

‎src/tint/lang/core/ir/disassembler.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,7 @@ void Disassembler::EmitValue(Value* val) {
413413
[&](ir::InstructionResult* rv) { out_ << "%" << IdOf(rv); },
414414
[&](ir::BlockParam* p) { out_ << "%" << IdOf(p) << ":" << p->Type()->FriendlyName(); },
415415
[&](ir::FunctionParam* p) { out_ << "%" << IdOf(p); },
416+
[&](ir::Function* f) { out_ << "%" << IdOf(f); },
416417
[&](Default) {
417418
if (val == nullptr) {
418419
out_ << "undef";
@@ -461,7 +462,8 @@ void Disassembler::EmitInstruction(Instruction* inst) {
461462
EmitValueWithType(uc);
462463
out_ << " = ";
463464
EmitInstructionName(uc);
464-
out_ << " %" << IdOf(uc->Target());
465+
out_ << " ";
466+
EmitOperand(uc, UserCall::kFunctionOperandOffset);
465467
if (!uc->Args().IsEmpty()) {
466468
out_ << ", ";
467469
}

‎src/tint/lang/core/ir/validator.cc‎

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,10 @@ class Validator {
190190
/// @param call the call to validate
191191
void CheckBuiltinCall(BuiltinCall* call);
192192

193+
/// Validates the given user call
194+
/// @param call the call to validate
195+
void CheckUserCall(UserCall* call);
196+
193197
/// Validates the given access
194198
/// @param a the access to validate
195199
void CheckAccess(ir::Access* a);
@@ -266,7 +270,7 @@ class Validator {
266270
diag::List diagnostics_;
267271
Disassembler dis_{mod_};
268272
Block* current_block_ = nullptr;
269-
Hashset<Function*, 4> seen_functions_;
273+
Hashset<Function*, 4> all_functions_;
270274
Vector<ControlInstruction*, 8> control_stack_;
271275

272276
void DisassembleIfNeeded();
@@ -286,6 +290,12 @@ void Validator::DisassembleIfNeeded() {
286290
Result<SuccessType> Validator::Run() {
287291
CheckRootBlock(mod_.root_block);
288292

293+
for (auto* func : mod_.functions) {
294+
if (!all_functions_.Add(func)) {
295+
AddError("function '" + Name(func) + "' added to module multiple times");
296+
}
297+
}
298+
289299
for (auto* func : mod_.functions) {
290300
CheckFunction(func);
291301
}
@@ -399,6 +409,12 @@ void Validator::CheckRootBlock(Block* blk) {
399409
TINT_SCOPED_ASSIGNMENT(current_block_, blk);
400410

401411
for (auto* inst : *blk) {
412+
if (inst->Block() != blk) {
413+
AddError(
414+
inst,
415+
InstError(inst, "instruction in root block does not have root block as parent"));
416+
continue;
417+
}
402418
auto* var = inst->As<ir::Var>();
403419
if (!var) {
404420
AddError(inst,
@@ -410,10 +426,6 @@ void Validator::CheckRootBlock(Block* blk) {
410426
}
411427

412428
void Validator::CheckFunction(Function* func) {
413-
if (!seen_functions_.Add(func)) {
414-
AddError("function '" + Name(func) + "' added to module multiple times");
415-
}
416-
417429
CheckBlock(func->Block());
418430
}
419431

@@ -425,6 +437,11 @@ void Validator::CheckBlock(Block* blk) {
425437
}
426438

427439
for (auto* inst : *blk) {
440+
if (inst->Block() != blk) {
441+
AddError(inst, InstError(inst, "block instruction does not have same block as parent"));
442+
AddNote(current_block_, "In block");
443+
continue;
444+
}
428445
if (inst->Is<ir::Terminator>() && inst != blk->Terminator()) {
429446
AddError(inst, "block: terminator which isn't the final instruction");
430447
continue;
@@ -467,11 +484,14 @@ void Validator::CheckInstruction(Instruction* inst) {
467484
// Note, a `nullptr` is a valid operand in some cases, like `var` so we can't just check
468485
// for `nullptr` here.
469486
if (!op->Alive()) {
470-
AddError(inst, i, InstError(inst, "instruction has operand which is not alive"));
487+
AddError(inst, i,
488+
InstError(inst, "instruction operand " + std::to_string(i) + " is not alive"));
471489
}
472490

473491
if (!op->Usages().Contains({inst, i})) {
474-
AddError(inst, i, InstError(inst, "instruction operand missing usage"));
492+
AddError(
493+
inst, i,
494+
InstError(inst, "instruction operand " + std::to_string(i) + " missing usage"));
475495
}
476496
}
477497

@@ -521,7 +541,7 @@ void Validator::CheckCall(Call* call) {
521541
[&](Construct*) {}, //
522542
[&](Convert*) {}, //
523543
[&](Discard*) {}, //
524-
[&](UserCall*) {}, //
544+
[&](UserCall* c) { CheckUserCall(c); }, //
525545
[&](Default) {
526546
// Validation of custom IR instructions
527547
});
@@ -540,6 +560,13 @@ void Validator::CheckBuiltinCall(BuiltinCall* call) {
540560
}
541561
}
542562

563+
void Validator::CheckUserCall(UserCall* call) {
564+
if (!all_functions_.Contains(call->Target())) {
565+
AddError(call, UserCall::kFunctionOperandOffset,
566+
InstError(call, "call target is not part of the module"));
567+
}
568+
}
569+
543570
void Validator::CheckAccess(ir::Access* a) {
544571
bool is_ptr = a->Object()->Type()->Is<core::type::Pointer>();
545572
auto* ty = a->Object()->Type()->UnwrapPtr();

‎src/tint/lang/core/ir/validator_test.cc‎

Lines changed: 109 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,39 @@ note: # Disassembly
7070
)");
7171
}
7272

73+
TEST_F(IR_ValidatorTest, RootBlock_VarBlockMismatch) {
74+
auto* var = b.Var(ty.ptr<private_, i32>());
75+
mod.root_block = b.RootBlock();
76+
mod.root_block->Append(var);
77+
78+
auto* f = b.Function("f", ty.void_());
79+
f->Block()->Append(b.Return(f));
80+
var->SetBlock(f->Block());
81+
82+
auto res = ir::Validate(mod);
83+
ASSERT_FALSE(res);
84+
EXPECT_EQ(res.Failure().reason.str(),
85+
R"(:2:38 error: var: instruction in root block does not have root block as parent
86+
%1:ptr<private, i32, read_write> = var
87+
^^^
88+
89+
:1:1 note: In block
90+
%b1 = block { # root
91+
^^^^^^^^^^^
92+
93+
note: # Disassembly
94+
%b1 = block { # root
95+
%1:ptr<private, i32, read_write> = var
96+
}
97+
98+
%f = func():void -> %b2 {
99+
%b2 = block {
100+
ret
101+
}
102+
}
103+
)");
104+
}
105+
73106
TEST_F(IR_ValidatorTest, Function) {
74107
auto* f = b.Function("my_func", ty.void_());
75108

@@ -106,6 +139,38 @@ note: # Disassembly
106139
)");
107140
}
108141

142+
TEST_F(IR_ValidatorTest, CallToFunctionOutsideModule) {
143+
auto* f = b.Function("f", ty.void_());
144+
auto* g = b.Function("g", ty.void_());
145+
mod.functions.Pop(); // Remove g
146+
147+
b.Append(f->Block(), [&] {
148+
b.Call(g);
149+
b.Return(f);
150+
});
151+
b.Append(g->Block(), [&] { b.Return(g); });
152+
153+
auto res = ir::Validate(mod);
154+
ASSERT_FALSE(res);
155+
EXPECT_EQ(res.Failure().reason.str(),
156+
R"(:3:20 error: call: call target is not part of the module
157+
%2:void = call %g
158+
^^
159+
160+
:2:3 note: In block
161+
%b1 = block {
162+
^^^^^^^^^^^
163+
164+
note: # Disassembly
165+
%f = func():void -> %b1 {
166+
%b1 = block {
167+
%2:void = call %g
168+
ret
169+
}
170+
}
171+
)");
172+
}
173+
109174
TEST_F(IR_ValidatorTest, Block_NoTerminator) {
110175
b.Function("my_func", ty.void_());
111176

@@ -124,6 +189,48 @@ note: # Disassembly
124189
)");
125190
}
126191

192+
TEST_F(IR_ValidatorTest, Block_VarBlockMismatch) {
193+
auto* var = b.Var(ty.ptr<function, i32>());
194+
195+
auto* f = b.Function("f", ty.void_());
196+
f->Block()->Append(var);
197+
f->Block()->Append(b.Return(f));
198+
199+
auto* g = b.Function("g", ty.void_());
200+
g->Block()->Append(b.Return(g));
201+
202+
var->SetBlock(g->Block());
203+
204+
auto res = ir::Validate(mod);
205+
ASSERT_FALSE(res);
206+
EXPECT_EQ(res.Failure().reason.str(),
207+
R"(:3:41 error: var: block instruction does not have same block as parent
208+
%2:ptr<function, i32, read_write> = var
209+
^^^
210+
211+
:2:3 note: In block
212+
%b1 = block {
213+
^^^^^^^^^^^
214+
215+
:2:3 note: In block
216+
%b1 = block {
217+
^^^^^^^^^^^
218+
219+
note: # Disassembly
220+
%f = func():void -> %b1 {
221+
%b1 = block {
222+
%2:ptr<function, i32, read_write> = var
223+
ret
224+
}
225+
}
226+
%g = func():void -> %b2 {
227+
%b2 = block {
228+
ret
229+
}
230+
}
231+
)");
232+
}
233+
127234
TEST_F(IR_ValidatorTest, Access_NegativeIndex) {
128235
auto* f = b.Function("my_func", ty.void_());
129236
auto* obj = b.FunctionParam(ty.vec3<f32>());
@@ -1030,8 +1137,7 @@ TEST_F(IR_ValidatorTest, Instruction_DeadOperand) {
10301137

10311138
auto res = ir::Validate(mod);
10321139
ASSERT_FALSE(res);
1033-
EXPECT_EQ(res.Failure().reason.str(),
1034-
R"(:3:46 error: var: instruction has operand which is not alive
1140+
EXPECT_EQ(res.Failure().reason.str(), R"(:3:46 error: var: instruction operand 0 is not alive
10351141
%2:ptr<function, f32, read_write> = var, %3
10361142
^^
10371143
@@ -1062,7 +1168,7 @@ TEST_F(IR_ValidatorTest, Instruction_OperandUsageRemoved) {
10621168

10631169
auto res = ir::Validate(mod);
10641170
ASSERT_FALSE(res);
1065-
EXPECT_EQ(res.Failure().reason.str(), R"(:3:46 error: var: instruction operand missing usage
1171+
EXPECT_EQ(res.Failure().reason.str(), R"(:3:46 error: var: instruction operand 0 missing usage
10661172
%2:ptr<function, f32, read_write> = var, %3
10671173
^^
10681174

0 commit comments

Comments
 (0)