Skip to content

Commit

Permalink
[WGSL] Unterminated block comments crash the lexer
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259661
rdar://113161516

Reviewed by Dan Glastonbury.

In Lexer::skipBlockComments, we need to check if we are at the end of the file
before we shift. I also had to tweak the parser since it would ignore the first
token if it's the only token. This happens because we call `lex()` when constructing
the parser and then check if the lexer is at the end of the file, instead of the
checking the token produced by `lex()`.

* Source/WebGPU/WGSL/Lexer.cpp:
(WGSL::Lexer<T>::lex):
(WGSL::Lexer<T>::shift):
(WGSL::Lexer<T>::skipBlockComments):
(WGSL::Lexer<T>::skipWhitespaceAndComments):
* Source/WebGPU/WGSL/Lexer.h:
* Source/WebGPU/WGSL/Parser.cpp:
(WGSL::Parser<Lexer>::parseShader):
* Source/WebGPU/WGSL/tests/invalid/unterminated-comment.wgsl: Added.

Canonical link: https://commits.webkit.org/266476@main
  • Loading branch information
tadeuzagallo committed Aug 1, 2023
1 parent f71ea9e commit 65d4092
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 11 deletions.
22 changes: 14 additions & 8 deletions Source/WebGPU/WGSL/Lexer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ namespace WGSL {
template <typename T>
Token Lexer<T>::lex()
{
skipWhitespaceAndComments();
if (!skipWhitespaceAndComments())
return makeToken(TokenType::Invalid);

m_tokenStartingPosition = m_currentPosition;

Expand Down Expand Up @@ -394,6 +395,8 @@ Token Lexer<T>::lex()
template <typename T>
T Lexer<T>::shift(unsigned i)
{
ASSERT(m_code + i <= m_codeEnd);

T last = m_current;
// At one point timing showed that setting m_current to 0 unconditionally was faster than an if-else sequence.
m_current = 0;
Expand Down Expand Up @@ -421,15 +424,15 @@ void Lexer<T>::newLine()
}

template <typename T>
void Lexer<T>::skipBlockComments()
bool Lexer<T>::skipBlockComments()
{
ASSERT(peek(0) == '/' && peek(1) == '*');
shift(2);

T ch = 0;
unsigned depth = 1u;

while ((ch = shift())) {
while (!isAtEndOfFile() && (ch = shift())) {
if (ch == '/' && peek() == '*') {
shift();
depth += 1;
Expand All @@ -440,13 +443,14 @@ void Lexer<T>::skipBlockComments()
// This block comment is closed, so for a construction like "/* */ */"
// there will be a successfully parsed block comment "/* */"
// and " */" will be processed separately.
return;
return true;
}
} else if (ch == '\n')
newLine();
}

// FIXME: Report unbalanced block comments, such as "/* this is an unbalanced comment."
return false;
}

template <typename T>
Expand All @@ -460,7 +464,7 @@ void Lexer<T>::skipLineComment()
}

template <typename T>
void Lexer<T>::skipWhitespaceAndComments()
bool Lexer<T>::skipWhitespaceAndComments()
{
while (!isAtEndOfFile()) {
if (isUnicodeCompatibleASCIIWhitespace(m_current)) {
Expand All @@ -469,13 +473,15 @@ void Lexer<T>::skipWhitespaceAndComments()
} else if (peek(0) == '/') {
if (peek(1) == '/')
skipLineComment();
else if (peek(1) == '*')
skipBlockComments();
else
else if (peek(1) == '*') {
if (!skipBlockComments())
return false;
} else
break;
} else
break;
}
return true;
}

template <typename T>
Expand Down
4 changes: 2 additions & 2 deletions Source/WebGPU/WGSL/Lexer.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ class Lexer {
T shift(unsigned = 1);
T peek(unsigned = 0);
void newLine();
void skipBlockComments();
bool skipBlockComments();
void skipLineComment();
void skipWhitespaceAndComments();
bool skipWhitespaceAndComments();

// Reads [0-9]+
std::optional<uint64_t> parseDecimalInteger();
Expand Down
2 changes: 1 addition & 1 deletion Source/WebGPU/WGSL/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ Result<void> Parser<Lexer>::parseShader()
{
// FIXME: parse directives here.

while (!m_lexer.isAtEndOfFile()) {
while (current().type != TokenType::EndOfFile) {
auto globalExpected = parseGlobalDecl();
if (!globalExpected)
return makeUnexpected(globalExpected.error());
Expand Down
3 changes: 3 additions & 0 deletions Source/WebGPU/WGSL/tests/invalid/unterminated-comment.wgsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// RUN: %not %wgslc

/*

0 comments on commit 65d4092

Please sign in to comment.