786130 - IonMonkey: Crash [@ js::SkipSpace] with use-after-free

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on ionmonkey revision 92b9b2840a79 (run with --ion -n -m --ion-eager -a):


var lfcode = new Array();
lfcode.push("\
gczeal(2);\
try {\
  xxxyyyzzz();\
} catch (e) {\
  var match = e.toString().search(/ReferenceError/);\
}\
");
lfcode.push("\
StrictEquality(true, new Boolean(true), false, 0);\
function StrictEquality(x, y, expect, i) {\
    result = ( /\\W+/   ^ expect+\".getUTCMinutes()\");\
}\
");
while (true) {
  var file = lfcode.shift(); if (file == undefined) { break; }
  loadFile(file);
}
function loadFile(lfVarx) {
  if (lfVarx.substr(-3) == ".js") {
  } else {
	evaluate(lfVarx);
  }
}

Comment 1

[Christian Holler (:decoder)]

Crash trace:


==10866== Invalid read of size 2
==10866==    at 0x818ECB2: js::SkipSpace(unsigned short const*, unsigned short const*) (jsstrinlines.h:100)
==10866==    by 0x819419B: bool js::StringToNumberType<double>(JSContext*, JSString*, double*) (jsnuminlines.h:54)
==10866==    by 0x8192A76: ToNumberSlow (jsnum.cpp:1375)
==10866==    by 0x8192D8E: ToInt32Slow (jsnum.cpp:1448)
==10866==    by 0x806BE6C: ToInt32 (jsapi.h:2866)
==10866==    by 0x858BF6B: js::BitXor(JSContext*, JS::Value const&, JS::Value const&, int*) (jsinterpinlines.h:895)
==10866==    by 0x78CD6AA: ???
==10866==  Address 0xdadadada is not stack'd, malloc'd or (recently) free'd

Comment 2

[Sean Stangl [:sstangl]]

Can't reproduce locally on x86 debug or x86_64 debug.

Comment 3

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision d16c4404e8c4).

Comment 4

[Christian Holler (:decoder)]

JSBugMon: Fix Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first good revision is:
changeset:   105607:6cd206b37176
parent:      104959:b63bb39ed1c0
parent:      105606:a0240c1043ee
user:        David Anderson
date:        Wed Aug 29 17:51:24 2012 -0700
summary:     Merge from mozilla-central.

Oops! We didn't test rev b63bb39ed1c0, a parent of the blamed revision! Let's do that now.
Rev b63bb39ed1c0: Updating... Compiling... Testing... gdb -n -batch -x /home/decoder/LangFuzz/fuzzing/util/gdb-quick.txt /home/decoder/Desktop/autobisect-cache/js-dbg-32-b63bb39ed1c0-linux core.5401
Exit status: CRASHED signal 11 (SIGSEGV) (0.032 seconds)
bad (interesting) 
As expected, the parent's label is the opposite of the blamed rev's label.

Oops! We didn't test rev a0240c1043ee, a parent of the blamed revision! Let's do that now.
We did not test rev a0240c1043ee because it is not a descendant of either 92b9b2840a79 or d16c4404e8c4.
Rev a0240c1043ee: Updating... Compiling... Testing... Exit status: ABNORMAL return code 1 (0.012 seconds)
good (not interesting) 
Bisect lied to us! Parent rev a0240c1043ee was also good!

Perhaps we should expand the search to include the common ancestor of the blamed changeset's parents.
The common ancestor of b63bb39ed1c0 and a0240c1043ee is 88e47f6905e9.
Rev 88e47f6905e9: Updating... Compiling... Testing... Exit status: ABNORMAL return code 1 (0.012 seconds)
good (not interesting) 
Try setting -s to 88e47f6905e9, and -e to b63bb39ed1c0, and re-run autoBisect.

Comment 5

[Christian Holler (:decoder)]

Gary: what autoBisect is telling here in the last line seems to be misleading. When I use "-s 88e47f6905e9 -e b63bb39ed1c0" I'm getting the first *bad* changeset, although we were bisecting for the fix before that.

Comment 6

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Christian Holler (:decoder) from comment #5)
> Gary: what autoBisect is telling here in the last line seems to be
> misleading. When I use "-s 88e47f6905e9 -e b63bb39ed1c0" I'm getting the
> first *bad* changeset, although we were bisecting for the fix before that.

Yes, you're right. It had never worked properly - I've disabled this print statement. Will have to think of some other way to do what I wanted it to do. :-/

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

I can reproduce this against the given cset, but not tip. It looks like a rooting bug. It's time to use HandleValue everywhere, since IonMonkey does not use the conservative scanner for its stack range.

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

Applying the BitXor part of this patch to the problematic cset, does in fact fix this bug.

Comment 9

[Sean Stangl [:sstangl]]

Comment on attachment 659838 [details] [diff] [review]
fix

Review of attachment 659838 [details] [diff] [review]:
-----------------------------------------------------------------

Oof, good catch.

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

https://hg.mozilla.org/projects/ionmonkey/rev/01f6ddbb6542

Comment 11

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 12

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

We made more improvements to autoBisectJs. Hope this shows up a better bisection result.

Comment 13

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Gary Kwong [:gkw, :nth10sd] from comment #12)
> We made more improvements to autoBisectJs. Hope this shows up a better
> bisection result.

Worth noting: with bugs like this there's not going to be a useful regressing cset, so even if auto-bisect finds something, it won't really be related.