Personal tools

Javascript:SpiderMonkey:ExactStackRooting

From MozillaWiki

Jump to: navigation, search

Contents

Goal

The goal of this project is to know the exact locations of all GC Cell pointers stored on the stack. This will enable further development of compacting and generational GC modes.

Dynamic Rooting Analysis

Since a failure to root a stack variable properly will become a use-after-free error after we have moving GC's, it is vitally important that we catch and fix all rooting violations now, rather than later. To this end, we have enabled a dynamic correctness checker that runs on every push to mozilla-inbound. It verifies that no stack pointers are left unrooted. It is displayed on tbpl under the name SM(r).

Static Rooting Analysis

Tbpl can run two static analysis builds, one for the full browser (linux64-br-haz) and one for just the JS shell (linux64-sh-haz). The former shows up on tbpl as SM(Hf), the latter as SM(Hs):

  • Hf - hazard build for firefox browser
  • Hs - hazard build for JS shell
  • Hb - hazard build for B2G (in progress)
  • Hm - hazard build for mobile firefox browser (in progress)

These builds are performed as follows:

  • run the mozharness script http://hg.mozilla.org/build/mozharness/scripts/spidermonkey_build.py, which sets up a mozilla_mock environment and runs the analysis within it, then uploads the resulting files
    • compile an optimized JS shell to later run the analysis
    • compile the browser with gcc, using a slightly modified version of the sixgill (http://svn.sixgill.org) gcc plugin, producing a set of .xdb files describing everything encountered during the compilation
    • analyze the .xdb files with scripts in js/src/devtools/rootAnalysis

Running the analysis

Theoretically, you could run the analysis yourself by checking out mozharness and running

 python mozharness/scripts/spidermonkey_build.py -c hazards/common.py -c hazards/build_browser.py -c users/sfink/spidermonkey.py

but you'll need a bunch of dependencies that aren't bundled up in any convenient way, and you'll need to edit that last config file for your environment.

The far easier way to run an analysis is to push to try with the trychooser line |try: -b do -p linux64-br-haz -u none| (or, if the hazards of interest are contained entirely within js/src, use |try: -b do -p linux64-sh-haz -u none| for a much faster result). The expected turnaround time for linux64-br-haz is just under 2 hours.

The output will be uploaded to a path similar to http://stage.mozilla.org/pub/mozilla.org/firefox/try-builds/sfink@mozilla.com-a5d4f2abfda378eb57e4b6bbc5bb9d5186e0d62d/try-linux64-br-haz/ . At the moment, you'll need to dig that path out of the log file. (I will be adding it to the summary information displayed on tbpl "soon".)

If the analysis fails, you will see a message like this near the end of the log file:

10:01:31     INFO - #####
10:01:31     INFO - ##### Running check-expectations step.
10:01:31     INFO - #####
10:01:31     INFO - Running main action method: check_expectations
10:01:31     INFO - Reading from file /builds/slave/l64-sh-haz_try_dep-00000000000/build/source/js/src/devtools/rootAnalysis/expect.shell.json
10:01:31     INFO - Contents:
10:01:31     INFO -  {
10:01:31     INFO -    "expect-hazards": 1
10:01:31     INFO -  }
10:01:31     INFO - Reading from file /builds/slave/l64-sh-haz_try_dep-00000000000/build/analysis/rootingHazards.txt
10:01:31  WARNING - 3 more hazards than expected (expected 1, saw 4)
10:01:31  WARNING - # TBPL WARNING #

Analysis output

The main output file of interest is hazards.txt. Example snippet:

Function 'jsopcode.cpp:uint8 DecompileExpressionFromStack(JSContext*, int32, int32, class JS::Handle<JS::Value>, int8**)' has unrooted 'ed' of type 'ExpressionDecompiler' live across GC call 'uint8 ExpressionDecompiler::decompilePC(uint8*)' at js/src/jsopcode.cpp:1866
    js/src/jsopcode.cpp:1866: Assume(74,75, !__temp_23*, true)
    js/src/jsopcode.cpp:1867: Assign(75,76, return := 0)
    js/src/jsopcode.cpp:1867: Call(76,77, ed.~ExpressionDecompiler())
GC Function: uint8 ExpressionDecompiler::decompilePC(uint8*)
    JSString* js::ValueToSource(JSContext*, class JS::Handle<JS::Value>)
    uint8 js::Invoke(JSContext*, JS::Value*, JS::Value*, uint32, JS::Value*, class JS::MutableHandle<JS::Value>)
    uint8 js::Invoke(JSContext*, JS::CallArgs, uint32)
    JSScript* JSFunction::getOrCreateScript(JSContext*)
    uint8 JSFunction::createScriptForLazilyInterpretedFunction(JSContext*, class JS::Handle<JSFunction*>)
    uint8 JSRuntime::cloneSelfHostedFunctionScript(JSContext*, class JS::Handle<js::PropertyName*>, class JS::Handle<JSFunction*>)
    JSScript* js::CloneScript(JSContext*, class JS::Handle<JSObject*>, class JS::Handle<JSFunction*>, const class JS::Handle<JSScript*>, uint32)
    JSObject* js::CloneStaticBlockObject(JSContext*, class JS::Handle<JSObject*>, class JS::Handle<js::StaticBlockObject*>)
    js::StaticBlockObject* js::StaticBlockObject::create(js::ExclusiveContext*)
    js::Shape* js::EmptyShape::getInitialShape(js::ExclusiveContext*, js::Class*, js::TaggedProto, JSObject*, JSObject*, uint32, uint32)
    js::Shape* js::EmptyShape::getInitialShape(js::ExclusiveContext*, js::Class*, js::TaggedProto, JSObject*, JSObject*, uint64, uint32)
    js::UnownedBaseShape* js::BaseShape::getUnowned(js::ExclusiveContext*, js::StackBaseShape*)
    js::BaseShape* js_NewGCBaseShape(js::ThreadSafeContext*) [with js::AllowGC allowGC = (js::AllowGC)1u]
    js::BaseShape* js::gc::NewGCThing(js::ThreadSafeContext*, uint32, uint64, uint32) [with T = js::BaseShape; js::AllowGC allowGC = (js::AllowGC)1u; size_t = long unsigned int]
    void js::gc::RunDebugGC(JSContext*)
    void js::MinorGC(JSRuntime*, uint32)
    GC

This means that a rooting hazard was discovered at js/src/jsopcode.cpp line 1866, in the function DecompileExpressionFromStack (it is prefixed with the filename because it's a static function.) The problem is that they're an unrooted variable 'ed' that holds an ExpressionDecompiler live across a call to decompilePC. "Live" means that the variable is used after the call to decompilePC returns. decompilePC may trigger a GC according to the static call stack given starting from the line beginning with "GC Function:". The hazard itself has some barely comprehensible Assume(...) and Call(...) gibberish that describes the exact path of the variable into the function call. That stuff is rarely useful -- usually, you'll only need to look at it if it's complaining about a temporary and you want to know where the temporary came from. The type 'ExpressionDecompiler' is believed to hold pointers to GC-controlled objects of some sort. The analysis currently does not describe the exact field it is worried about.

To unpack this a little, the analysis is saying the following can happen:

  • ExpressionDecompiler contains some pointer to a GC thing. For example, it might have a field 'obj' of type 'JSObject*'.
  • DecompileExpressionFromStack is called.
  • A pointer is stored in that field of the 'ed' variable.
  • decompilePC is invoked, which calls ValueToSource, which calls Invoke, which eventually calls js::MinorGC
  • during the resulting garbage collection, the object pointed to by ed.obj is moved to a different location. All pointers stored in the JS heap are updated automatically, as are all rooted pointers. ed.obj is not, because the GC doesn't know about it.
  • after decompilePC returns, something accesses ed.obj. This is now a stale pointer, and may refer to just about anything -- the wrong object, an invalid object, or whatever. Badness 10000, as TeX would say.

So you broke the analysis by adding a hazard. Now what?

Backout, fix the hazard, or (final resort) update the expected number of hazards in js/src/devtools/rootAnalysis/expect.browser.json.

The most common way to fix a hazard is to change the variable to be a Rooted type, as described in http://dxr.mozilla.org/mozilla-central/source/js/public/RootingAPI.h#l21

For more complicated cases, ask on #jsapi. If you don't get a response, ping sfink, terrence, or jonco, and for the really hairy stuff, billm.

But to do that, you have to identify the hazard. Right now, we still have quite a few known hazards. For now, you'll want to look through hazards.txt for hazards in files that your patch touched. You can also compare with the last good run's hazards.txt file to see which hazards were added.