From c3f8fdfe694c79f08893f32d7e28707977122797 Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 21 Jun 2021 17:53:56 +0000 Subject: [PATCH] validation: no statically dead statement Bug: tint:65 Change-Id: Iefc103afe77bc4423ec981eeb3bf066d239e99de Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54981 Auto-Submit: Sarah Mashayekhi Kokoro: Kokoro Reviewed-by: Antonio Maiorano Commit-Queue: Antonio Maiorano --- src/resolver/control_block_validation_test.cc | 31 ++++++++++++++++ src/resolver/function_validation_test.cc | 18 +++++++++ src/resolver/resolver.cc | 19 ++++++++++ src/resolver/resolver.h | 1 + src/resolver/validation_test.cc | 37 +++---------------- 5 files changed, 75 insertions(+), 31 deletions(-) diff --git a/src/resolver/control_block_validation_test.cc b/src/resolver/control_block_validation_test.cc index 9a2c7162a1..f4db5e17af 100644 --- a/src/resolver/control_block_validation_test.cc +++ b/src/resolver/control_block_validation_test.cc @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include "src/ast/break_statement.h" +#include "src/ast/continue_statement.h" #include "src/ast/fallthrough_statement.h" #include "src/ast/switch_statement.h" #include "src/resolver/resolver_test_helper.h" @@ -108,6 +110,35 @@ TEST_F(ResolverControlBlockValidationTest, SwitchWithTwoDefault_Fail) { "clause"); } +TEST_F(ResolverControlBlockValidationTest, UnreachableCode_continue) { + // loop { + // continue; + // var z : i32; + // } + WrapInFunction(Loop(Block( + create(), + Decl(Source{{12, 34}}, Var("z", ty.i32(), ast::StorageClass::kNone))))); + + EXPECT_FALSE(r()->Resolve()) << r()->error(); + EXPECT_EQ(r()->error(), "12:34 error: code is unreachable"); +} + +TEST_F(ResolverControlBlockValidationTest, UnreachableCode_break) { + // switch (a) { + // case 1: { break; var a : u32 = 2;} + // default: {} + // } + auto* decl = Decl(Source{{12, 34}}, + Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2))); + + WrapInFunction(Loop(Block(Switch( + Expr(1), Case(Literal(1), Block(create(), decl)), + DefaultCase())))); + + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "12:34 error: code is unreachable"); +} + TEST_F(ResolverControlBlockValidationTest, SwitchConditionTypeMustMatchSelectorType2_Fail) { // var a : u32 = 2; diff --git a/src/resolver/function_validation_test.cc b/src/resolver/function_validation_test.cc index ca4c0a9095..93852c8045 100644 --- a/src/resolver/function_validation_test.cc +++ b/src/resolver/function_validation_test.cc @@ -133,6 +133,24 @@ TEST_F(ResolverFunctionValidationTest, EXPECT_TRUE(r()->Resolve()) << r()->error(); } +TEST_F(ResolverFunctionValidationTest, UnreachableCode_return) { + // fn func() -> { + // return; + // var a: i32 = 2; + //} + auto* decl = Decl(Source{{12, 34}}, + Var("a", ty.i32(), ast::StorageClass::kNone, Expr(2))); + + Func("func", ast::VariableList{}, ty.void_(), + ast::StatementList{ + Return(), + decl, + }, + ast::DecorationList{}); + EXPECT_FALSE(r()->Resolve()); + EXPECT_EQ(r()->error(), "12:34 error: code is unreachable"); +} + TEST_F(ResolverFunctionValidationTest, FunctionEndWithoutReturnStatement_Fail) { // fn func -> int { var a:i32 = 2; } diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc index 9b7234f862..e8afe07ab7 100644 --- a/src/resolver/resolver.cc +++ b/src/resolver/resolver.cc @@ -1502,6 +1502,25 @@ bool Resolver::Statements(const ast::StatementList& stmts) { return false; } } + if (!ValidateStatements(stmts)) { + return false; + } + + return true; +} + +bool Resolver::ValidateStatements(const ast::StatementList& stmts) { + auto next_stmt = stmts.begin(); + for (auto* stmt : stmts) { + next_stmt++; + if (stmt->IsAnyOf()) { + if (stmt != stmts.back()) { + diagnostics_.add_error("code is unreachable", (*next_stmt)->source()); + return false; + } + } + } return true; } diff --git a/src/resolver/resolver.h b/src/resolver/resolver.h index d92b4b136e..a8930ae44b 100644 --- a/src/resolver/resolver.h +++ b/src/resolver/resolver.h @@ -277,6 +277,7 @@ class Resolver { const sem::Matrix* matrix_type); bool ValidateParameter(const VariableInfo* info); bool ValidateReturn(const ast::ReturnStatement* ret); + bool ValidateStatements(const ast::StatementList& stmts); bool ValidateStorageTexture(const ast::StorageTexture* t); bool ValidateStructure(const sem::Struct* str); bool ValidateSwitch(const ast::SwitchStatement* s); diff --git a/src/resolver/validation_test.cc b/src/resolver/validation_test.cc index 3af33e0e49..600a588bed 100644 --- a/src/resolver/validation_test.cc +++ b/src/resolver/validation_test.cc @@ -403,30 +403,6 @@ TEST_F(ResolverValidationTest, Expr_MemberAccessor_VectorSwizzle_BadIndex) { EXPECT_EQ(r()->error(), "3:3 error: invalid vector swizzle member"); } -TEST_F(ResolverValidationTest, - Stmt_Loop_ContinueInLoopBodyBeforeDecl_UsageInContinuing) { - // loop { - // continue; // Bypasses z decl - // var z : i32; - // - // continuing { - // z = 2; - // } - // } - - auto error_loc = Source{Source::Location{12, 34}}; - auto* body = Block(create(), - Decl(Var("z", ty.i32(), ast::StorageClass::kNone))); - auto* continuing = Block(Assign(Expr(error_loc, "z"), 2)); - auto* loop_stmt = Loop(body, continuing); - WrapInFunction(loop_stmt); - - EXPECT_FALSE(r()->Resolve()) << r()->error(); - EXPECT_EQ(r()->error(), - "12:34 error: continue statement bypasses declaration of 'z' in " - "continuing block"); -} - TEST_F(ResolverValidationTest, Stmt_Loop_ContinueInLoopBodyBeforeDeclAndAfterDecl_UsageInContinuing) { // loop { @@ -440,17 +416,16 @@ TEST_F(ResolverValidationTest, // } auto error_loc = Source{Source::Location{12, 34}}; - auto* body = Block(create(), - Decl(Var("z", ty.i32(), ast::StorageClass::kNone)), - create()); - auto* continuing = Block(Assign(Expr(error_loc, "z"), 2)); + auto* body = + Block(create(), + Decl(error_loc, Var("z", ty.i32(), ast::StorageClass::kNone)), + create()); + auto* continuing = Block(Assign(Expr("z"), 2)); auto* loop_stmt = Loop(body, continuing); WrapInFunction(loop_stmt); EXPECT_FALSE(r()->Resolve()) << r()->error(); - EXPECT_EQ(r()->error(), - "12:34 error: continue statement bypasses declaration of 'z' in " - "continuing block"); + EXPECT_EQ(r()->error(), "12:34 error: code is unreachable"); } TEST_F(ResolverValidationTest,