482800 - TM: make sure we don't ever leave a tree through record_LeaveFrame

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Comment 1

[Andreas Gal :gal]

Attachment: assert in debug builds if this happens

Comment 2

[Andreas Gal :gal]

As I suspected via fall into LeaveFrame subsequent to a JSOP_STOP, which does not terminate the loop since we don't expect it to appear inside a loop. We must not have noticed when we walked out of the loop. We need to investigate why. JSOP_STOP is the wrong place to fix this bug.

trying to attach another branch to the tree (hits = 1)
recording starting from http://dromaeo.com/tests/sunspider-string-fasta.html:50@14
globalObj=0x1dde6280, shape=23896
import vp=0xf39524 name=$global0 type=string flags=0
import vp=0xd490cc name=$callee0 type=object flags=0
import vp=0xd490d0 name=$this0 type=object flags=0
import vp=0xd490d4 name=$fastaRepeat.n type=int flags=0
import vp=0xd490d8 name=$fastaRepeat.seq type=string flags=0
import vp=0xd4915c name=$fastaRepeat.seqi type=int flags=0
import vp=0xd49160 name=$fastaRepeat.lenOut type=int flags=0
import vp=0xd49164 name=$fastaRepeat.s type=string flags=0
import vp=0xd49168 name=$stack0 type=int flags=0
import vp=0xd4916c name=$stack1 type=int flags=0
00021:  51  lt
    start
    state = param 0 ecx
    0
    sp = ld state[0]
    xt3882
    rp = ld state[xt3882]
    12
    js_UnboxDouble1332 = ld state[12]
    8
    fmul503 = ld state[8]
    16
    eos = ld state[16]
    fadd383
    eq13318 = ld state[fadd383]
    xf14493
    globalObj = ld state[xf14493]
    616
    ATOM_TO_STRING(atom) = ld fmul503[616]
    -56
    $callee0 = ld sp[-56]
    -48
    js_ConcatStrings601 = ld sp[-48]
    eq13319
    ld23291 = ld sp[eq13319]
    $fastaRepeat.n = i2f ld23291
    -32
    $fastaRepeat.seq = ld sp[-32]
    -24
    ld23292 = ld sp[-24]
    $fastaRepeat.seqi = i2f ld23292
    -16
    ld23293 = ld sp[-16]
    $fastaRepeat.lenOut = i2f ld23293
    -8
    $fastaRepeat.s = ld sp[-8]
    ld23294 = ld sp[0]
    $stack0 = i2f ld23294
    ld23295 = ld sp[8]
    $stack1 = i2f ld23295
    lt1144 = lt ld23294, ld23295
    xf14676: xf lt1144 -> pc=0x1e2f5285 imacpc=0x0 sp+16 rp+0

00025:  51  getarg 0
00028:  51  setlocal 1
00032:  52  getlocal 0
00035:  52  getlocal 1
00038:  52  add
    sti sp[0] = lt1144
    sti sp[0] = ld23291
    sti sp[-16] = ld23291
    sti sp[0] = ld23292
    sti sp[8] = ld23291
    add2705 = add ld23292, ld23291
    ov990 = ov add2705
    xt9488: xt ov990 -> pc=0x1e2f5296 imacpc=0x0 sp+16 rp+0

00039:  52  getarg 1
00042:  52  length
00043:  52  lt
    i2f1690 = i2f add2705
    sti sp[0] = add2705
    sti sp[8] = $fastaRepeat.seq
    ld23296 = ld $fastaRepeat.seq[0]
    JSSTRING_LENGTH_MASK
    and9175 = and ld23296, JSSTRING_LENGTH_MASK
    JSSTRDEP_LENGTH_MASK
    and9176 = and ld23296, JSSTRDEP_LENGTH_MASK
    JSSTRFLAG_PREFIX
    xf14494 = and ld23296, JSSTRFLAG_PREFIX
    eq13473 = eq xf14494, 0
    cmov28 = cmov eq13473 ? and9176 : and9175
    JSSTRFLAG_DEPENDENT
    and9177 = and ld23296, JSSTRFLAG_DEPENDENT
    eq13474 = eq and9177, 0
    cmov29 = cmov eq13474 ? and9175 : cmov28
    shape = i2f cmov29
    sti sp[8] = cmov29
    guard(kshape) = lt add2705, cmov29
    eos: xf guard(kshape) -> pc=0x1e2f529b imacpc=0x0 sp+16 rp+0

00047:  53  bindname "ret"
00050:  53  getarg 1
00053:  53  callprop "substring"
    sti sp[0] = guard(kshape)
    eor
    ld23297 = ld js_UnboxDouble1332[eor]
    globalObj
    ld23298 = ld ld23297[globalObj]
    $callee0 = ld ld23298[0]
    sti sp[0] = ld23298
    sti sp[8] = $fastaRepeat.seq
    $anonymous.str
    ld10143 = ld $anonymous.str[0]
    ld23036 = ld ld10143[xt3882]
    ld10144 = ld ld23036[16]
    and4568
    guard(native-map) = eq ld10144, and4568
    ATOM_TO_STRING(atom): xf guard(native-map) -> pc=0x1e2f52a5 imacpc=0x0 sp+16 rp+0

    32
    shape = ld ld10143[32]
    #0x5d56
    js_ConcatStrings602 = eq shape, #0x5d56
    xt9348: xf js_ConcatStrings602 -> pc=0x1e2f52a5 imacpc=0x0 sp+16 rp+0

00056:  53  getlocal 0
00059:  53  getlocal 0
00062:  53  getlocal 1
00065:  53  add
00066:  53  call 2
    sti sp[16] = $fastaRepeat.seq
    PCVAL_TO_OBJECT(pcval)
    sti sp[8] = PCVAL_TO_OBJECT(pcval)
    sti sp[24] = ld23292
    sti sp[32] = ld23292
    sti sp[40] = ld23291
    sti sp[32] = add2705
    and4570 = String_p_substring ( js_UnboxDouble1332 $fastaRepeat.seq ld23292 add2705 )

    eq13475 = eq and4570, 0
    xt9489: xt eq13475 -> pc=0x1e2f52b2 imacpc=0x0 sp+40 rp+0

00069:  53  setname "ret"
00073:  54  getlocal 0
00076:  54  getlocal 1
00079:  54  add
00080:  54  setlocal 0
00084:  54  goto 139 (55)
00139:  60  getarg 0
00142:  60  getlocal 1
00145:  60  sub
    sti sp[8] = and4570
    st fmul503[616] = and4570
    sti sp[0] = ld23292
    sti sp[8] = ld23291
    sti sp[0] = add2705
    sti sp[-24] = add2705
    sti sp[0] = ld23291
    sti sp[8] = ld23291
    xt3884 = ov 0
    xt9490: xt xt3884 -> pc=0x1e2f5301 imacpc=0x0 sp+16 rp+0

00146:  60  setarg 0
00150:  50  getarg 0
00153:  50  zero
00154:  50  gt
00158:  50  stop
LeaveFrame (back to null), callDepth=0
Assertion failure: callDepth > 0, at ../../../js/src/jstracer.cpp:6180

Program received signal SIGTRAP, Trace/breakpoint trap.
JS_Assert (s=0x3f9d57 "callDepth > 0", file=0x3f8069 "../../../js/src/jstracer.cpp", ln=6180) at ../../../js/src/jsutil.cpp:68
68	    abort();
(gdb)

Comment 3

[Andreas Gal :gal]

Attachment: make sure we don't walk out of loops if the condition is constant

The previous patch asserted in fastaRepeat as expected, since thats where dmandelin observed the slowdown. The attached patch avoids walking out of the loop if the loop condition is constant (in which case fuseIf and flipIf were not called). I have no clue why the loop condition would be constant here though and I have not been able to create a shell test case. Help with that welcome.

Comment 4

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/5e0cc374593c

Comment 5

[Andreas Gal :gal]

Fails mochi tests. Graydon is backing out.

Thread 0 (crashed)
 0  libmozjs.so!TraceRecorder::nativeStackOffset(int*) const [jstracer.cpp:797ba90aed16 : 1387 + 0x6]
    eip = 0x009e938f   esp = 0xbfbd8904   ebp = 0xbfbd893c   ebx = 0x00a12608
    esi = 0x9f2ea614   edi = 0xab3ea880   eax = 0x00000008   ecx = 0x00000000
    edx = 0xffffffff   efl = 0x00010217
 1  libmozjs.so!TraceRecorder::set(int*, nanojit::LIns*, bool) [jstracer.cpp:797ba90aed16 : 1899 + 0xa]
    eip = 0x009ec420   esp = 0xbfbd8944   ebp = 0xbfbd896c
 2  libmozjs.so!TraceRecorder::record_LeaveFrame() [jstracer.cpp:797ba90aed16 : 6183 + 0x1b]
    eip = 0x009ecc15   esp = 0xbfbd8974   ebp = 0xbfbd898c
 3  libmozjs.so!js_Interpret [jsinterp.cpp:797ba90aed16 : 3074 + 0x1a]
    eip = 0x0099095c   esp = 0xbfbd8994   ebp = 0xbfbd8bfc
 4  libmozjs.so!js_Execute [jsinterp.cpp:797ba90aed16 : 1561 + 0xa]
    eip = 0x0099b886   esp = 0xbfbd8c04   ebp = 0xbfbd8c9c
 5  libmozjs.so!JS_EvaluateUCScriptForPrincipals [jsapi.cpp:797ba90aed16 : 5159 + 0x12]
    eip = 0x00963ff0   esp = 0xbfbd8ca4   ebp = 0xbfbd8cdc
 6  libxul.so!nsJSContext::EvaluateString(nsAString_internal const&, void*, nsIPrincipal*, char const*, unsigned int, unsigned int, nsAString_internal*, int*) [nsJSEnvironment.cpp:797ba90aed16 : 1603 + 0x1d]
    eip = 0x01345e10   esp = 0xbfbd8ce4   ebp = 0xbfbd8d7c
 7  libxul.so!nsScriptLoader::EvaluateScript(nsScriptLoadRequest*, nsString const&) [nsScriptLoader.cpp:797ba90aed16 : 671 + 0x33]
    eip = 0x01223fdf   esp = 0xbfbd8d84   ebp = 0xbfbd8e5c
 8  libxul.so!nsScriptLoader::ProcessRequest(nsScriptLoadRequest*) [nsScriptLoader.cpp:797ba90aed16 : 585 + 0xc]
    eip = 0x0122481b   esp = 0xbfbd8e64   ebp = 0xbfbd8f1c
 9  libxul.so!nsScriptLoader::ProcessPendingRequests() [nsScriptLoader.cpp:797ba90aed16 : 724 + 0xa]
    eip = 0x012248cd   esp = 0xbfbd8f24   ebp = 0xbfbd8f5c
10  libxul.so!nsRunnableMethod<nsScriptLoader, void>::Run() [nsThreadUtils.h : 264 + 0x10]
    eip = 0x0122597e   esp = 0xbfbd8f64   ebp = 0xbfbd8f7c
11  libxul.so!nsThread::ProcessNextEvent(int, int*) [nsThread.cpp:797ba90aed16 : 510 + 0x5]
    eip = 0x017b1a85   esp = 0xbfbd8f84   ebp = 0xbfbd8fbc
12  libxul.so!NS_ProcessNextEvent_P(nsIThread*, int) [nsThreadUtils.cpp : 230 + 0xd]
    eip = 0x0177f667   esp = 0xbfbd8fc4   ebp = 0xbfbd8fec
13  libxul.so!nsThread::Shutdown() [nsThread.cpp:797ba90aed16 : 465 + 0xb]
    eip = 0x017b1c9b   esp = 0xbfbd8ff4   ebp = 0xbfbd902c
14  libxul.so!NS_GetXPTCallStub_P + 0x30
    eip = 0x017be85f   esp = 0xbfbd9034   ebp = 0xbfbd904c
15  libxul.so!nsProxyObjectCallInfo::Run() [nsProxyEvent.cpp:797ba90aed16 : 181 + 0x13]
    eip = 0x017b65c9   esp = 0xbfbd9054   ebp = 0xbfbd906c
16  libxul.so!nsThread::ProcessNextEvent(int, int*) [nsThread.cpp:797ba90aed16 : 510 + 0x5]
    eip = 0x017b1a85   esp = 0xbfbd9074   ebp = 0xbfbd90ac
17  libxul.so!NS_ProcessNextEvent_P(nsIThread*, int) [nsThreadUtils.cpp : 230 + 0xd]
    eip = 0x0177f667   esp = 0xbfbd90b4   ebp = 0xbfbd90dc
18  libxul.so!nsThread::Shutdown() [nsThread.cpp:797ba90aed16 : 465 + 0xb]
    eip = 0x017b1c9b   esp = 0xbfbd90e4   ebp = 0xbfbd911c
19  libxul.so!NS_GetXPTCallStub_P + 0x30
    eip = 0x017be85f   esp = 0xbfbd9124   ebp = 0xbfbd9138
20  libxul.so!nsProxyObjectCallInfo::Run() [nsProxyEvent.cpp:797ba90aed16 : 181 + 0x13]
    eip = 0x017b65c9   esp = 0xbfbd9140   ebp = 0xbfbd9158
21  libxul.so!nsThread::ProcessNextEvent(int, int*) [nsThread.cpp:797ba90aed16 : 510 + 0x5]
    eip = 0x017b1a85   esp = 0xbfbd9160   ebp = 0xbfbd9198
22  libxul.so!NS_ProcessNextEvent_P(nsIThread*, int) [nsThreadUtils.cpp : 230 + 0xd]
    eip = 0x0177f667   esp = 0xbfbd91a0   ebp = 0xbfbd91c8
23  libxul.so!nsBaseAppShell::Run() [nsBaseAppShell.cpp:797ba90aed16 : 170 + 0x9]
    eip = 0x016d8bd6   esp = 0xbfbd91d0   ebp = 0xbfbd91e8
24  libxul.so!nsAppStartup::Run() [nsAppStartup.cpp:797ba90aed16 : 192 + 0x5]
    eip = 0x0159f28e   esp = 0xbfbd91f0   ebp = 0xbfbd9208
25  libxul.so!XRE_main [nsAppRunner.cpp:797ba90aed16 : 3220 + 0x7]
    eip = 0x00ef2b6f   esp = 0xbfbd9210   ebp = 0xbfbd9828
26  firefox-bin!main [nsBrowserApp.cpp:797ba90aed16 : 156 + 0xe]
    eip = 0x080495b1   esp = 0xbfbd9830   ebp = 0xbfbd9888
27  libc-2.5.so + 0x15deb
    eip = 0x00360dec   esp = 0xbfbd9890   ebp = 0xbfbd98f8

In DEBUG this would have asserted in LeaveFrame() I think. There must be another way to leave the loop without closing.

Comment 6

[Andreas Gal :gal]

Failing test:

/tests/dom/tests/mochitest/ajax/mochikit/test_Mochikit.html

Comment 7

[Robert Sayre]

http://mxr.mozilla.org/mozilla-central/source/dom/tests/mochitest/ajax/mochikit/tests/MochiKit-Selector.html?force=1#53

The crash occurs in the call to testExpected() on line 78, I think.

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 367094 [details] [diff] [review]
make sure we don't walk out of loops if the condition is constant

Belated nits:

>-    if (callDepth-- <= 0)
>-        ABORT_TRACE("returned out of a loop we started tracing");
>+    JS_ASSERT(callDepth > 0);
>+    callDepth--;

Was callDepth ever signed? It is unsigned in class TraceRecorder now, so the <= 0 test couldn't have worked and the compiler should have warned or errored.

Anyway, JS_ASSERT(callDepth != 0); seems strictly better for unsigned callDepth.

Also, --callDepth; is preferred style both among C++ iterator users and even old C farts like me.

/be

Comment 9

[Andreas Gal :gal]

Attachment: cleanup generation of branch and loop condition/edge code

This bug morphed into a major cleanup of the branch and loop condition/edge code. While debugging the failure in the previous patch I noticed that the unfused ifne/ifeq case could generate infinitive loops by not emitting a constant loop condition for a thin loop exit path:

function f() {
    for (var i = 0; i < 10; ++i)
	if (i > 5)
	    i = 10;
}

f();
f();
f();
f();
f();
f();

Here the loop condition of the 2nd trace is 10 < 10, which is constant folded. We always close the loop (jmp header), but since the guard is constant we don't emit it. As a result, the code runs a really, really long time.

The attached patch undos some of the early day design shortcuts. We can't de-allocate the recorder from within its instance methods, so we just kept stuffing all state handling that might result in a delete into monitorRecording, which is bad for many reasons, amongst others we have to communicate state between the record_JSOP_xxx part and monitorRecording using some weird one-shot instance fields. Most of that is gone now. monitorRecording merely detects when errors occured or when the trace was compiled, which now happens in the conditional/relational op in case of a fused loop, in the ifne/ifeq in case of an unfused loop, or on the GOTO in case of a break (endLoop). This will also greatly help with structuring the recorder for recursion.

Passes trace-tests.js and identical performance for the SS harness. 20ms speedup for bench.sh (not sure why, bench.sh has been noisy for me lately).

Comment 10

[Andreas Gal :gal]

Attachment: refreshed

Comment 11

[Brendan Eich [:brendan]]

Comment on attachment 368637 [details] [diff] [review]
refreshed

> TraceRecorder::closeLoop(JSTraceMonitor* tm, bool& demote)
> {
>+    JS_ASSERT((*cx->fp->regs->pc == JSOP_LOOP || *cx->fp->regs->pc == JSOP_NOP) && !cx->fp->imacpc);

Comment that loop header op is JSOP_LOOP or (once blacklisted) JSOP_NOP.

>     debug_only_v(printf("recording completed at %s:%u@%u via closeLoop\n",
>                         cx->fp->script->filename,
>                         js_FramePCToLineNumber(cx, cx->fp),
>                         FramePCOffset(cx->fp));)
>-    return true;
>+    return;
> }

Nit: no return; at bottom of function.

>+        exitType = LOOP_EXIT;
>+        /*

Space before major comment unless after {.

>+         * If we are about to walk out of the loop, generate code for the inverse loop
>+         * condition, pretending we recorded the case that stays on trace.
>+         */
>+        if ((*pc == JSOP_IFEQ || *pc == JSOP_IFEQX) == cond) {
>+            JS_ASSERT(*pc == JSOP_IFNE || *pc == JSOP_IFNEX || *pc == JSOP_IFEQ || *pc == JSOP_IFEQX);
>+            debug_only_v(printf("Walking out of the loop, terminating it anyway.\n");)
>+            cond = !cond;
>+        }
>+        /*

Ditto.

>+         * Conditional guards do not have to be emitted if the condition is constant. We
>+         * make a note whether the loop condition is true or false here, so we later know
>+         * whether to emit a loop edge or a loop end.
>+         */
>+        if (x->isconst()) {
>+            loop = (x->constval() == cond);
>+            return;
>+        }
>+    } else {
>+        exitType = BRANCH_EXIT;
>+    }
>+    if (!x->isconst())
>+        guard(cond, x, exitType);
> }
> 
> /* Emit code for a fused IFEQ/IFNE. */
> JS_REQUIRES_STACK void
> TraceRecorder::fuseIf(jsbytecode* pc, bool cond, LIns* x)
> {
>-    if (x->isconst()) // no need to guard if condition is constant
>-        return;
>-    if (*pc == JSOP_IFEQ) {
>-        flipIf(pc, cond);
>-        guard(cond, x, BRANCH_EXIT);
>-        trackCfgMerges(pc);
>-    } else if (*pc == JSOP_IFNE) {
>-        flipIf(pc, cond);
>-        guard(cond, x, BRANCH_EXIT);
>-    }
>+    if (*pc == JSOP_IFEQ || *pc == JSOP_IFNE) {
>+        emitIf(pc, cond, x);
>+        if (*pc == JSOP_IFEQ)
>+            trackCfgMerges(pc);
>+    }
>+}
>+
>+/* Check whether we have reached the end of the trace. */
>+JS_REQUIRES_STACK bool
>+TraceRecorder::checkTraceEnd(jsbytecode *pc)
>+{
>+    if (js_IsLoopEdge(pc, (jsbytecode*)fragment->root->ip)) {
>+        /*
>+         * If we compile a loop, the trace should have a zero stack balance at the loop
>+         * edge. Currently we are parked on a comparison op or IFNE/IFEQ, so advance
>+         * pc to the loop header and adjust the stack pointer and pretend we have
>+         * reached the loop header.
>+         */
>+        if (loop) {
>+            bool fused = pc != cx->fp->regs->pc;
>+            JS_ASSERT(!cx->fp->imacpc && (pc == cx->fp->regs->pc || pc == cx->fp->regs->pc + 1));
>+            JSFrameRegs orig = *cx->fp->regs;
>+            cx->fp->regs->pc = (jsbytecode*)fragment->root->ip;
>+            cx->fp->regs->sp -= fused ? 2 : 1;
>+            bool demote = false;
>+            closeLoop(traceMonitor, demote);
>+            *cx->fp->regs = orig;
>+            /*

Ditto on blank line before major comment, and: previous clump of decls and code could use some spacing. Put the assertion first, then a blank line and fused's decl and uses, then a blank line and demote's decl and use plus closeLoop call and orig restoration.

>+    switch (lr->exitType) {
>+    case LOOP_EXIT:

Half-indent case and goto labels.

>-      default: goto abort_recording;
>+      default: goto stop_recording;
> # define OPDEF(x,val,name,token,length,nuses,ndefs,prec,format)               \
>       case x:                                                                 \
>         debug_only_v(                                                         \
>             js_Disassemble1(cx, cx->fp->script, cx->fp->regs->pc,             \
>                             (cx->fp->imacpc)                                  \
>                             ? 0                                               \
>                             : cx->fp->regs->pc - cx->fp->script->code,        \
>                             !cx->fp->imacpc, stdout);)                        \

This debug_only_v is x-invariant so should go before the switch and not in each case via macro expansion. I thought jorendorff was gonna do this but it must be in a patch not ready to land. Thought I'd mention it in case you care to do the deed.

>+    jsbytecode* pc = cx->fp->regs->pc;
>     /*

Ditto.

>      * Don't guard if the same path is always taken.  If it isn't, we have to
>      * fuse comparisons and the following branch, because the interpreter does
>      * that.

(Context for above "Ditto".)

>+    jsbytecode* pc = cx->fp->regs->pc;
>     /*

Ditto.

>      * Don't guard if the same path is always taken.  If it isn't, we have to
>      * fuse comparisons and the following branch, because the interpreter does
>      * that.

(Context again.)

> TraceRecorder::record_JSOP_GOTO()
> {
>+    /*
>+     * If we hit a break, end the loop and generate an always taken loop exit guard.
>+     * For other downward gotos (like if/else) continue recording.
>+     */
>+    jssrcnote* sn = js_GetSrcNote(cx->fp->script, cx->fp->regs->pc);

Maybe a blank line here, and did you say js_GetSrcNote was showing up from here on profiles?

>+++ b/js/src/jstracer.h
>@@ -150,17 +150,17 @@ public:
>     bool            has(const void* v) const;
>     nanojit::LIns*  get(const void* v) const;
>     void            set(const void* v, nanojit::LIns* ins);
>     void            clear();
> };
> 
> #ifdef JS_JIT_SPEW
> extern bool js_verboseDebug;
>-#define debug_only_v(x) if (js_verboseDebug) { x; }
>+#define debug_only_v(x) if (js_verboseDebug) { x; fflush(stdout); }

Separate bug -- if we define debug_only_v now, we should fix it to allow semicolon outside the arg-list, which requires ((void) 0) for the not-defined stub:

> #else
> #define debug_only_v(x)
> #endif

r=me with nits picked.

/be

Comment 12

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

(In reply to comment #11)
> Maybe a blank line here, and did you say js_GetSrcNote was showing up from here
> on profiles?

I've seen it, yeah.

Comment 13

[Brendan Eich [:brendan]]

(In reply to comment #12)
> (In reply to comment #11)
> > Maybe a blank line here, and did you say js_GetSrcNote was showing up from here
> > on profiles?
> 
> I've seen it, yeah.

Trivial to add JSOP_BREAK as a synonym for JSOP_GOTO, just for the tracer. Bug?

/be

Comment 14

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I think gal filed it, based on our conversation in my last MV trip.

Comment 15

[Andreas Gal :gal]

That was supposed to read "properly". Landing over night. Way too tired to watch it cycle. Please back out for me if this looks bad.

http://hg.mozilla.org/tracemonkey/rev/14b568cd9c43

Comment 16

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/14b568cd9c43

Comment 17

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/00430742fab4