With DCHECKs enabled, it crashes during %GetFrameCount(), but not when processing the async function (it's hit in a FunctionKind::kNormal function), probably AsyncFunctionNext().
The attached test case crashes in d8.
SharedFunctionInfo::internal_formal_parameter_count() is set to -1 during a call to JavaScriptFrame::receiver(), which expects it to be >= 0.
Backtrace:
#0 v8::base::OS::Abort () at ../../../src/base/platform/platform-posix.cc:240
#1 0x08fc4bfe in V8_Fatal (file=0x92395e8 "../../../src/frames-inl.h", line=163, format=0x92160d2 "Check failed: %s.")
at ../../../src/base/logging.cc:116
#2 0x084d248c in v8::internal::JavaScriptFrame::GetParameterSlot (this=0xffd86918, index=-1) at ../../../src/frames-inl.h:163
#3 0x084cb38b in v8::internal::JavaScriptFrame::GetParameter (this=0xffd86918, index=-1) at ../../../src/frames-inl.h:170
#4 0x085d91ad in v8::internal::JavaScriptFrame::receiver (this=0xffd86918) at ../../../src/frames.cc:866
#5 0x085d8fdf in v8::internal::JavaScriptFrame::Summarize (this=0xffd86918, functions=0xffd86890,
mode=v8::internal::FrameSummary::kExactSummary) at ../../../src/frames.cc:857
#6 0x085da1c0 in v8::internal::OptimizedFrame::Summarize (this=0xffd86918, frames=0xffd86890,
mode=v8::internal::FrameSummary::kExactSummary) at ../../../src/frames.cc:1010
#7 0x08971dad in v8::internal::__RT_impl_Runtime_GetFrameCount (args=..., isolate=0xa4ae058) at ../../../src/runtime/runtime-debug.cc:460
#8 0x08971905 in v8::internal::Runtime_GetFrameCount (args_length=1, args_object=0xffd86b14, isolate=0xa4ae058)
at ../../../src/runtime/runtime-debug.cc:444
[...]
so, how should debugging async functions work?
particularly the following:
```
async function f() {
debugger;
print("msg1"); // send StepNext command
var thing = await someOperation(); // send StepNext command
print("msg2"); // send StepNext command
}
f();
```
Currently, this will break at `debugger;`, at `print("msg1");`, at `var thing = await someOperation();`, and then at `}` (the last line of the routine).
I think you'd expect that last StepNext to pick up after the function resumes, but currently it steps to the end of the function. Same thing happens with generators (eg https://jsfiddle.net/dcsjLg92/) --- although arguably it makes more sense for generators.
WDYT?
This sounds like fairly broken behavior to be, both for generators and async functions. Do you have an idea for how it could be fixed? If generators have been broken this way for a while, maybe this shouldn't be a ship blocker, but still important to address. Yang, what do you think?
I actually think stepping for generators are fine, but for async funcions are not. Generators expose its resumable behavior via yield expression, which is similar to return. async functions are supposed to hide that, so as dev I would expect step next to work across await calls.
There's another issue with `StepIn` on non-async values, which adds some extra steps that should be invisible (I'm guessing this is just the call to Promise.resolve()).
So for instance, `debug-step.js` when modified for async functions (and with the `let x = 1;` line changed to `let x = await 1;`, we end up stepping an unexpected number of times (so, when 1000 steps are completed, `i` is 249 rather than 333).
I guess "StepIn" for AwaitExpression with a non-Promise operand should really only step produce a single breakpoint.
This might complicate the proposed changes to StepNext even more, since sometimes StepIn behaves like StepNext?
Step-in should behave like step-next if there is no function call at the location where we perform the step. I think fixing the number of steps is not that hard. Let me implement the step-next behavior first before moving on to step-in.
Step-in at an await call is going to be a bit more challenging anyways, if we want to achieve the behavior that with step-in, we step into the await call, and when we then step-out from there, we end up back in the calling async function. I'll have to get some more insight before thinking about that.
I think mirrors are a missing feature. Generator mirrors are tested, for example, in test/mjsunit/es6/generators-mirror.js . Mirrors are generally implemented in src/debug/mirrors.js and sometimes take extra runtime functions to support them. Ideally, mirrors should support internal properties that match those in the spec. Caitlin, do you think you could implement this for async functions?
FWIW I would be fine if we use generator mirror and generator object mirror for async functions, for now, to stage. We need tests to make sure that works without surprises and crashes.
I'm not sure how the GeneratorMirror makes any sense for async functions, since the generator object is never exposed, and is omitted from frame locals in %GetFrameDetails (due to being a temporary variable).
There's probably stuff that would be helpful to expose to the debugger, but it could probably use a design doc to make it work.
I checked out following variants, with and without experimental JS flag in ToT.
(async function(){
debugger;
throw 1;
})();
setTimeout(`(async function(){
debugger;
throw 1;
})();`,0);
eval(`(async function(){
debugger;
throw 1;
})();`);
Works good to me. l446240525@, could you check in latest dev and confirm that issue is still here?
