829243 - Statically determine functions which can GC

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

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 [inactive]]

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 [Laid off!]]

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]]

(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:]]

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:]]

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]]

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 [Laid off!]]

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 [inactive]]

> 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]]

Also, js::MinorGC should be in the list since it doesn't call Collect, even indirectly.

Comment 10

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

Ah, this is the bug I lost track of. Bug 895027 implements the suggestions in comments 8 and 9.

Comment 11

[Terrence Cole [:terrence]]

This seems to be working and quite stable now.