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>
This commit is contained in:
Ben Clayton 2021-06-28 16:09:57 +00:00 committed by Tint LUCI CQ
parent 65cb20c1db
commit c41c7cdf31
2 changed files with 26 additions and 12 deletions

View File

@ -3153,8 +3153,7 @@ bool ParserImpl::expect(const std::string& use, Token::Type tok) {
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 @@ bool ParserImpl::expect(const std::string& use, Token::Type tok) {
// 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 @@ bool ParserImpl::expect(const std::string& use, Token::Type tok) {
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<uint32_t> ParserImpl::expect_nonzero_positive_sint(
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 @@ bool ParserImpl::sync_to(Token::Type tok, bool consume) {
// 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 @@ bool ParserImpl::is_sync_token(const Token& t) const {
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_++;

View File

@ -824,6 +824,12 @@ class ParserImpl {
/// @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.