Skip to content

Commit 9fdb174

Browse files
committed
[MERGE #5451 @paolosevMSFT] Ensure that GeneratorReturnExceptions are not handled by InterpreterStackFrame::DebugProcess
Merge pull request #5451 from paolosevMSFT:user/paolosev/bug17418795 When running a generator function with the -debuglaunch flag the engine crashes with simple code like: ``` function* test() { try { yield 42; } catch (e) { } } var t = test(); t.next(); var v = t.return(); ``` The reason is that a special exception is thrown by JavascriptOperators::OP_ResumeYield() to carry the return value from a generator. When the generator has a try/catch, like in this case, this exception is caught and rethrown by InterpreterStackFrame::OP_TryCatch(), and after that it is caught and handled by JavascriptGenerator::CallGenerator(). But when we run with -debuglaunch the exception gets caught by InterpreterStackFrame::DebugProcess(), which does not rethrow it. Therefore the generator code does not have a chance to complete correctly and the process crashes because an invalid var is returned in place of a generator yielded object. As fix this PR adds a check for IsGeneratorReturnException() in InterpreterStackFrame::DebugProcess() to avoid skipping this exception.
2 parents d439708 + a9f8db9 commit 9fdb174

File tree

4 files changed

+48
-40
lines changed

4 files changed

+48
-40
lines changed

lib/Runtime/Language/InterpreterStackFrame.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2353,8 +2353,9 @@ namespace Js
23532353
if (exception)
23542354
{
23552355
bool skipException = false;
2356-
if (exception != scriptContext->GetThreadContext()->GetPendingSOErrorObject()
2357-
&& exception != scriptContext->GetThreadContext()->GetPendingOOMErrorObject())
2356+
if (!exception->IsGeneratorReturnException() &&
2357+
exception != scriptContext->GetThreadContext()->GetPendingSOErrorObject() &&
2358+
exception != scriptContext->GetThreadContext()->GetPendingOOMErrorObject())
23582359
{
23592360
skipException = exception->IsDebuggerSkip();
23602361
}

test/DebuggerCommon/generators.js

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,28 @@ g = gf4(); /**bp:
124124
125125
stack();resume('step_into');
126126
stack();resume('step_into');
127-
stack();resume('step_into');
128-
stack();resume('step_out');
129-
stack();resume('step_out');
130127
**/
131128

132129
g.next(1);
133130
g.return(2);
134131

132+
function* gf5() {
133+
try {
134+
yield 32;
135+
} catch (e) {
136+
}
137+
}
138+
139+
g = gf5(); /**bp:
140+
stack();resume('step_over');
141+
142+
resume('step_over');
143+
144+
stack();resume('step_into');
145+
stack();resume('step_out');
146+
stack();
147+
**/
148+
g.next();
149+
g.return(1);
150+
135151
WScript.Echo("PASS");

test/DebuggerCommon/generators.js.dbg.baseline

Lines changed: 21 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@
524524
{
525525
"callStack": [
526526
{
527-
"line": 132,
527+
"line": 129,
528528
"column": 0,
529529
"sourceText": "g.return(2)",
530530
"function": "Global code"
@@ -540,7 +540,7 @@
540540
"function": "gf4"
541541
},
542542
{
543-
"line": 132,
543+
"line": 129,
544544
"column": 0,
545545
"sourceText": "g.return(2)",
546546
"function": "Global code"
@@ -550,59 +550,45 @@
550550
{
551551
"callStack": [
552552
{
553-
"line": 77,
554-
"column": 4,
555-
"sourceText": "yield 1",
556-
"function": "gf3"
557-
},
558-
{
559-
"line": 82,
560-
"column": 4,
561-
"sourceText": "yield* gf3()",
562-
"function": "gf4"
563-
},
564-
{
565-
"line": 132,
553+
"line": 138,
566554
"column": 0,
567-
"sourceText": "g.return(2)",
555+
"sourceText": "g = gf5()",
568556
"function": "Global code"
569557
}
570558
]
571559
},
572560
{
573561
"callStack": [
574562
{
575-
"line": 78,
576-
"column": 4,
577-
"sourceText": "yield 2",
578-
"function": "gf3"
579-
},
563+
"line": 148,
564+
"column": 0,
565+
"sourceText": "g.return(1)",
566+
"function": "Global code"
567+
}
568+
]
569+
},
570+
{
571+
"callStack": [
580572
{
581-
"line": 82,
582-
"column": 4,
583-
"sourceText": "yield* gf3()",
584-
"function": "gf4"
573+
"line": 133,
574+
"column": 8,
575+
"sourceText": "yield 32",
576+
"function": "gf5"
585577
},
586578
{
587-
"line": 132,
579+
"line": 148,
588580
"column": 0,
589-
"sourceText": "g.return(2)",
581+
"sourceText": "g.return(1)",
590582
"function": "Global code"
591583
}
592584
]
593585
},
594586
{
595587
"callStack": [
596588
{
597-
"line": 82,
598-
"column": 4,
599-
"sourceText": "yield* gf3()",
600-
"function": "gf4"
601-
},
602-
{
603-
"line": 132,
589+
"line": 150,
604590
"column": 0,
605-
"sourceText": "g.return(2)",
591+
"sourceText": "WScript.Echo(\"PASS\")",
606592
"function": "Global code"
607593
}
608594
]

test/es6/generators-functionality.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -949,6 +949,11 @@ var tests = [
949949
assert.areEqual({ value: 2, done: false }, g.return(100), "Return causes the finally block to execute");
950950
assert.throws(function () { g.next(); }, ExpectedException, "Second next call should throw from finally block");
951951
assert.areEqual({ value: undefined, done: true }, g.next(), "Method execution has finished");
952+
953+
gf = function* () { try { throw new ExpectedException(); } catch (e) { throw e; } }
954+
g = gf();
955+
assert.throws(function () { g.next() }, ExpectedException, "First next call should rethrow exception");
956+
assert.areEqual({ value: 2, done: true }, g.return(2), "Return statement completes the function from try block");
952957
}
953958
},
954959
{

0 commit comments

Comments
 (0)