Last Comment Bug 829243 - Statically determine functions which can GC
: Statically determine functions which can GC
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal with 1 vote (vote)
: ---
Assigned To: general
:
Mentors:
Depends on: 895027
Blocks: 831409
  Show dependency treegraph
 
Reported: 2013-01-10 13:03 PST by Brian Hackett (:bhackett)
Modified: 2013-08-08 13:55 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Brian Hackett (:bhackett) 2013-01-10 13:03:30 PST
Automatically determining the set of functions which can GC takes a lot of pain and guesswork out of sprinkling AutoAssertNoGC and AssertCanGC around the codebase.  Doing this is also necessary for statically identifying rooting hazards (next step).

I wrote a few scripts, stored at https://github.com/bhackett1024/gcAnalysis, for this purpose.  These analyses use the sixgill frontend (http://sixgill.org, requires gcc 4.7 and talk to me if you're interested in running this yourself) to get CFGs and type information for SpiderMonkey, then do some rudimentary callgraph analysis.  Functions which can GC are (a) js::GC, (b) any function which makes a virtual or indirect call, and (c) their transitive callers.  Annotations are used to handle indirect calls which cannot GC, mainly various Ion data structures which are virtual functions.

I put up a list of identified functions, along with a call chain down to one of (a) or (b) above, at the following URL:

http://people.mozilla.org/~bhackett/gcFunctions.html

This currently identifies 5936 functions which can GC.  Right now this is just updated manually, so ping or email me if you want a more current list.  There will be some false positives for various reasons, which will improve after the rooting hazard analysis gets off the ground.
Comment 1 Nicholas Nethercote [:njn] 2013-01-10 17:06:20 PST
Cool.  How many functions don't GC?  My sense with this is that there are a few functions (e.g. ones relating to error reporting) which, if they could be made to never GC, would cut down this list a lot.
Comment 2 Brian Hackett (:bhackett) 2013-01-10 17:10:14 PST
Well, the frontend picked up about 35000 different function bodies when building spidermonkey, but that includes a lot of random template instantiations etc.  I agree, and using static analyses like this makes it easier to not just get the codebase rooted, but also make transformations (like forbidding GCs when generating exceptions) that cut down / shift the roots required.
Comment 3 Terrence Cole [:terrence] 2013-01-10 18:07:15 PST
(In reply to Brian Hackett (:bhackett) from comment #2)
> I agree, and using static analyses like this makes it
> easier to not just get the codebase rooted, but also make transformations
> (like forbidding GCs when generating exceptions) that cut down / shift the
> roots required.

That's the main reason I'm excited about this. Using AutoAssertNoGC to ensure we don't GC in the frontend/Compiler/Whatever is daunting, but doable. With this tool it should be almost easy.

Steve hooked this analysis up today to an IRC bot named mrgiggles. We can now ask mrgiggles if a function can GC and expect a reasonable answer back. This gets us about half of what I wanted AssertCanGC/AutoAssertNoGC for.
Comment 4 Steve Fink [:sfink] [:s:] 2013-01-10 20:37:02 PST
Yeah, I just loaded a snapshot of http://people.mozilla.org/~bhackett/gcFunctions.html into memory. People were playing with it this evening and noticed that it does not report js::Interpret as something that can GC.
Comment 5 Steve Fink [:sfink] [:s:] 2013-01-15 13:22:33 PST
Another problem: js::ctypes::GetDeflatedUTF8StringLength calls JS_ReportErrorFlagsAndNumber, which can GC, but the above HTML file does not list GetDeflatedUTF8StringLength.
Comment 6 Terrence Cole [:terrence] 2013-01-15 13:38:00 PST
As far as I know, ctypes is not built with the shell, so I imagine that's why the analysis missed it.
Comment 7 Brian Hackett (:bhackett) 2013-01-15 13:43:25 PST
Yeah, I'm just building the shell.  That will change as we start needing to add roots throughout the browser.  I just updated gcFunctions.html; the source rev is the same but this fixes some bugs in the frontend that were causing false negatives, including js::Interpret (edges in the CFG were being dropped so that Interpret looked like it had an empty body).
Comment 8 Nicholas Nethercote [:njn] 2013-02-03 15:46:14 PST
> Functions which can GC are (a) js::GC, (b) any function which makes a virtual or > indirect call, and (c) their transitive callers.

I think (a) should be Collect instead of js::GC.  Otherwise it could miss GCSlice, GCFinalSlice, js::GCDebugSlice, etc.

Note that the analysis currently *doesn't* miss them, because Collect is marked as can-GC because it contains an indirect call to a JSGCCallback, but I think that's more by luck than design;  it's possible that callback will be annotated in the future as no-GC.
Comment 9 Terrence Cole [:terrence] 2013-06-04 14:25:44 PDT
Also, js::MinorGC should be in the list since it doesn't call Collect, even indirectly.
Comment 10 Steve Fink [:sfink] [:s:] 2013-07-26 15:22:37 PDT
Ah, this is the bug I lost track of. Bug 895027 implements the suggestions in comments 8 and 9.
Comment 11 Terrence Cole [:terrence] 2013-08-08 13:55:35 PDT
This seems to be working and quite stable now.

Note You need to log in before you can comment on or make changes to this bug.