Last Comment Bug 424558 - Use after free in decompiler
: Use after free in decompiler
Status: VERIFIED FIXED
[sg:moderate] fixed-in-tracemonkey
: testcase, valgrind, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Igor Bukanov
:
:
Mentors:
Depends on: 531746
Blocks: jsfunfuzz
  Show dependency treegraph
 
Reported: 2008-03-22 11:47 PDT by Bob Clary [:bc:]
Modified: 2011-08-10 16:28 PDT (History)
14 users (show)
bob: in‑litmus-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.4+
.4-fixed
.10+
.10-fixed


Attachments
test.js (4.93 KB, text/plain)
2008-03-22 11:47 PDT, Bob Clary [:bc:]
no flags Details
wip (1.32 KB, patch)
2008-03-24 10:19 PDT, Mats Palmgren (:mats)
brendan: review+
Details | Diff | Splinter Review
testcase 2 (548 bytes, text/javascript)
2009-10-09 13:05 PDT, Jesse Ruderman
no flags Details
fix v2 (29.51 KB, patch)
2009-11-25 14:26 PST, Igor Bukanov
no flags Details | Diff | Splinter Review
fix v3 (30.93 KB, patch)
2009-11-26 15:05 PST, Igor Bukanov
no flags Details | Diff | Splinter Review
fix v3 (31.57 KB, patch)
2009-11-27 03:13 PST, Igor Bukanov
no flags Details | Diff | Splinter Review
fix v4 (31.57 KB, patch)
2009-11-28 08:42 PST, Igor Bukanov
brendan: review+
mbeltzner: approval1.9.2.2-
Details | Diff | Splinter Review
tm-based nit-picked version of "wip" patch (1009 bytes, patch)
2010-03-14 14:07 PDT, Brendan Eich [:brendan]
brendan: review+
mbeltzner: approval1.9.2.4+
mbeltzner: approval1.9.1.10+
mbeltzner: approval1.9.0.19-
Details | Diff | Splinter Review

Description Bob Clary [:bc:] 2008-03-22 11:47:14 PDT
Created attachment 311171 [details]
test.js

CentOS 5 64 bit

==24943== Invalid read of size 1
==24943==    at 0x4A06990: memmove (mc_replace_strmem.c:503)
==24943==    by 0x4813DA: SprintPut (jsopcode.c:455)
==24943==    by 0x481435: SprintCString (jsopcode.c:463)
==24943==    by 0x48654F: Decompile (jsopcode.c:2079)
==24943==    by 0x48A04A: Decompile (jsopcode.c:2988)
==24943==    by 0x491EA6: DecompileCode (jsopcode.c:4606)
==24943==    by 0x4928D7: js_DecompileFunction (jsopcode.c:4786)
==24943==    by 0x41FBB0: JS_DecompileFunction (jsapi.c:4830)
==24943==    by 0x45B963: fun_toStringHelper (jsfun.c:1491)
==24943==    by 0x45B9E7: fun_toSource (jsfun.c:1508)
==24943==    by 0x467FF6: js_Invoke (jsinterp.c:1186)
Comment 1 Bob Clary [:bc:] 2008-03-22 12:08:24 PDT
appears to be 64 bit only.
Comment 2 Mats Palmgren (:mats) 2008-03-24 10:17:04 PDT
's' is within the buffer, then SprintEnsureBuffer() realloc's the
buffer making 's' point to free'd memory.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/js/src/jsopcode.c&rev=3.307&root=/cvsroot&mark=448,455#417

With some debug printf's:

sp->base=0x55abb48 sp->offset=985 sp->size=989 s=0x55abeed
  (missing 49 bytes to fit this string at sp->offset)
  ('s' is 933 bytes into sp->base, 3 bytes follows it. 's' does not overlap with sp->offset)
len=52: (code.indexOf("<") == -1 || code.indexOf(".") == -1)
sp->base was reallocated!, now at 0x55ac080 with size 1038

==6994== Invalid read of size 1
==6994==    at 0x4C23290: memmove (mc_replace_strmem.c:516)
==6994==    by 0x458276: SprintPut (jsopcode.c:466)
==6994==    by 0x4582BC: SprintCString (jsopcode.c:474)
==6994==    by 0x45A1BA: Decompile (jsopcode.c:2090)
==6994==    by 0x45C874: Decompile (jsopcode.c:2999)
==6994==    by 0x461D67: DecompileCode (jsopcode.c:4617)
==6994==    by 0x4638D3: js_DecompileFunction (jsopcode.c:4797)
==6994==    by 0x40C993: JS_DecompileFunction (jsapi.c:4835)
==6994==    by 0x43F919: fun_toStringHelper (jsfun.c:1485)
==6994==    by 0x43F966: fun_toSource (jsfun.c:1502)
==6994==    by 0x447C55: js_Invoke (jsinterp.c:1186)
==6994==    by 0x4473FC: js_InternalInvoke (jsinterp.c:1359)
==6994==    by 0x44F6CD: js_TryMethod (jsobj.c:4750)
==6994==    by 0x4857A9: js_ValueToSource (jsstr.c:2733)
==6994==    by 0x4568B6: obj_toSource (jsobj.c:802)
==6994==    by 0x447C55: js_Invoke (jsinterp.c:1186)
==6994==    by 0x4473FC: js_InternalInvoke (jsinterp.c:1359)
==6994==    by 0x44F6CD: js_TryMethod (jsobj.c:4750)
==6994==    by 0x4857A9: js_ValueToSource (jsstr.c:2733)
==6994==    by 0x485814: str_uneval (jsstr.c:470)
==6994==    by 0x4A7404: js_Interpret (jsinterp.c:4820)
==6994==    by 0x44729A: js_Execute (jsinterp.c:1526)
==6994==    by 0x40C777: JS_ExecuteScript (jsapi.c:4876)
==6994==    by 0x408DCC: Process (js.c:272)
==6994==    by 0x409730: main (js.c:500)
==6994==  Address 0x55abf20 is 1,016 bytes inside a block of size 1,063 free'd
==6994==    at 0x4C22082: realloc (vg_replace_malloc.c:429)
==6994==    by 0x4186BC: JS_ArenaRealloc (jsarena.c:230)
==6994==    by 0x45800E: SprintEnsureBuffer (jsopcode.c:430)
==6994==    by 0x45822F: SprintPut (jsopcode.c:456)
==6994==    by 0x4582BC: SprintCString (jsopcode.c:474)
==6994==    by 0x45A1BA: Decompile (jsopcode.c:2090)
==6994==    by 0x45C874: Decompile (jsopcode.c:2999)
==6994==    by 0x461D67: DecompileCode (jsopcode.c:4617)
==6994==    by 0x4638D3: js_DecompileFunction (jsopcode.c:4797)
==6994==    by 0x40C993: JS_DecompileFunction (jsapi.c:4835)
==6994==    by 0x43F919: fun_toStringHelper (jsfun.c:1485)
==6994==    by 0x43F966: fun_toSource (jsfun.c:1502)
==6994==    by 0x447C55: js_Invoke (jsinterp.c:1186)
==6994==    by 0x4473FC: js_InternalInvoke (jsinterp.c:1359)
==6994==    by 0x44F6CD: js_TryMethod (jsobj.c:4750)
==6994==    by 0x4857A9: js_ValueToSource (jsstr.c:2733)
==6994==    by 0x4568B6: obj_toSource (jsobj.c:802)
==6994==    by 0x447C55: js_Invoke (jsinterp.c:1186)
==6994==    by 0x4473FC: js_InternalInvoke (jsinterp.c:1359)
==6994==    by 0x44F6CD: js_TryMethod (jsobj.c:4750)
==6994==    by 0x4857A9: js_ValueToSource (jsstr.c:2733)
==6994==    by 0x485814: str_uneval (jsstr.c:470)
==6994==    by 0x4A7404: js_Interpret (jsinterp.c:4820)
==6994==    by 0x44729A: js_Execute (jsinterp.c:1526)
==6994==    by 0x40C777: JS_ExecuteScript (jsapi.c:4876)
==6994==    by 0x408DCC: Process (js.c:272)
==6994==    by 0x409730: main (js.c:500)
Comment 3 Mats Palmgren (:mats) 2008-03-24 10:19:49 PDT
Created attachment 311400 [details] [diff] [review]
wip

FWIW, this fixes the testcase, I haven't investigated other callers of
SprintEnsureBuffer() though...
Comment 4 Jesse Ruderman 2009-10-09 13:05:43 PDT
Created attachment 405558 [details]
testcase 2

This triggers the same Valgrind warning on 32-bit Mac.
Comment 5 Igor Bukanov 2009-11-25 14:26:20 PST
Created attachment 414568 [details] [diff] [review]
fix v2

Backup of an untested patch.
Comment 6 Gary Kwong [:gkw] [:nth10sd] 2009-11-25 21:42:07 PST
(In reply to comment #5)
> Created an attachment (id=414568) [details]
> fix v2
> 
> Backup of an untested patch.

Drive-by fuzzing of 64-bit js shell binary of this patch on TM tip on 10.6.2 without -j:

a += 0;

asserts at Assertion failure: s + len <= sp->base, at ../jsopcode.cpp:563


=====

$ ./js-dbg-tm-darwin 
js> a += 0;
Assertion failure: s + len <= sp->base, at ../jsopcode.cpp:563
Abort trap
Comment 7 Igor Bukanov 2009-11-26 15:05:33 PST
Created attachment 414777 [details] [diff] [review]
fix v3

The fix changes SprintCString to insist that the function is never called with a string pointing into the buffer that could be reallocated. This required to fix a few its callers to use offsets rather then char* strings and rely on a special SprintOffset when printing. 

Strictly speaking the fix from the comment 3 would also work as it turned (at least AFAICS) there is no code that does something like:

lval = POP_STR();
rval = POP_STR();

SprintCString(lval)
SprintCString(rval)

That type of code would defeat the minimal fix as the code could reallocate twice defeating the checks there. 

With a bigger fix from this patch that would be detected immediately via an assert in SprintCString.

Gary, could you fuzz this?
Comment 8 Igor Bukanov 2009-11-26 15:10:45 PST
Regarding severity of this bug. As a consequences of this bug code can read (and only read!) memory that is freed when realloc moves the allocated memory to a bigger allocation size. In an embedding with many threads that could be used in principle to read the memory allocated by other threads.
Comment 9 Gary Kwong [:gkw] [:nth10sd] 2009-11-26 15:42:44 PST
(In reply to comment #7)
> Created an attachment (id=414777) [details]
> fix v3

var f = new Function("for (var z = 0; z < 21; ++z) { if (z % 3 == 0) { yield function ([y]) { }; } else {}  } ");
try {
    g = eval("(" + "" + f + ")");
    "" + g;
} catch(e) {
    var a = b;
}


throws a typein:6: ReferenceError: b is not defined in 64-bit js debug shells on 10.6 without -j on TM tip, but does not throw an error without this patch.

Is this intended?


===

$ ./js-dbg-tm-darwin 
js> var f = new Function("for (var z = 0; z < 21; ++z) { if (z % 3 == 0) { yield function ([y]) { }; } else {}  } ");
js> try {
    g = eval("(" + "" + f + ")");
    "" + g;
} catch(e) {
    var a = b;
}
typein:6: ReferenceError: b is not defined
js>
Comment 10 Igor Bukanov 2009-11-27 01:54:18 PST
(In reply to comment #9)
> throws a typein:6: ReferenceError: b is not defined in 64-bit js debug shells
> on 10.6 without -j on TM tip, but does not throw an error without this patch.

This is a bug that is covered by 2 tests under js/src/tests/. I have not find it since I have generated the base failures with the patch applied :(
Comment 11 Igor Bukanov 2009-11-27 03:13:49 PST
Created attachment 414832 [details] [diff] [review]
fix v3

The new version fixes the regression from the comment 9 and improves the assert coverage.
Comment 12 Gary Kwong [:gkw] [:nth10sd] 2009-11-27 07:03:35 PST
(In reply to comment #11)
> Created an attachment (id=414832) [details]
> fix v3
> 
> The new version fixes the regression from the comment 9 and improves the assert
> coverage.

This patch seems pretty good. The fuzzers aren't showing up anything after a few hours of work. :)
Comment 13 Brendan Eich [:brendan] 2009-11-27 13:48:42 PST
Comment on attachment 414832 [details] [diff] [review]
fix v3

>+        JS_ASSERT(sp->size > 0);

Use != 0 for unsigned types to avoid a warning (historically this can get you a warning; it's also perilously close to the degenerate unsigned comparison error).

>+    /*
>+     * Check s + len does not overflow. We use that s != 0 implies s - 1 < s
>+     * using unsigned arithmetic.

"We use that P implies Q" should be reworded, either to "Note that P implies Q" or "We use the fact that P implies Q" -- the former is shorter.

>+SprintOffset(Sprinter *sp, ptrdiff_t offset)
>+{
>+    size_t len;
>+    const char *chars;
>+    char *bp;
>+
>+    JS_ASSERT(offset >= 0);
>+    JS_ASSERT((size_t) offset < sp->size);
>+    len = strlen(OFF2STR(sp, offset));
>+
>+    /*
>+     * Check that 0 that terminates the string starting at off is inside the

s/that 0/that the 0/
s/that terminates/terminating/

Patch seems fine, wondering if there isn't a less invasive one that copes with realloc moving the buffer that SprintCString is both reading from and writing to. That would minimize the change and be no less safe AFAICT.

/be
Comment 14 Igor Bukanov 2009-11-28 08:24:26 PST
(In reply to comment #13)
> Patch seems fine, wondering if there isn't a less invasive one that copes with
> realloc moving the buffer that SprintCString is both reading from and writing
> to.

The minimal version is in the attachment from the comment 3. It works since as far as I can see after checking the code few times there is no sequences like

    lval = POP_STR();
    rval = POP_STR();
    SprintCString(lval)
    SprintCString(rval)

Such sequences may realloc the buffer twice defeating the checks proposed in the minimal fix. 

Now, the minimal fix does not guarantee that future modifications of the code would not introduce the above sequences resurrecting the bug. To protect against that via an assert coverage on the hot path in the decompiler I do not see how it is possible to be less invasive than the patch from the comment 11.
Comment 15 Igor Bukanov 2009-11-28 08:42:40 PST
Created attachment 414917 [details] [diff] [review]
fix v4

The new version addresses the comment 13.
Comment 16 Brendan Eich [:brendan] 2009-11-28 10:12:18 PST
(In reply to comment #14)
> (In reply to comment #13)
> > Patch seems fine, wondering if there isn't a less invasive one that copes with
> > realloc moving the buffer that SprintCString is both reading from and writing
> > to.
> 
> The minimal version is in the attachment from the comment 3. It works since as
> far as I can see after checking the code few times there is no sequences like
> 
>     lval = POP_STR();
>     rval = POP_STR();
>     SprintCString(lval)
>     SprintCString(rval)

That's right, by design the only safe pattern is pop*;push where * is Kleene closure and pop is POP_STR and variants, push is todo = Sprint* | PushOff(todo).

This is an old design and it's unchecked, so unreliable. On the other hand, we have not had many bugs in it because it fits the pattern of the decompiler's abstract interpreter. I'm therefore inclined toward a minimal patch with asserts.

> To protect
> against that via an assert coverage on the hot path in the decompiler I do not
> see how it is possible to be less invasive than the patch from the comment 11.

Hmm, it is hard to check. An assertion that no two SprintCString calls, both taking arguments on the sprint stack, occur without an intervening PopOff, might be enough.

I'll review the bigger patch in detail.

/be
Comment 17 Brendan Eich [:brendan] 2009-11-28 10:35:10 PST
Comment on attachment 414917 [details] [diff] [review]
fix v4

>+#define POP_OFF()             PopOff(ss, op)

No problem having a macro for symmetry but please #undef in the right order at the bottom of the function.

Another way to go would be a C++ auto-storage class helper or two, e.g., a StringOffset type, to sugar the OFF2STR a bit. But this can wait for another bug, if it is worth doing at all.

r=me with that, thanks.

/be
Comment 19 Igor Bukanov 2009-11-30 04:36:19 PST
I backed out the patch due to regression in the bug 531746. Instead I suggest to land the patch from the comment 3.
Comment 20 Igor Bukanov 2009-11-30 04:47:25 PST
Comment on attachment 311400 [details] [diff] [review]
wip

Minimal fix that works.
Comment 21 Brendan Eich [:brendan] 2009-11-30 09:32:26 PST
Comment on attachment 311400 [details] [diff] [review]
wip

>-    char *bp;
>+    ptrdiff_t offset = sp->size; /* save old size */
>+    char *bp = sp->base;         /* save old base */
> 
>     /* Allocate space for s, including the '\0' at the end. */
>     if (!SprintEnsureBuffer(sp, len))
>         return -1;
> 
>+    if (sp->base != bp && /* buffer was realloc'ed */
>+        s >= bp && s < bp + offset /* s was within the buffer */) {
>+        s = sp->base + (s - bp); /* this is where it lives now */

Please put the comments on the right of code, indented to start in the same c-basic-offset=4 column if possible.

What was the bug with the bigger patch? Switching to offsets instead of strings shouldn't have regressed anything if that's all that changed.

/be
Comment 22 Igor Bukanov 2009-11-30 12:04:39 PST
> What was the bug with the bigger patch? 

A single typo of using += instead of = in GetLocal has caused the regressin in the bug 531746. But there is a bigger problem with the patch. Right now GetLocal callers ignore OOM failures. That is no good since in couple of places that is feed right into SprintCString leading to straigh NULL deref (I will file a bug about it). But the patch made things worse since with it GetLocal returns -1 and now those unchecked callers will access Sprint->buf[-1].
Comment 24 Daniel Veditz [:dveditz] 2010-02-05 13:25:18 PST
Is this a regression or does it affect the 1.9.1 branch as well?
Comment 25 Bob Clary [:bc:] 2010-02-08 10:58:12 PST
test 2 reproduces the error on 64bit linux and mac 10.5 on 1.9.1.
Comment 26 Mike Beltzner [:beltzner, not reading bugmail] 2010-02-26 13:53:17 PST
Comment on attachment 414917 [details] [diff] [review]
fix v4

a1922=beltzner
Comment 28 Mats Palmgren (:mats) 2010-03-13 06:47:28 PST
The 2009-11-29 m-c checkin (comment 23) was backed out 2009-11-30:
https://hg.mozilla.org/mozilla-central/rev/e101dddbc432
AFAICT, the bug is still not fixed in mozilla-central, and probably
shouldn't have landed for 1.9.2.2 if I'm right.
Comment 29 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-13 12:57:13 PST
Comment on attachment 414917 [details] [diff] [review]
fix v4

Mats: thanks for noticing. That backout really should have been recorded here, and this bug should have been re-opened. This sort of sloppy recordkeeping gets us into a lot of trouble.
Comment 30 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-13 13:00:45 PST
Timeless: why did you check this patch in, by the way? It wasn't marked checkin-needed, and I don't see you anywhere else on the bug (did you even understand the patch) or a request from Igor to land it ...
Comment 31 Brendan Eich [:brendan] 2010-03-13 13:08:08 PST
What caused the backout?

/be
Comment 32 Robert Sayre 2010-03-13 13:18:41 PST
yikes, I must have missed the backout.
Comment 33 timeless 2010-03-13 17:05:55 PST
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/9ae6affe7bba
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/282811d159ab

perhaps i shouldn't have, i did intentionally select to include it. for trunk, i had recently pushed a change of igor's which had sat waiting for 11 days. personally for my changes i appreciate it when others push for me.
there was no indication in the bug of any reason not to push it, and had the necessary stamps.
Comment 34 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-14 12:48:13 PDT
timeless: in the future, unless it includes the "checkin-needed" keyword, please make sure to check with patch owners before checking things in. Of course there was no way for you to tell from reading this bug, but generally it's better form.
Comment 35 Igor Bukanov 2010-03-14 13:26:23 PDT
(In reply to comment #32)
> yikes, I must have missed the backout.

It is my fault. I forgot to land the small patch from the comment 3 and forgot to cite the changeset for bigger patch backout. So the big patch should be taken out and the small should be landed instead with nit fixed.
Comment 36 Brendan Eich [:brendan] 2010-03-14 14:07:46 PDT
Created attachment 432446 [details] [diff] [review]
tm-based nit-picked version of "wip" patch

I will land this on tm right now.

/be
Comment 37 Brendan Eich [:brendan] 2010-03-14 14:13:44 PDT
http://hg.mozilla.org/tracemonkey/rev/eea1a473c074

/be
Comment 38 Brendan Eich [:brendan] 2010-03-22 10:05:03 PDT
Is this waiting on a tm->m-c merge? Or could the patch go into the release repo/branch(s/es, I'm not remembering the details) too?

/be
Comment 39 Mats Palmgren (:mats) 2010-04-05 04:25:18 PDT
Please land on trunk ASAP so we can get this onto branches soon...
Comment 40 Reed Loden [:reed] (use needinfo?) 2010-04-05 04:35:55 PDT
(In reply to comment #37)
> http://hg.mozilla.org/tracemonkey/rev/eea1a473c074

In the future, please correctly attribute patches to their actual authors. You can do this using the "-u" option to `hg commit`. Example: `hg commit -u "John Doe <jdoe@example.com>"`. The days of CVS are long past; Hg gives us far more abilities and features, so let's use them.
Comment 42 christian 2010-04-12 10:24:17 PDT
This is too late for 1.9.2.4 & 1.9.1.10. We'll want it for  .5 and .11
Comment 43 Reed Loden [:reed] (use needinfo?) 2010-04-12 10:30:42 PDT
(In reply to comment #42)
> This is too late for 1.9.2.4 & 1.9.1.10. We'll want it for  .5 and .11

So, shouldn't the blocking flags be set to "needed" or even ".5+" and ".11+"?
Comment 44 Al Billings [:abillings] 2010-04-12 10:50:03 PDT
We need the flags to exist first, Reed. If you want to make them, please do. :-)
Comment 45 Reed Loden [:reed] (use needinfo?) 2010-04-12 11:02:21 PDT
(In reply to comment #44)
> We need the flags to exist first, Reed. If you want to make them, please do.
> :-)

Done. In the future, please file a bug under mozilla.org :: Bugzilla: Other b.m.o Issues when you need new ones. If nobody asks us to make them, we won't ever know that they need to be created.
Comment 46 Mike Beltzner [:beltzner, not reading bugmail] 2010-04-23 13:39:55 PDT
Comment on attachment 432446 [details] [diff] [review]
tm-based nit-picked version of "wip" patch

a=beltzner for 1.9.1.10 and 1.9.2.4; for the latter, please land both on default and GECKO1924_20100413_RELBRANCH tags
Comment 48 christian 2010-04-26 15:35:47 PDT
We need this landed on 1.9.1 as well.
Comment 49 Al Billings [:abillings] 2010-04-26 15:54:20 PDT
BC, can you verify this for 1.9.1 (assuming it lands) and 1.9.2 with tonight's nightly builds?
Comment 50 Bob Clary [:bc:] 2010-04-27 10:31:03 PDT
verified 1.9.2, 1.9.3 with test case 2 on linux x86_64 and mac os x x86.
note that 1.9.1 shows the valgrind error in the unthreaded shell but not the threaded shell.
Comment 51 christian 2010-04-27 10:44:20 PDT
Bob, that means we don't need this on 1.9.1, correct?
Comment 52 Bob Clary [:bc:] 2010-04-27 10:47:57 PDT
yep
Comment 53 christian 2010-04-27 10:58:41 PDT
Awesome, removing blocking flags.
Comment 54 Daniel Veditz [:dveditz] 2010-04-27 11:09:03 PDT
lowering severity to reflect comment 8
Comment 55 Mats Palmgren (:mats) 2010-04-27 11:14:06 PDT
The absence of an error does not prove correctness.
I feel uncomfortable leaving SprintPut() as is on 1.9.1.
Comment 56 Bob Clary [:bc:] 2010-04-27 11:27:43 PDT
(In reply to comment #51)
> Bob, that means we don't need this on 1.9.1, correct?

crap. i misread this and missed the "don't".  We do need this on 1.9.1.

Christian and dveditz: I can't fix the flags. Can you reset them to their original values?
Comment 57 christian 2010-04-27 11:29:55 PDT
Blocking it is then. Let's get this landed on 1.9.1 to be safe. Code complete for 1.9.1 is tonight @ 11:59 pm PST.
Comment 59 Bob Clary [:bc:] 2010-04-27 13:43:13 PDT
v 1.9.1 linux x86_64, mac os x x86 using threaded/nonthreaded shell.

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