JS debugger needs a way to recover calls to JSNatives from script

NEW
Assigned to

Status

()

Core
JavaScript Engine
7 years ago
4 years ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

Other Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

7 years ago
JSNatives have no JSStackFrame. The debugger (and Error.stack for that matter) should show them, so we need a way to recover that information.

There are quite a few cases, but consider the basic case where a script actually explicitly, directly calls a native Function via JSOP_CALL.

The callee, this-value, and arguments are all on the stack at the moment the JSNative is first called. But the JSNative may then overwrite all those slots. Overwriting the callee slot--the one the debugger cares about the most--is in fact mandatory, since that's the return slot.

It seems like either

  * we change the JSNative contract to ban writing to vp[0] until
    after the native is done calling back into the engine; OR

  * we accept overhead, a write instruction to store the native callee
    redundantly somewhere else on the stack; OR

  * we accept that the stack can be wrong.

Comment 1

7 years ago
This question has come up several times in the last year of stack refactoring.  There have been several cases where "oh, but what if we were called by a native" has been a confounding case for our analysis.  For example, when constructor natives lost their stack frames, we got lucky that XPCCallContexts essentially remembers the native callee which allowed us to hack up nsXPConnect::GetCaller to make nsContentUtils::GetDocumentFromCaller work again.

Option 2 seems simplest (well, after Option 1).  However, when Waldo tried to solve a similar problem in bug 631135 with a technique that didn't even add any overhead to natives-called-from-jit or property-access-from-jit, still Dromaeo regressed.  So that makes me think we'd really feel it.

As for Option 1: I dislike vp[0] clobbering and I have had bugs where I rearranged some lines of code but failed to notice that I was reading a callee after an early rval-setting.  I have tried to remove uses of this in SM as I see them (most obviated by stack scanning).  In fact, CallArgs now asserts CallArgs::rval() doesn't precede CallArgs::callee().  So it would be cool to officially ban this.

The only technicality is that pushInvokeArgs/pushInvokeFrame can pile up multiple native calls on top of a single StackFrame so, to really catch 'em all, we'd need more than just regs.sp/pc snooping; StackSpace would have to do a bit of extra record-keeping, but this shouldn't be too hard or expensive.
(Assignee)

Comment 2

7 years ago
(In reply to comment #1)

Thanks for the thorough response.

> Option 2 seems simplest (well, after Option 1).

Hey, option 3 is simplest of all! ;)

> The only technicality is that pushInvokeArgs/pushInvokeFrame can pile up
> multiple native calls on top of a single StackFrame so, to really catch 'em
> all, we'd need more than just regs.sp/pc snooping; StackSpace would have to do
> a bit of extra record-keeping, but this shouldn't be too hard or expensive.

I was hoping you'd say this! I had come to the same conclusion. Script-to-native calls are surely the hottest case. They're almost all leaf calls.

Sounds like we should shoot for option 1. I'll look into it.

Comment 3

7 years ago
(In reply to comment #2)
> > Option 2 seems simplest (well, after Option 1).
> 
> Hey, option 3 is simplest of all! ;)

Oops, joke fail, I meant to say option 3...

> Sounds like we should shoot for option 1. I'll look into it.

Sweet, three birds, one bug!  If you handle the vp[0] clobbering (which is what I hope you mean by "I'll look into it" :), I'll write CalleeIter.

Comment 4

7 years ago
I too am in favor of option 1, and had been thinking about possibly implementing it at some point when I found the time.
(Assignee)

Comment 5

7 years ago
OK, narrowing this bug to be about calls to JSNatives from script.

Filed bug 656462 for calls to JSNatives from everywhere else.
Assignee: general → jorendorff
Summary: JS debugger API needs a way to recover JSNative stack frames → JS debugger needs a way to recover calls to JSNatives from script
You need to log in before you can comment on or make changes to this bug.