602067 - JS call logging gets mismatched entry/exit even in simple cases

Status: RESOLVED DUPLICATE of bug 605330

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

The mechanism in bug 507012 has bit-rotted a bit. Even in simple cases (no JM, no exceptions or deep bails) it no longer gets matching entries and exits.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Add JSOP_BEGIN to CompileScript-generated code

WIP - Add a JSOP_BEGIN to CompileScript-generated code. Now, anything that ends with a JSOP_STOP/RETURN/SETRVAL starts with a JSOP_BEGIN. Before this, I had the case where Execute() would log an entry to top-level code, then that code would log an exit when it hit JSOP_STOP. So I couldn't lexically wrap the start/stop in Execute(). More importantly, other callers of RunScript would not log the enter.

Having written that, I'm wondering why the other callers can't log the enter. There is at least one outside of jsinterp.cpp, so it's brittle, but it might allow eliminating JSOP_BEGIN.

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: Add a debug arg to JS call enter/exit probes

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: Fix JS call entry/exit matching

Make the JS function entry/exit callback receive a balanced number of enters and exits. Does not yet work with JaegerMonkey.

Main change: switch from caller-based probes to callee-based probes. Previously, I was wrapping calls to functions with Probes::enterJSFun/exitJSFun. Now, I do nothing at the call site, but instead put enterJSFun() at the top of functions and exitJSFun() at the exits. This was for a number of reasons:

 - The exits must be at the RETURN/STOP/RETRVAL points anyway, because there's nothing to lexically wrap with probes. This avoids doing the mismatch of caller-enters, callee-exits.
 - It turns out that doing anything just after a function call is a bad place, because there are some weird fixups that happen to make sure things don't get re-executed when bailing.
 - There are fewer caller sites within the implementation than callee sites.
 - There are fewer callers than callees in the TM-generated code
 - One use of these callbacks is to log whether we are currently on trace (and later, method jitted). For a function doStuff(), it's more useful to know whether the *body* is traced than the *call* to it.

Finally, this patch also fixes the tracking when exceptions or other errors are encountered. Previously, they would lose exits.

I attempted to handle the JM case as well, but my initial attempts are getting nowhere. I don't understand all the ways it can bail out, probably. Considering how long it's taken me to just fix up the interpreter + TM, I think I'll need help doing JM.

Help!

Comment 4

[Steve Fink [:sfink] [:s:]]

Attachment: Move everything over to Probes::

Not strictly part of this bug, but these patches are more FYI anyway since dvander is going to be doing a version of this: switch the TM tracking over to using Probes::callTrackingActive(), which will return true for more than just MOZ_TRACE_JSCALLS. This is now possible because I ripped out all fp inspection from the dtrace probes, and ETW never had it. It isn't the "right" way to do the dtrace stuff, because dtrace should be fully dynamic, and this will only be a fully dynamic disable (if you enable dynamically but things have already been trace-JITted, you won't get any hits.)

Because of dtrace's low runtime overhead, it really *could* be fully dynamic. But that would require injecting nop's into the JITted code and then registering them with the dtrace in-kernel "service", which sounds Hard and is not portable across dtrace implementations AFAIK. </ramble>