From 34ae7d2ce78787e4e0d35d40fe02e893b4aef3db Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Wed, 17 Feb 2021 16:16:02 +0000 Subject: [PATCH] wgsl/parser: Avoid stack overflows Most recursive control flow passes through Sync(). Error out if the Sync() function is recursively called too many times. This replaces the more specific kMaxConstExprDepth, which also passes through Sync(). Fixed: chromium:1178436 Change-Id: I64a05f9f6a4fe6d2b53a3ca75642b30e98c7a35f Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/41724 Commit-Queue: Ben Clayton Reviewed-by: dan sinclair --- src/reader/wgsl/parser_impl.cc | 51 +++++++++---------- src/reader/wgsl/parser_impl.h | 4 +- .../wgsl/parser_impl_const_expr_test.cc | 2 +- src/reader/wgsl/parser_impl_error_msg_test.cc | 10 ++-- 4 files changed, 31 insertions(+), 36 deletions(-) diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc index 957cf1a2f0..0000298120 100644 --- a/src/reader/wgsl/parser_impl.cc +++ b/src/reader/wgsl/parser_impl.cc @@ -83,10 +83,10 @@ using Expect = ParserImpl::Expect; template using Maybe = ParserImpl::Maybe; -/// Controls the maximum number of times we'll call into the const_expr function +/// Controls the maximum number of times we'll call into the sync() function /// from itself. This is to guard against stack overflow when there is an -/// excessive number of type constructors inside the const_expr. -constexpr uint32_t kMaxConstExprDepth = 128; +/// excessive number of blocks. +constexpr uint32_t kMaxSyncDepth = 128; /// The maximum number of tokens to look ahead to try and sync the /// parser on error. @@ -2709,38 +2709,29 @@ Maybe ParserImpl::const_literal() { // : type_decl PAREN_LEFT (const_expr COMMA)? const_expr PAREN_RIGHT // | const_literal Expect ParserImpl::expect_const_expr() { - return expect_const_expr_internal(0); -} - -Expect ParserImpl::expect_const_expr_internal( - uint32_t depth) { auto t = peek(); - if (depth > kMaxConstExprDepth) { - return add_error(t, "max const_expr depth reached"); - } - auto source = t.source(); auto type = type_decl(); if (type.errored) return Failure::kErrored; if (type.matched) { - auto params = expect_paren_block( - "type constructor", [&]() -> Expect { - ast::ExpressionList list; - auto param = expect_const_expr_internal(depth + 1); - if (param.errored) - return Failure::kErrored; - list.emplace_back(param.value); - while (match(Token::Type::kComma)) { - param = expect_const_expr_internal(depth + 1); - if (param.errored) - return Failure::kErrored; - list.emplace_back(param.value); - } - return list; - }); + auto params = expect_paren_block("type constructor", + [&]() -> Expect { + ast::ExpressionList list; + auto param = expect_const_expr(); + if (param.errored) + return Failure::kErrored; + list.emplace_back(param.value); + while (match(Token::Type::kComma)) { + param = expect_const_expr(); + if (param.errored) + return Failure::kErrored; + list.emplace_back(param.value); + } + return list; + }); if (params.errored) return Failure::kErrored; @@ -3170,9 +3161,15 @@ T ParserImpl::expect_lt_gt_block(const std::string& use, F&& body) { template T ParserImpl::sync(Token::Type tok, F&& body) { + if (sync_depth_ >= kMaxSyncDepth) { + return add_error(peek(), "maximum parser recursive depth reached"); + } + sync_tokens_.push_back(tok); + ++sync_depth_; auto result = body(); + --sync_depth_; assert(sync_tokens_.back() == tok); sync_tokens_.pop_back(); diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h index 4105416c07..2573a0cc1a 100644 --- a/src/reader/wgsl/parser_impl.h +++ b/src/reader/wgsl/parser_impl.h @@ -812,9 +812,6 @@ class ParserImpl { Expect expect_type_decl_array(ast::ArrayDecorationList decos); Expect expect_type_decl_matrix(Token t); - Expect expect_const_expr_internal( - uint32_t depth); - Expect expect_type(const std::string& use); Maybe non_block_statement(); @@ -834,6 +831,7 @@ class ParserImpl { std::unique_ptr lexer_; std::deque token_queue_; bool synchronized_ = true; + uint32_t sync_depth_ = 0; std::vector sync_tokens_; int silence_errors_ = 0; std::unordered_map registered_constructs_; diff --git a/src/reader/wgsl/parser_impl_const_expr_test.cc b/src/reader/wgsl/parser_impl_const_expr_test.cc index 8930fd2f9d..b3d302053d 100644 --- a/src/reader/wgsl/parser_impl_const_expr_test.cc +++ b/src/reader/wgsl/parser_impl_const_expr_test.cc @@ -144,7 +144,7 @@ TEST_F(ParserImplTest, ConstExpr_Recursion) { ASSERT_TRUE(p->has_error()); ASSERT_TRUE(e.errored); ASSERT_EQ(e.value, nullptr); - EXPECT_EQ(p->error(), "1:517: max const_expr depth reached"); + EXPECT_EQ(p->error(), "1:517: maximum parser recursive depth reached"); } } // namespace diff --git a/src/reader/wgsl/parser_impl_error_msg_test.cc b/src/reader/wgsl/parser_impl_error_msg_test.cc index eec3330f15..1497c8c1ab 100644 --- a/src/reader/wgsl/parser_impl_error_msg_test.cc +++ b/src/reader/wgsl/parser_impl_error_msg_test.cc @@ -526,17 +526,17 @@ TEST_F(ParserImplErrorTest, GlobalDeclConstBadConstLiteral) { } TEST_F(ParserImplErrorTest, GlobalDeclConstExprMaxDepth) { - uint32_t kMaxConstExprDepth = 128; + uint32_t kMaxDepth = 128; std::stringstream src; std::stringstream mkr; src << "const i : i32 = "; mkr << " "; - for (size_t i = 0; i < kMaxConstExprDepth + 8; i++) { + for (size_t i = 0; i < kMaxDepth + 8; i++) { src << "f32("; - if (i < kMaxConstExprDepth + 1) { + if (i < kMaxDepth) { mkr << " "; - } else if (i == kMaxConstExprDepth + 1) { + } else if (i == kMaxDepth) { mkr << "^^^"; } } @@ -546,7 +546,7 @@ TEST_F(ParserImplErrorTest, GlobalDeclConstExprMaxDepth) { } src << ";"; std::stringstream err; - err << "test.wgsl:1:533 error: max const_expr depth reached\n" + err << "test.wgsl:1:529 error: maximum parser recursive depth reached\n" << src.str() << "\n" << mkr.str() << "\n"; EXPECT(src.str().c_str(), err.str().c_str());