1132770 - Crash [@ js::ShapeTable::search] with stack exhaustion

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 3094601af679 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-debug --enable-gccompacting, run with --ion-eager):

this.__defineSetter__("x", function(){
  while(x != 0) { x &= x-1; ++r; }
});
x = 0;



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
js::ShapeTable::search (this=0x17fd350, id=id@entry=..., adding=adding@entry=false) at js/src/vm/Shape.cpp:190
190	{
#0  js::ShapeTable::search (this=0x17fd350, id=id@entry=..., adding=adding@entry=false) at js/src/vm/Shape.cpp:190
#1  0x0000000000573ee0 in js::Shape::search (cx=cx@entry=0x1713a00, start=0x7ffff516a5f0, id=..., pentry=pentry@entry=0x7fffff7ff090, adding=adding@entry=false) at js/src/vm/Shape-inl.h:69
#2  0x000000000057584c in lookup (id=..., cx=0x1713a00, this=<optimized out>) at js/src/vm/NativeObject.cpp:263
#3  LookupOwnPropertyInline<(js::AllowGC)1> (donep=<synthetic pointer>, propp=..., id=..., obj=..., cx=0x1713a00) at js/src/vm/NativeObject-inl.h:467
#4  LookupPropertyInline<(js::AllowGC)1> (propp=..., objp=..., id=..., obj=..., cx=0x1713a00, cx@entry=0x7fffff7ff090) at js/src/vm/NativeObject-inl.h:554
#5  js::NativeLookupProperty<(js::AllowGC)1> (cx=cx@entry=0x1713a00, obj=..., obj@entry=..., id=id@entry=..., objp=..., objp@entry=..., propp=propp@entry=...) at js/src/vm/NativeObject.cpp:1527
#6  0x000000000086d598 in LookupProperty (propp=..., objp=..., id=..., obj=..., cx=0x1713a00) at js/src/jsobj.cpp:2872
#7  js::LookupName (cx=cx@entry=0x1713a00, name=..., name@entry=..., scopeChain=..., scopeChain@entry=..., objp=..., objp@entry=..., pobjp=pobjp@entry=..., propp=..., propp@entry=...) at js/src/jsobj.cpp:2882
#8  0x0000000000758445 in js::jit::NameIC::update (cx=0x1713a00, cacheIndex=<optimized out>, scopeChain=..., vp=...) at js/src/jit/IonCaches.cpp:4245
#9  0x00007ffff7fea042 in ?? ()
#10 0x00007fffffffad90 in ?? ()
#11 0x00007fffff7ff368 in ?? ()
#12 0x00007ffff7fe8620 in ?? ()
#13 0xfff9000000000000 in ?? ()
#14 0x00000000016e2060 in js::jit::GetPropertyIC::UpdateInfo ()
#15 0x00007ffff514e7f0 in ?? ()
#16 0x00007ffff7e04638 in ?? ()
#17 0x0000000000000300 in ?? ()
#18 0x0000000000000000 in ?? ()
rax	0x51546628	1364485672
rbx	0x7ffff516a5f0	140737305290224
rcx	0x7fff	32767
rdx	0x0	0
rsi	0x7ffff5100b68	140737304857448
rdi	0x17fd350	25154384
rbp	0x7ffff5100b68	140737304857448
rsp	0x7fffff7ff000	140737479962624
r8	0x0	0
r9	0x7fffff7ff2b0	140737479963312
r10	0x7fffff7ff270	140737479963248
r11	0x7fffff7ff2b0	140737479963312
r12	0x7fffff7ff2b0	140737479963312
r13	0x1691240	23663168
r14	0x7fffff7ff080	140737479962752
r15	0x1713a00	24197632
rip	0x5989ae <js::ShapeTable::search(jsid, bool)+14>
=> 0x5989ae <js::ShapeTable::search(jsid, bool)+14>:	push   %rbx
   0x5989af <js::ShapeTable::search(jsid, bool)+15>:	mov    (%rdi),%r10d


This looks like a stack exhaustion, so not s-s. I am also seeing crashes on the heap, also stack exhaustions, so I assume those are related.

Comment 1

[Christian Holler (:decoder)]

Marking as fuzzblocker because stack exhaustions can product lots of signatures.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Patch

I can't reproduce this but I think it's a regression from bug 1129382 and this patch should fix it.

The problem is that the getprop/setprop/getname ICs can now attach stubs that call the script recursively, so we can't omit the overrecursion check in that case.

For normal (scripted) calls, things work because MCall::possiblyCalls returns true, but I don't want to do that here because that also affects LICM. So this patch just sets the performsCalls flag during lowering.

decoder, can you confirm this patch fixes the crash? :)

Comment 3

[Jan de Mooij [:jandem]]

[Tracking Requested - why for this release]:

Comment 4

[Jan de Mooij [:jandem]]

I can reproduce the crash with this one and the patch indeed fixes it:

  Object.defineProperty(this, "x", {set: function() { this.x = 2; }});
  setJitCompilerOption("ion.warmup.trigger", 30);
  x ^= 1;

It's a bit nicer/simpler than the test in the patch, will use this one.

Comment 5

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7451604bd50d

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7451604bd50d