Open
Bug 716761
Opened 13 years ago
Updated 2 years ago
Avoid calling ScriptDebugEpilogue a second time when onPop turns a return into a throw or vice versa
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
NEW
People
(Reporter: jimb, Unassigned)
Details
When a function returns, a Debugger.Frame instance's onPop handler can turn that return into a throw.
In JaegerMonkey-generated code, the jitted code calls stubs::ScriptDebugEpilogue, which calls js::ScriptDebugEpilogue, which runs the onPop handler. js::ScriptDebugEpilogue notices that the resumption value returned by the onPop handler is { throw: VALUE }, requesting that VALUE be thrown at that point.
stubs::ScriptDebugEpilogue calls THROW. Control enters JaegerThrowpoline, which then calls js_InternalThrow. Unfortunately, js_InternalThrow will then call js::ScriptDebugEpilogue a second time, for the same frame --- it has no way of knowing that the exception was generated by the first js::ScriptDebugEpilogue call, and thus believes it needs to inform the debugger that the throwing frame is being popped.
In practice, this is okay, because calling js::ScriptDebugEpilogue twice on the same frame happens to have no effect. However, this is an uncomfortable lack of symmetry, and in theory it would be nicer if js::ScriptDebugEpilogue could count on being called exactly once per frame.
One way to fix this would be to change the way throwing works in JM: instead of having THROW edit the stub call's continuation to go to js_InternalThrow, and then go wherever that call directs, THROW itself could call js_InternalThrow, which could edit the stub call's continuation as it pleases.
A quick audit shows that there are a few places that use THROW in a position that is not in the tail position of a stub call (GetPropCompiler::generateGetterStub). These would need cleaning up.
Reporter | ||
Comment 1•13 years ago
|
||
Similar problems occur when an onPop handler converts a throw into a return. However, that could be fixed by changing the way the forceReturn trampolines are generated: call AnyFrameEpilogue from js_InternalThrow, instead of from the trampoline, and pass a flag indicating that ScriptDebugEpilogue should not be run again.
Reporter | ||
Updated•13 years ago
|
Summary: Avoid calling ScriptDebugEpilogue a second time when onPop turns a return into a throw → Avoid calling ScriptDebugEpilogue a second time when onPop turns a return into a throw or vice versa
Reporter | ||
Comment 2•13 years ago
|
||
The description of the fix in comment 0 needs an edit:
One way to fix this would be to change the way throwing works in JM. In the status quo, THROW edits the stub's continuation to return to JaegerThrowpoline. JaegerThrowpoline calls js_InternalThrow, and then either jumps into more JIT code or exits the VMFrame. Instead, THROW itself could call js_InternalThrow, which would edit the continuation as needed.
js_InternalThrow would no longer need to use extern "C" calling conventions; it could become simply js::mjit::InternalThrow.
The forceReturn trampolines may need to be written out by hand, as they would need to be usable as continuations of stub calls. It looks like the stub call generation code that we have at present generates both the pre-call and post-call code, and forceReturn trampolines need to start with post-call code. However, there would only need to be one trampoline; the Trampoline and TrampolineCompiler types could go away.
Reporter | ||
Comment 3•13 years ago
|
||
Also, JaegerThrowpoline would need to be become a JaegerExitVMFramePoline, only used in the (common) case where js_InternalThrow kicks everything into the interpreter.
Reporter | ||
Comment 4•13 years ago
|
||
All that said, this doesn't seem like a high-priority investment given that IonMonkey is on the way.
Assignee | ||
Updated•10 years ago
|
Assignee: general → nobody
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•