[tint][ir][val] Reject discards outside of functions
While working on root causing the issue that led to this change I
found some other related issues that should be addressed. They have
been bundled up into this change:
- Run() is modified so that later validation passes don't run if
CheckStructureSoundness fails, since they assume that it passed.
- ContainingFunction returns nullptr if the input is in the root
block, which helps prevent ContainingEntryPoint from crashing in a
very confusing way.
Fixes: 382298307
Fixes: 382291447
Fixes: 382294238
Change-Id: Ibce8c5c52d6df960a54b0438cff7a07fe740c961
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/218174
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/src/tint/lang/core/ir/validator.cc b/src/tint/lang/core/ir/validator.cc
index c96bda2..7b9e25a 100644
--- a/src/tint/lang/core/ir/validator.cc
+++ b/src/tint/lang/core/ir/validator.cc
@@ -1250,6 +1250,10 @@
/// @param inst the instruction
/// @returns the function
const ir::Function* ContainingFunction(const ir::Instruction* inst) {
+ if (inst->Block() == mod_.root_block) {
+ return nullptr;
+ }
+
return block_to_function_.GetOrAdd(inst->Block(), [&] { //
return ContainingFunction(inst->Block()->Parent());
});
@@ -1259,6 +1263,10 @@
/// @param f the function
/// @returns all end points that call the function
Hashset<const ir::Function*, 4> ContainingEndPoints(const ir::Function* f) {
+ if (!f) {
+ return {};
+ }
+
Hashset<const ir::Function*, 4> result{};
Hashset<const ir::Function*, 4> visited{f};
@@ -1266,6 +1274,10 @@
while (!call_sites.IsEmpty()) {
auto call_site = call_sites.Pop();
auto calling_function = ContainingFunction(call_site);
+ if (!calling_function) {
+ continue;
+ }
+
if (visited.Contains(calling_function)) {
continue;
}
@@ -1336,8 +1348,13 @@
Result<SuccessType> Validator::Run() {
RunStructuralSoundnessChecks();
- CheckForOrphanedInstructions();
- CheckForNonFragmentDiscards();
+ if (!diagnostics_.ContainsErrors()) {
+ CheckForOrphanedInstructions();
+ }
+
+ if (!diagnostics_.ContainsErrors()) {
+ CheckForNonFragmentDiscards();
+ }
if (diagnostics_.ContainsErrors()) {
diagnostics_.AddNote(Source{}) << "# Disassembly\n" << Disassemble().Text();
@@ -1861,6 +1878,9 @@
AddError(inst) << "root block: invalid instruction: " << inst->TypeInfo().name;
}
},
+ [&](const core::ir::Discard* d) {
+ AddError(d) << "root block: invalid instruction: " << d->TypeInfo().name;
+ },
[&](Default) {
// Note, this validation is looser then it could be. There are only certain
// expressions and builtins which can be used in an override. We can tighten this up
diff --git a/src/tint/lang/core/ir/validator_test.cc b/src/tint/lang/core/ir/validator_test.cc
index 5e4dc7b..1381505 100644
--- a/src/tint/lang/core/ir/validator_test.cc
+++ b/src/tint/lang/core/ir/validator_test.cc
@@ -3849,6 +3849,29 @@
)");
}
+TEST_F(IR_ValidatorTest, Discard_OutsideFunction) {
+ auto* d = b.Discard();
+ mod.root_block->Append(d);
+
+ auto res = ir::Validate(mod);
+ ASSERT_NE(res, Success);
+ EXPECT_EQ(res.Failure().reason.Str(),
+ R"(:2:3 error: discard: root block: invalid instruction: tint::core::ir::Discard
+ discard
+ ^^^^^^^
+
+:1:1 note: in block
+$B1: { # root
+^^^
+
+note: # Disassembly
+$B1: { # root
+ discard
+}
+
+)");
+}
+
TEST_F(IR_ValidatorTest, Terminator_HasResult) {
auto* ret_func = b.Function("ret_func", ty.void_());
b.Append(ret_func->Block(), [&] {
