- Revision
- 291746
- Author
- pan...@apple.com
- Date
- 2022-03-23 09:40:09 -0700 (Wed, 23 Mar 2022)
Log Message
No breakpoints hit on github.com, and some are invalid
https://bugs.webkit.org/show_bug.cgi?id=235607
Reviewed by Yusuke Suzuki.
JSTests:
Add test for multi-line parsing errors.
* stress/regress-88440831.js: Added.
Source/_javascript_Core:
New test: JSTests/stress/regress-88440831.js
Added test case in: inspector/debugger/breakpoints/resolved-dump-all-pause-locations.html
Previously not all line terminations resulted in setting the `m_lineStart` to the current m_code, which meant
that the location for pause-able locations and stack traces were inaccurate when they were on a line that
terminated multi-line comments, strings, or template strings. We now always update m_lineStart when shifting for
a line terminator, instead of only when the terminator appears outside a string or comment.
* debugger/Breakpoint.cpp:
(JSC::Breakpoint::resolve):
- The existing assertions were somewhat in conflict with each other. If we permit the line number to increase,
there is no guarantee that the column number will remain the same or increase, which can now more easily occur
with multi-line strings. Instead, we should make sure that the overall offset has increased.
* parser/Lexer.cpp:
(JSC::Lexer<T>::shiftLineTerminator):
(JSC::Lexer<T>::lexWithoutClearingLineTerminator):
(JSC::Lexer<T>::scanTemplateString):
LayoutTests:
Add test cases for resolving breakpoints on lines that begin with the end of multi-line strings, comments, and
template strings.
* inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt:
* inspector/debugger/breakpoints/resolved-dump-all-pause-locations.html:
* inspector/debugger/breakpoints/resources/dump-multiline.js: Added.
(test):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (291745 => 291746)
--- trunk/JSTests/ChangeLog 2022-03-23 14:12:54 UTC (rev 291745)
+++ trunk/JSTests/ChangeLog 2022-03-23 16:40:09 UTC (rev 291746)
@@ -1,3 +1,14 @@
+2022-03-23 Patrick Angle <pan...@apple.com>
+
+ No breakpoints hit on github.com, and some are invalid
+ https://bugs.webkit.org/show_bug.cgi?id=235607
+
+ Reviewed by Yusuke Suzuki.
+
+ Add test for multi-line parsing errors.
+
+ * stress/regress-88440831.js: Added.
+
2022-03-22 Yusuke Suzuki <ysuz...@apple.com>
[JSC] Test DFG / FTL DataIC
Added: trunk/JSTests/stress/regress-88440831.js (0 => 291746)
--- trunk/JSTests/stress/regress-88440831.js (rev 0)
+++ trunk/JSTests/stress/regress-88440831.js 2022-03-23 16:40:09 UTC (rev 291746)
@@ -0,0 +1,11 @@
+// This test passes if it does not crash in Debug builds
+
+(function() {
+ try {
+ eval('\'\\\n\n\'');
+ } catch {}
+
+ try {
+ new Function("/* comment */\n(/*;");
+ } catch {}
+})();
\ No newline at end of file
Modified: trunk/LayoutTests/ChangeLog (291745 => 291746)
--- trunk/LayoutTests/ChangeLog 2022-03-23 14:12:54 UTC (rev 291745)
+++ trunk/LayoutTests/ChangeLog 2022-03-23 16:40:09 UTC (rev 291746)
@@ -1,3 +1,18 @@
+2022-03-23 Patrick Angle <pan...@apple.com>
+
+ No breakpoints hit on github.com, and some are invalid
+ https://bugs.webkit.org/show_bug.cgi?id=235607
+
+ Reviewed by Yusuke Suzuki.
+
+ Add test cases for resolving breakpoints on lines that begin with the end of multi-line strings, comments, and
+ template strings.
+
+ * inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt:
+ * inspector/debugger/breakpoints/resolved-dump-all-pause-locations.html:
+ * inspector/debugger/breakpoints/resources/dump-multiline.js: Added.
+ (test):
+
2022-03-23 Ziran Sun <z...@igalia.com>
[InputElement] Add HTMLInputElement::showPicker() method
Modified: trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt (291745 => 291746)
--- trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt 2022-03-23 14:12:54 UTC (rev 291745)
+++ trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt 2022-03-23 16:40:09 UTC (rev 291746)
@@ -2814,3 +2814,82 @@
4
+-- Running test case: Debugger.resolvedBreakpoint.dumpAllLocations.Multiline
+
+INSERTING AT: 0:0
+PAUSES AT: 1:4
+ -> 0 #function test() {
+ => 1 |var x;
+ 2 }
+ 3
+ 4 // Strings
+
+INSERTING AT: 1:5
+PAUSES AT: 2:0
+ 0 function test() {
+ -> 1 v#ar x;
+ => 2 |}
+ 3
+ 4 // Strings
+ 5 let multiline1 = "test\
+
+INSERTING AT: 2:1
+PAUSES AT: 5:0
+ 0 function test() {
+ 1 var x;
+ -> 2 }#
+ 3
+ 4 // Strings
+ => 5 |let multiline1 = "test\
+ 6 string", multiline2 = test();
+ 7
+ 8 // Template Strings
+
+INSERTING AT: 5:1
+PAUSES AT: 6:9
+ 2 }
+ 3
+ 4 // Strings
+ -> 5 l#et multiline1 = "test\
+ => 6 string", |multiline2 = test();
+ 7
+ 8 // Template Strings
+ 9 let multiline3 = `test
+
+INSERTING AT: 6:10
+PAUSES AT: 9:0
+ 3
+ 4 // Strings
+ 5 let multiline1 = "test\
+ -> 6 string", m#ultiline2 = test();
+ 7
+ 8 // Template Strings
+ => 9 |let multiline3 = `test
+ 10 string`, multiline4 = test();
+ 11
+ 12 // Comments
+
+INSERTING AT: 9:1
+PAUSES AT: 10:9
+ 6 string", multiline2 = test();
+ 7
+ 8 // Template Strings
+ -> 9 l#et multiline3 = `test
+ => 10 string`, |multiline4 = test();
+ 11
+ 12 // Comments
+ 13 /* test
+
+INSERTING AT: 10:10
+PAUSES AT: 14:11
+ 7
+ 8 // Template Strings
+ 9 let multiline3 = `test
+ -> 10 string`, m#ultiline4 = test();
+ 11
+ 12 // Comments
+ 13 /* test
+ => 14 comment */ |let multiline5 = test();
+ 15
+
+
Modified: trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-all-pause-locations.html (291745 => 291746)
--- trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-all-pause-locations.html 2022-03-23 14:12:54 UTC (rev 291745)
+++ trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-all-pause-locations.html 2022-03-23 16:40:09 UTC (rev 291746)
@@ -8,6 +8,7 @@
<script src=""
<script src=""
<script src=""
+<script src=""
<script>
function test()
{
@@ -28,6 +29,11 @@
scriptRegex: /dump-unicode\.js$/,
});
+ window.addDumpAllPauseLocationsTestCase(suite, {
+ name: "Debugger.resolvedBreakpoint.dumpAllLocations.Multiline",
+ scriptRegex: /dump-multiline\.js$/,
+ });
+
suite.runTestCasesAndFinish();
}
</script>
Added: trunk/LayoutTests/inspector/debugger/breakpoints/resources/dump-multiline.js (0 => 291746)
--- trunk/LayoutTests/inspector/debugger/breakpoints/resources/dump-multiline.js (rev 0)
+++ trunk/LayoutTests/inspector/debugger/breakpoints/resources/dump-multiline.js 2022-03-23 16:40:09 UTC (rev 291746)
@@ -0,0 +1,15 @@
+function test() {
+ var x;
+}
+
+// Strings
+let multiline1 = "test\
+string", multiline2 = test();
+
+// Template Strings
+let multiline3 = `test
+string`, multiline4 = test();
+
+// Comments
+/* test
+comment */ let multiline5 = test();
Modified: trunk/Source/_javascript_Core/ChangeLog (291745 => 291746)
--- trunk/Source/_javascript_Core/ChangeLog 2022-03-23 14:12:54 UTC (rev 291745)
+++ trunk/Source/_javascript_Core/ChangeLog 2022-03-23 16:40:09 UTC (rev 291746)
@@ -1,3 +1,29 @@
+2022-03-23 Patrick Angle <pan...@apple.com>
+
+ No breakpoints hit on github.com, and some are invalid
+ https://bugs.webkit.org/show_bug.cgi?id=235607
+
+ Reviewed by Yusuke Suzuki.
+
+ New test: JSTests/stress/regress-88440831.js
+ Added test case in: inspector/debugger/breakpoints/resolved-dump-all-pause-locations.html
+
+ Previously not all line terminations resulted in setting the `m_lineStart` to the current m_code, which meant
+ that the location for pause-able locations and stack traces were inaccurate when they were on a line that
+ terminated multi-line comments, strings, or template strings. We now always update m_lineStart when shifting for
+ a line terminator, instead of only when the terminator appears outside a string or comment.
+
+ * debugger/Breakpoint.cpp:
+ (JSC::Breakpoint::resolve):
+ - The existing assertions were somewhat in conflict with each other. If we permit the line number to increase,
+ there is no guarantee that the column number will remain the same or increase, which can now more easily occur
+ with multi-line strings. Instead, we should make sure that the overall offset has increased.
+
+ * parser/Lexer.cpp:
+ (JSC::Lexer<T>::shiftLineTerminator):
+ (JSC::Lexer<T>::lexWithoutClearingLineTerminator):
+ (JSC::Lexer<T>::scanTemplateString):
+
2022-03-23 Xan Lopez <x...@igalia.com>
[JSC] Add DoNotHaveTagRegisters mode to unboxDouble
Modified: trunk/Source/_javascript_Core/debugger/Breakpoint.cpp (291745 => 291746)
--- trunk/Source/_javascript_Core/debugger/Breakpoint.cpp 2022-03-23 14:12:54 UTC (rev 291745)
+++ trunk/Source/_javascript_Core/debugger/Breakpoint.cpp 2022-03-23 16:40:09 UTC (rev 291746)
@@ -65,7 +65,7 @@
ASSERT(isLinked());
ASSERT(!isResolved());
ASSERT(lineNumber >= m_lineNumber);
- ASSERT(columnNumber >= m_columnNumber);
+ ASSERT(columnNumber >= m_columnNumber || lineNumber > m_lineNumber);
m_lineNumber = lineNumber;
m_columnNumber = columnNumber;
Modified: trunk/Source/_javascript_Core/parser/Lexer.cpp (291745 => 291746)
--- trunk/Source/_javascript_Core/parser/Lexer.cpp 2022-03-23 14:12:54 UTC (rev 291745)
+++ trunk/Source/_javascript_Core/parser/Lexer.cpp 2022-03-23 16:40:09 UTC (rev 291746)
@@ -712,6 +712,7 @@
shift();
++m_lineNumber;
+ m_lineStart = m_code;
}
template <typename T>
@@ -2099,11 +2100,15 @@
}
if (m_current == '*') {
shift();
+ auto startLineNumber = m_lineNumber;
+ auto startLineStartOffset = currentLineStartOffset();
if (parseMultilineComment())
goto start;
m_lexErrorMessage = "Multiline comment was not closed properly"_s;
token = UNTERMINATED_MULTILINE_COMMENT_ERRORTOK;
- goto returnError;
+ m_error = true;
+ fillTokenInfo(tokenRecord, token, startLineNumber, currentOffset(), startLineStartOffset, currentPosition());
+ return token;
}
if (m_current == '=') {
shift();
@@ -2472,6 +2477,8 @@
m_buffer8.shrink(0);
break;
case CharacterQuote: {
+ auto startLineNumber = m_lineNumber;
+ auto startLineStartOffset = currentLineStartOffset();
StringParseResult result = StringCannotBeParsed;
if (lexerFlags.contains(LexerFlags::DontBuildStrings))
result = parseString<false>(tokenData, strictMode);
@@ -2480,12 +2487,16 @@
if (UNLIKELY(result != StringParsedSuccessfully)) {
token = result == StringUnterminated ? UNTERMINATED_STRING_LITERAL_ERRORTOK : INVALID_STRING_LITERAL_ERRORTOK;
- goto returnError;
+ m_error = true;
+ fillTokenInfo(tokenRecord, token, startLineNumber, currentOffset(), startLineStartOffset, currentPosition());
+ return token;
}
shift();
token = STRING;
- break;
- }
+ m_atLineStart = false;
+ fillTokenInfo(tokenRecord, token, startLineNumber, currentOffset(), startLineStartOffset, currentPosition());
+ return token;
+ }
case CharacterIdentifierStart: {
if constexpr (ASSERT_ENABLED) {
UChar32 codePoint;
@@ -2506,7 +2517,6 @@
shiftLineTerminator();
m_atLineStart = true;
m_hasLineTerminatorBeforeToken = true;
- m_lineStart = m_code;
goto start;
case CharacterHash: {
// Hashbang is only permitted at the start of the source text.
@@ -2567,7 +2577,6 @@
shiftLineTerminator();
m_atLineStart = true;
m_hasLineTerminatorBeforeToken = true;
- m_lineStart = m_code;
if (!lastTokenWasRestrKeyword())
goto start;
@@ -2699,6 +2708,9 @@
ASSERT(!m_error);
ASSERT(m_buffer16.isEmpty());
+ int startingLineStartOffset = currentLineStartOffset();
+ int startingLineNumber = lineNumber();
+
// Leading backquote ` (for template head) or closing brace } (for template trailing) are already shifted in the previous token scan.
// So in this re-scan phase, shift() is not needed here.
StringParseResult result = parseTemplateLiteral(tokenData, rawStringsBuildMode);
@@ -2711,7 +2723,7 @@
// Since TemplateString always ends with ` or }, m_atLineStart always becomes false.
m_atLineStart = false;
- fillTokenInfo(tokenRecord, token, m_lineNumber, currentOffset(), currentLineStartOffset(), currentPosition());
+ fillTokenInfo(tokenRecord, token, startingLineNumber, currentOffset(), startingLineStartOffset, currentPosition());
return token;
}
