1352510 - Crash [@ js::jit::AutoDetectInvalidation::shouldSetReturnOverride] or Crash [@ js::jit::ArrayPushDense] or Assertion failure: hasIonScript(), at jsscript.h:1489

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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

function maybeSetLength(arr, b) {
    if (b) arr.length = 0x7fffffff;
}
var arr = [];
for (var i = 0; i < 2000; i++) {
    maybeSetLength(arr, i > 1500);
    var res = arr.push((0.017453));
}



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0833c927 in js::jit::AutoDetectInvalidation::shouldSetReturnOverride (this=<synthetic pointer>, this=<synthetic pointer>) at js/src/jit/VMFunctions.h:599
#0  0x0833c927 in js::jit::AutoDetectInvalidation::shouldSetReturnOverride (this=<synthetic pointer>, this=<synthetic pointer>) at js/src/jit/VMFunctions.h:599
#1  js::jit::AutoDetectInvalidation::~AutoDetectInvalidation (this=<synthetic pointer>, __in_chrg=<optimized out>) at js/src/jit/VMFunctions.h:603
#2  js::jit::ArrayPushDense (cx=0xf791d000, obj=..., v=..., length=0xffffc5e0) at js/src/jit/VMFunctions.cpp:360
#3  0x5aa866a6 in ?? ()
#4  0x5aa827f5 in ?? ()
eax	0x1	1
ebx	0xffffc580	-14976
ecx	0xffffc798	-14440
edx	0xf791d030	-141438928
esi	0x0	0
edi	0xf791d000	-141438976
ebp	0xffffc5b8	4294952376
esp	0xffffc540	4294952256
eip	0x833c927 <js::jit::ArrayPushDense(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, unsigned int*)+311>
=> 0x833c927 <js::jit::ArrayPushDense(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, unsigned int*)+311>:	mov    0x98(%esi),%edx
   0x833c92d <js::jit::ArrayPushDense(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, unsigned int*)+317>:	test   %edx,%edx


Marking s-s because this is a crash/assert in some lower level JIT code and involves invalidation. Please unhide after triage if this is really harmless.

Comment 1

[Jan de Mooij [:jandem]]

This is mine.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Patch

In ArrayPushDense we need to account for already-invalidated IonScripts. There are a few different ways to fix it, but this one doesn't affect performance of the fast path.

Comment 3

[Jan de Mooij [:jandem]]

Regression from bug 1346547. Not s-s, this is a nullptr crash accessing script->ion.

Comment 4

[Hannes Verschore [:h4writer]]

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

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

Good find !

Comment 5

[Marcia Knous [:marcia]]

Tracking 55+ for this crash.

Comment 6

[Julien Cristau [:jcristau]]

Jan this was reviewed 2 weeks ago; is anything blocking landing?

Comment 7

[Jan de Mooij [:jandem]]

(In reply to Julien Cristau [:jcristau] from comment #6)
> Jan this was reviewed 2 weeks ago; is anything blocking landing?

Sorry for the delay, I'll get this landed today or tomorrow. I need to stop posting more patches than I land..

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/96bd1936aa50
Handle already-invalidated IonScripts in ArrayPushDense. r=h4writer

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/96bd1936aa50