1139368 - Crash [@ ??] with RegExp

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision c5b90c003be8 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --enable-debug, run with --ion-offthread-compile=off --ion-eager --no-threads):

function test() {
  for(var foo; i<2; i++) {
    var a = /a/;
  }
  for(var i=0; i<2; i++) {
    a.exec("aaa");
  }
}
test();



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7fdc831 in ?? ()
#0  0x00007ffff7fdc831 in ?? ()
#1  0x0000000000000000 in ?? ()
rax	0x1b33a10	28523024
rbx	0xfff9000000000000	-1970324836974592
rcx	0x0	0
rdx	0x1fff1	131057
rsi	0x7fffffffbbe8	140737488337896
rdi	0x7fffffffbbe8	140737488337896
rbp	0xfff9000000000000	18444773748872577024
rsp	0x7fffffffbbe8	140737488337896
r8	0x0	0
r9	0x0	0
r10	0x7fffffffbbe0	140737488337888
r11	0x7ffff7e50d00	140737352371456
r12	0x8	8
r13	0x7fffffffca80	140737488341632
r14	0x203	515
r15	0x7fffffffcc70	140737488342128
rip	0x7ffff7fdc831	140737353992241
=> 0x7ffff7fdc831:	mov    (%rcx),%rax
   0x7ffff7fdc834:	movabs $0x7ffff7e57760,%r11


The crash address is 0x0 but since this is a crash on the heap, I'm marking this s-s until investigated.

Comment 1

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
=== Treeherder Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20150303001933" and the hash "074919869975".
The "bad" changeset has the timestamp "20150303003949" and the hash "1fb224ec0020".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=074919869975&tochange=1fb224ec0020

Comment 2

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

Jan, is bug 1136837 (or the other one) a likely regressor?

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Patch

JSBugMon is right, this is a regression from bug 1136837.

The problem is that MFilterTypeSet's TypePolicy would add an Unbox(object), and we'd hoist this unbox above the actual getprop operation. This is wrong because we can only guarantee the value is an object if we executed the getprop operation first.

This patch uses setDependency because that's what FilterTypeSet uses elsewhere. Hannes, it might be nice to make Unboxes added in FilterTypeSetPolicy::adjustInputs non-movable, and remove the setDependency calls. What do you think?

Comment 5

[Hannes Verschore [:h4writer]]

(In reply to Jan de Mooij [:jandem] from comment #3)
> This patch uses setDependency because that's what FilterTypeSet uses
> elsewhere. Hannes, it might be nice to make Unboxes added in
> FilterTypeSetPolicy::adjustInputs non-movable, and remove the setDependency
> calls. What do you think?

I would prefer not to do that, since that could possibly disable e.g. loop hoisting:

> for () {
>     if (x) { // independent from loop
>        // do things with X that is independent from loop
> 
>     }
> }

If MFilterTypeSet is non-movable, all the instructions depending on it cannot get moved either ...

Though I could adjust the constructor of MFilterTypeSet to take a "dependency" argument. Making sure that there is always a dependency set when using MFilterTypeSet ??

(Note: I've said this before, but I think we should improve and allow more uses of the "dependency" property next to AA.)

Comment 6

[Jan de Mooij [:jandem]]

(In reply to Hannes Verschore [:h4writer] from comment #5)
> If MFilterTypeSet is non-movable, all the instructions depending on it
> cannot get moved either ...

MFilterTypeSet *is* non-movable.

> Though I could adjust the constructor of MFilterTypeSet to take a
> "dependency" argument. Making sure that there is always a dependency set
> when using MFilterTypeSet ??

That may help yes..

Comment 7

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9fdbd96edb91

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9fdbd96edb91

Comment 9

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.