1350171 - Assertion failure: numFailures_ > 0 (numFailures_ should not overflow), at js/src/jit/ICState.h:113

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 201231223cd4 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu, run with --fuzzing-safe):

test();
function test() {
    var o = {};
    o.watch('x', function() {
        eval("var g = 42");
        test(g, 42);
    });
    try {
        o.x = 3;
    } catch (ex) {}
}


Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0823630e in js::jit::ICState::trackNotAttached (this=0xf538f120) at js/src/jit/ICState.h:113
#0  0x0823630e in js::jit::ICState::trackNotAttached (this=0xf538f120) at js/src/jit/ICState.h:113
#1  0x0822ed67 in js::jit::DoSetPropFallback (cx=0xf793a800, frame=0xfffb42e8, stub_=0xf538f110, stack=0xfffb42d8, lhs=..., rhs=...) at js/src/jit/BaselineIC.cpp:1597
#2  0x2a31fe24 in ?? ()
#3  0x2a322f6b in ?? ()
#4  0xf5391170 in ?? ()
#5  0x2a318c66 in ?? ()
#6  0x0820e65f in EnterBaseline (cx=0x2a3210cd, cx@entry=0xf793a800, data=...) at js/src/jit/BaselineJIT.cpp:162
#7  0x0822b2fd in js::jit::EnterBaselineMethod (cx=0xf793a800, state=...) at js/src/jit/BaselineJIT.cpp:200
#8  0x0816d47c in js::RunScript (cx=0xf793a800, state=...) at js/src/vm/Interpreter.cpp:384
#9  0x0816d78f in js::InternalCallOrConstruct (cx=0xf793a800, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:472
#10 0x0816da6c in InternalCall (cx=cx@entry=0xf793a800, args=...) at js/src/vm/Interpreter.cpp:499
#11 0x0816dbfb in js::Call (cx=0xf793a800, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:518
#12 0x081c1459 in js::WatchHandler (cx=0xf793a800, obj_=0xf560a208, id_=..., old=..., nvp=0xfffb4aa8, closure=0xf560a218) at js/src/builtin/Object.cpp:615
#13 0x0864aa60 in js::WatchpointMap::triggerWatchpoint (this=0xf5250ba0, cx=0xf793a800, obj=..., id=..., vp=...) at js/src/jswatchpoint.cpp:135
#14 0x08773535 in js::NativeSetProperty (cx=0xf793a800, obj=..., id=..., value=..., receiver=..., qualified=js::Qualified, result=...) at js/src/vm/NativeObject.cpp:2490
#15 0x0817094a in js::SetProperty (cx=0xf793a800, obj=..., id=..., v=..., receiver=..., result=...) at js/src/vm/NativeObject.h:1459
#16 0x0822dff7 in js::jit::DoSetPropFallback (cx=0xf793a800, frame=0xfffb4e38, stub_=0xf538f110, stack=0xfffb4e28, lhs=..., rhs=...) at js/src/jit/BaselineIC.cpp:1566
#17 0x2a31fe24 in ?? ()
[...]
#125 0x2a318c66 in ?? ()
#126 0x0820e65f in EnterBaseline (cx=0x2a3210cd, cx@entry=0xf793a800, data=...) at js/src/jit/BaselineJIT.cpp:162
#127 0x0822b2fd in js::jit::EnterBaselineMethod (cx=0xf793a800, state=...) at js/src/jit/BaselineJIT.cpp:200
eax	0x0	0
ebx	0x8d0eff4	147910644
ecx	0xf7da4864	-136689564
edx	0x0	0
esi	0xf793a800	-141318144
edi	0x8d0eff4	147910644
ebp	0xfffb4008	4294656008
esp	0xfffb4000	4294656000
eip	0x823630e <js::jit::ICState::trackNotAttached()+62>
=> 0x823630e <js::jit::ICState::trackNotAttached()+62>:	movl   $0x0,0x0
   0x8236318 <js::jit::ICState::trackNotAttached()+72>:	ud2    


Looks to me like we are recursing a lot here, but not sure what's going on. Marking s-s until triaged due to the assertion mentioning something overflowing and it's in IC.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

Just a silly bug: the SetProperty call can enter the function + the IC recursively, so before we try to attach the AddSlot stub we need to check whether we have to transition.

Comment 2

[Jan de Mooij [:jandem]]

Opening up. numFailures_ is used for heuristics and overflowing it shouldn't cause any correctness issues.

Comment 3

[Hannes Verschore [:h4writer]]

Comment on attachment 8850908 [details] [diff] [review]
Patch

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

Not sure this patch is correct.

- Upon enter "DoFallBack" we have 5 specific stubs.
- We don't need to to transition.
- We start to create a new "specific stub"
- We recursively add a new stub
- We now test and see that we need to transition
- We remove all stubs
- We add already created "specific stub"

When we transition instead of adding this stub, should we drop it and not attach it?

Comment 4

[Hannes Verschore [:h4writer]]

Comment on attachment 8850908 [details] [diff] [review]
Patch

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

Okay this is obviously correct.
(I thought this transition happened between creating the stub and adding the stub. Which is not the case. This is only happening BEFORE we created a stub. As a result this is ok.)

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5cbddcaa3e0c
Try to transition IC mode before attaching AddSlot stubs. r=h4writer

Comment 6

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

https://hg.mozilla.org/mozilla-central/rev/5cbddcaa3e0c

Comment 7

[Ryan VanderMeulen [:RyanVM]]

This was a regression from bug 1328140?

Comment 8

[Jan de Mooij [:jandem]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #7)
> This was a regression from bug 1328140?

Yes.