reader/wgsl: Only resync if num errors less than limit
As all loops in the parser require `synchronized_` to be true, don't set this to true if we've hit the maximum number of error diagnostics.
This lets the parser bubble up as soon as the limit was reached, instead of only aborting once we finally reach the top level loop.
ClusterFuzz has uncovered a number of cases where, in a loop, it can produce an error, then resynchronize, then error again on the next loop iteration. This produces more than the max limit of errors, and can stall the tests long enough to time out.
No unit tests for this, as it requires a really contrived input to trigger, and to exhaustively catch all the places we now call maybe_set_synchronized() would result in a large colleciton of horrible tests. Instead, let ClusterFuzz do the testing.
Fixed: chromium:1224031
Fixed: chromium:1224032
Fixed: chromium:1224042
Fixed: chromium:1224049
Fixed: chromium:1224050
Fixed: chromium:1224130
Fixed: chromium:1224131
Fixed: chromium:1224132
Fixed: chromium:1224144
Fixed: chromium:1224191
Change-Id: I63f160e28d13b85ee92e1b921239d7fb8ac22a08
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/56070
Auto-Submit: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc
index 3930299..3c3a1ea 100644
--- a/src/reader/wgsl/parser_impl.cc
+++ b/src/reader/wgsl/parser_impl.cc
@@ -3153,8 +3153,7 @@
auto t = peek();
if (t.Is(tok)) {
next();
- synchronized_ = true;
- return true;
+ return maybe_set_synchronized();
}
// Special case to split `>>` and `>=` tokens if we are looking for a `>`.
@@ -3165,13 +3164,13 @@
// Push the second character to the token queue.
auto source = t.source();
source.range.begin.column++;
- if (t.IsShiftRight())
+ if (t.IsShiftRight()) {
token_queue_.push_front(Token(Token::Type::kGreaterThan, source));
- else if (t.IsGreaterThanEqual())
+ } else if (t.IsGreaterThanEqual()) {
token_queue_.push_front(Token(Token::Type::kEqual, source));
+ }
- synchronized_ = true;
- return true;
+ return maybe_set_synchronized();
}
// Handle the case when `]` is expected but the actual token is `]]`.
@@ -3181,8 +3180,7 @@
auto source = t.source();
source.range.begin.column++;
token_queue_.push_front({Token::Type::kBracketRight, source});
- synchronized_ = true;
- return true;
+ return maybe_set_synchronized();
}
std::stringstream err;
@@ -3230,7 +3228,9 @@
Expect<std::string> ParserImpl::expect_ident(const std::string& use) {
auto t = peek();
if (t.IsIdentifier()) {
- synchronized_ = true;
+ if (!maybe_set_synchronized()) {
+ return Failure::kErrored;
+ }
next();
return {t.to_str(), t.source()};
}
@@ -3333,10 +3333,10 @@
// Is this synchronization token |tok|?
if (t.Is(tok)) {
- if (consume)
+ if (consume) {
next();
- synchronized_ = true;
- return true;
+ }
+ return maybe_set_synchronized();
}
break;
}
@@ -3352,6 +3352,14 @@
return false;
}
+bool ParserImpl::maybe_set_synchronized() {
+ if (builder_.Diagnostics().error_count() < max_errors_) {
+ synchronized_ = true;
+ return true;
+ }
+ return false;
+}
+
template <typename F, typename T>
T ParserImpl::without_error(F&& body) {
silence_errors_++;
diff --git a/src/reader/wgsl/parser_impl.h b/src/reader/wgsl/parser_impl.h
index 19b29b8..99da98f 100644
--- a/src/reader/wgsl/parser_impl.h
+++ b/src/reader/wgsl/parser_impl.h
@@ -824,6 +824,12 @@
/// @see sync().
bool is_sync_token(const Token& t) const;
+ /// Sets synchronized_ to true if the number of reported errors is less than
+ /// #max_errors_.
+ /// @returns true if the number of reported errors is less than
+ /// #max_errors_, otherwise false.
+ bool maybe_set_synchronized();
+
/// without_error() calls the function `func` muting any grammatical errors
/// found while executing the function. This can be used as a best-effort to
/// produce a meaningful error message when the parser is out of sync.
