- Revision
- 202768
- Author
- [email protected]
- Date
- 2016-07-01 17:59:38 -0700 (Fri, 01 Jul 2016)
Log Message
fix "ASSERTION FAILED: currentOffset() >= currentLineStartOffset()"
https://bugs.webkit.org/show_bug.cgi?id=158572
<rdar://problem/26884092>
Reviewed by Mark Lam.
Source/_javascript_Core:
There is a bug in our lexer when we notice the pattern:
```<return|continue|break|...etc> // some comment here```
Our code will say that the token for the comment is a semicolon.
This is the correct semantics, however, it would give the semicolon
a start offset of the comment, but it will give its line start offset
the newline after the comment. This breaks the invariant in the lexer/parser
that the offset for the current line starting point must be less than or equal to
than the start offset of any token on that line. This invariant was broken because
the line start offset was greater than the token start offset. To maintain this
invariant, we claim that the semicolon token is located where the comment starts,
and that its line start offset is the line start offset for the line with the
comment on it. There are other solutions that maintain this invariant, but this
solution provides the best error messages.
* parser/Lexer.cpp:
(JSC::Lexer<T>::lex):
* parser/Parser.h:
(JSC::Parser::internalSaveLexerState):
* tests/stress/obscure-error-message-dont-crash.js: Added.
(try.eval.or.catch):
LayoutTests:
* js/parser-syntax-check-expected.txt:
* js/script-tests/parser-syntax-check.js:
(invalid.or.break.catch):
(catch): Deleted.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (202767 => 202768)
--- trunk/LayoutTests/ChangeLog 2016-07-02 00:56:08 UTC (rev 202767)
+++ trunk/LayoutTests/ChangeLog 2016-07-02 00:59:38 UTC (rev 202768)
@@ -1,3 +1,16 @@
+2016-07-01 Saam Barati <[email protected]>
+
+ fix "ASSERTION FAILED: currentOffset() >= currentLineStartOffset()"
+ https://bugs.webkit.org/show_bug.cgi?id=158572
+ <rdar://problem/26884092>
+
+ Reviewed by Mark Lam.
+
+ * js/parser-syntax-check-expected.txt:
+ * js/script-tests/parser-syntax-check.js:
+ (invalid.or.break.catch):
+ (catch): Deleted.
+
2016-07-01 Ryan Haddad <[email protected]>
Marking fast/history/page-cache-webdatabase-pending-transaction.html as flaky on Mac
Modified: trunk/LayoutTests/js/parser-syntax-check-expected.txt (202767 => 202768)
--- trunk/LayoutTests/js/parser-syntax-check-expected.txt 2016-07-02 00:56:08 UTC (rev 202767)
+++ trunk/LayoutTests/js/parser-syntax-check-expected.txt 2016-07-02 00:59:38 UTC (rev 202768)
@@ -1285,6 +1285,13 @@
PASS Valid: "function f() { class C { constructor() { this._x = 45; } get foo() { return this._x;} } class D extends C { constructor(x = () => super.foo) { super(); this._x_f = x; } x() { return this._x_f(); } } }"
PASS Valid: "class C { constructor() { this._x = 45; } get foo() { return this._x;} } class D extends C { constructor(x = () => super()) { x(); } x() { return super.foo; } }"
PASS Valid: "function f() { class C { constructor() { this._x = 45; } get foo() { return this._x;} } class D extends C { constructor(x = () => super()) { x(); } x() { return super.foo; } } }"
+Weird things that used to crash.
+PASS Invalid: "or ([[{break //(elseifo (a=0;a<2;a++)n=
+ [[{aFYY sga=
+ [[{a=Yth FunctionRY&=Ylet 'a'}V a"
+PASS Invalid: "function f() { or ([[{break //(elseifo (a=0;a<2;a++)n=
+ [[{aFYY sga=
+ [[{a=Yth FunctionRY&=Ylet 'a'}V a }"
PASS e.line is 1
PASS foo is 'PASS'
PASS bar is 'PASS'
Modified: trunk/LayoutTests/js/script-tests/parser-syntax-check.js (202767 => 202768)
--- trunk/LayoutTests/js/script-tests/parser-syntax-check.js 2016-07-02 00:56:08 UTC (rev 202767)
+++ trunk/LayoutTests/js/script-tests/parser-syntax-check.js 2016-07-02 00:59:38 UTC (rev 202768)
@@ -754,6 +754,11 @@
valid("class C { constructor() { this._x = 45; } get foo() { return this._x;} } class D extends C { constructor(x = () => super.foo) { super(); this._x_f = x; } x() { return this._x_f(); } }");
valid("class C { constructor() { this._x = 45; } get foo() { return this._x;} } class D extends C { constructor(x = () => super()) { x(); } x() { return super.foo; } }");
+debug("Weird things that used to crash.");
+invalid(`or ([[{break //(elseifo (a=0;a<2;a++)n=
+ [[{aFYY sga=
+ [[{a=Yth FunctionRY&=Ylet 'a'}V a`)
+
try { eval("a.b.c = {};"); } catch(e1) { e=e1; shouldBe("e.line", "1") }
foo = 'FAIL';
bar = 'PASS';
Modified: trunk/Source/_javascript_Core/ChangeLog (202767 => 202768)
--- trunk/Source/_javascript_Core/ChangeLog 2016-07-02 00:56:08 UTC (rev 202767)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-07-02 00:59:38 UTC (rev 202768)
@@ -1,3 +1,32 @@
+2016-07-01 Saam Barati <[email protected]>
+
+ fix "ASSERTION FAILED: currentOffset() >= currentLineStartOffset()"
+ https://bugs.webkit.org/show_bug.cgi?id=158572
+ <rdar://problem/26884092>
+
+ Reviewed by Mark Lam.
+
+ There is a bug in our lexer when we notice the pattern:
+ ```<return|continue|break|...etc> // some comment here```
+ Our code will say that the token for the comment is a semicolon.
+ This is the correct semantics, however, it would give the semicolon
+ a start offset of the comment, but it will give its line start offset
+ the newline after the comment. This breaks the invariant in the lexer/parser
+ that the offset for the current line starting point must be less than or equal to
+ than the start offset of any token on that line. This invariant was broken because
+ the line start offset was greater than the token start offset. To maintain this
+ invariant, we claim that the semicolon token is located where the comment starts,
+ and that its line start offset is the line start offset for the line with the
+ comment on it. There are other solutions that maintain this invariant, but this
+ solution provides the best error messages.
+
+ * parser/Lexer.cpp:
+ (JSC::Lexer<T>::lex):
+ * parser/Parser.h:
+ (JSC::Parser::internalSaveLexerState):
+ * tests/stress/obscure-error-message-dont-crash.js: Added.
+ (try.eval.or.catch):
+
2016-07-01 Benjamin Poulain <[email protected]>
__defineGetter__/__defineSetter__ should throw exceptions
Modified: trunk/Source/_javascript_Core/parser/Lexer.cpp (202767 => 202768)
--- trunk/Source/_javascript_Core/parser/Lexer.cpp 2016-07-02 00:56:08 UTC (rev 202767)
+++ trunk/Source/_javascript_Core/parser/Lexer.cpp 2016-07-02 00:59:38 UTC (rev 202768)
@@ -1783,6 +1783,15 @@
JSTokenType token = ERRORTOK;
m_terminator = false;
+ auto fillTokenInfo = [&] (int lineNumber, int endOffset, int lineStartOffset, JSTextPosition endPosition) {
+ tokenLocation->line = lineNumber;
+ tokenLocation->endOffset = endOffset;
+ tokenLocation->lineStartOffset = lineStartOffset;
+ ASSERT(tokenLocation->endOffset >= tokenLocation->lineStartOffset);
+ tokenRecord->m_endPosition = endPosition;
+ m_lastToken = token;
+ };
+
start:
skipWhitespace();
@@ -2257,37 +2266,36 @@
// Fall through to complete single line comment parsing.
inSingleLineComment:
- while (!isLineTerminator(m_current)) {
- if (atEnd())
- return EOFTOK;
- shift();
+ {
+ auto lineNumber = m_lineNumber;
+ auto endOffset = currentOffset();
+ auto lineStartOffset = currentLineStartOffset();
+ auto endPosition = currentPosition();
+
+ while (!isLineTerminator(m_current)) {
+ if (atEnd())
+ return EOFTOK;
+ shift();
+ }
+ shiftLineTerminator();
+ m_atLineStart = true;
+ m_terminator = true;
+ m_lineStart = m_code;
+ if (!lastTokenWasRestrKeyword())
+ goto start;
+
+ token = SEMICOLON;
+ fillTokenInfo(lineNumber, endOffset, lineStartOffset, endPosition);
+ return token;
}
- shiftLineTerminator();
- m_atLineStart = true;
- m_terminator = true;
- m_lineStart = m_code;
- if (!lastTokenWasRestrKeyword())
- goto start;
- token = SEMICOLON;
- // Fall through into returnToken.
-
returnToken:
- tokenLocation->line = m_lineNumber;
- tokenLocation->endOffset = currentOffset();
- tokenLocation->lineStartOffset = currentLineStartOffset();
- ASSERT(tokenLocation->endOffset >= tokenLocation->lineStartOffset);
- tokenRecord->m_endPosition = currentPosition();
- m_lastToken = token;
+ fillTokenInfo(m_lineNumber, currentOffset(), currentLineStartOffset(), currentPosition());
return token;
returnError:
m_error = true;
- tokenLocation->line = m_lineNumber;
- tokenLocation->endOffset = currentOffset();
- tokenLocation->lineStartOffset = currentLineStartOffset();
- ASSERT(tokenLocation->endOffset >= tokenLocation->lineStartOffset);
- tokenRecord->m_endPosition = currentPosition();
+ fillTokenInfo(m_lineNumber, currentOffset(), currentLineStartOffset(), currentPosition());
RELEASE_ASSERT(token & ErrorTokenFlag);
return token;
}
Modified: trunk/Source/_javascript_Core/parser/Parser.h (202767 => 202768)
--- trunk/Source/_javascript_Core/parser/Parser.h 2016-07-02 00:56:08 UTC (rev 202767)
+++ trunk/Source/_javascript_Core/parser/Parser.h 2016-07-02 00:59:38 UTC (rev 202768)
@@ -1518,6 +1518,7 @@
result.oldLineStartOffset = m_token.m_location.lineStartOffset;
result.oldLastLineNumber = m_lexer->lastLineNumber();
result.oldLineNumber = m_lexer->lineNumber();
+ ASSERT(static_cast<unsigned>(result.startOffset) >= result.oldLineStartOffset);
return result;
}
Added: trunk/Source/_javascript_Core/tests/stress/obscure-error-message-dont-crash.js (0 => 202768)
--- trunk/Source/_javascript_Core/tests/stress/obscure-error-message-dont-crash.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/obscure-error-message-dont-crash.js 2016-07-02 00:59:38 UTC (rev 202768)
@@ -0,0 +1,13 @@
+//@ runDefault
+
+let success = false;
+try {
+ eval(`or ([[{break //comment
+ [[{aFY sga=
+ [[{a=Yth FunctionRY&=Ylet 'a'}V a`)
+} catch(e) {
+ success = e.toString() === "SyntaxError: Unexpected token '//'. Expected a ':' following the property name 'break'.";
+}
+
+if (!success)
+ throw new Error("Bad result")
