Assertion failure: IsPackedArray(aobj), at js/src/vm/Interpreter.cpp:4563 with OOM

RESOLVED FIXED in Firefox 48

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: decoder, Assigned: jandem)

Tracking

(Blocks: 2 bugs, {assertion, regression, testcase})

Trunk
mozilla48
x86_64
Linux
assertion, regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 wontfix, firefox48 fixed)

Details

(Whiteboard: [jsbugmon:])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
The following testcase crashes on mozilla-central revision 9879757aa0d4 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --thread-count=2 --ion-extra-checks --ion-check-range-analysis --baseline-eager):

oomTest(function () 
 eval(`var wm = WeakMap( ... "ABCDEFGHIJK"      )                          `)
)



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000a4d66b in js::SpreadCallOperation (cx=cx@entry=0x7ffff6907800, script=..., script@entry=..., pc=pc@entry=0x7ffff6924094 ")o", thisv=..., thisv@entry=..., callee=callee@entry=..., arr=..., arr@entry=..., newTarget=newTarget@entry=..., res=res@entry=...) at js/src/vm/Interpreter.cpp:4563
#0  0x0000000000a4d66b in js::SpreadCallOperation (cx=cx@entry=0x7ffff6907800, script=..., script@entry=..., pc=pc@entry=0x7ffff6924094 ")o", thisv=..., thisv@entry=..., callee=callee@entry=..., arr=..., arr@entry=..., newTarget=newTarget@entry=..., res=res@entry=...) at js/src/vm/Interpreter.cpp:4563
#1  0x00000000006010e3 in js::jit::DoSpreadCallFallback (cx=0x7ffff6907800, frame=<optimized out>, stub_=<optimized out>, vp=0x7fffffffb2e0, res=...) at js/src/jit/BaselineIC.cpp:6248
#2  0x00007ffff7ff195b in ?? ()
[...]
#24 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff6907800	140737330051072
rcx	0x7ffff6ca53cd	140737333842893
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffffb0d0	140737488335056
rsp	0x7fffffffafa0	140737488334752
r8	0x7ffff7fe0780	140737354008448
r9	0x6372732f736a2f6c	7165916604736876396
r10	0x7fffffffad60	140737488334176
r11	0x7ffff6c27960	140737333328224
r12	0x7fffffffb190	140737488335248
r13	0x7ffff6924094	140737330167956
r14	0x7fffffffb1b0	140737488335280
r15	0x7fffffffafd0	140737488334800
rip	0xa4d66b <js::SpreadCallOperation(JSContext*, JS::Handle<JSScript*>, unsigned char*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>)+1499>
=> 0xa4d66b <js::SpreadCallOperation(JSContext*, JS::Handle<JSScript*>, unsigned char*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>)+1499>:	movl   $0x11d3,0x0
   0xa4d676 <js::SpreadCallOperation(JSContext*, JS::Handle<JSScript*>, unsigned char*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>)+1510>:	callq  0x4a2e90 <abort()>


Please note that this test is whitespace sensitive. The additional whitespace inside the template string is required for reproducing the issue.
Reduced test case:

  oomTest(() => eval(`Array(... "ABC")`))

This is failing on the following assert in js::SpreadCallOperation:

    MOZ_ASSERT(IsPackedArray(aobj));

The last OOM occurs with the following stack:

  * 0:  js_failedAllocBreakpoint() at Utility.h:116
    1:  js::oom::ShouldFailWithOOM at Utility.h:148
    2:  js::LifoAlloc::alloc at LifoAlloc.h:274
    3:  js::ObjectGroup::Property* js::LifoAlloc::new_<js::ObjectGroup::Property, jsid&> at LifoAlloc.h:425
    4:  js::ObjectGroup::getProperty at TypeInference-inl.h:1011
    5:  js::AddTypePropertyId at TypeInference.cpp:2728
    6:  js::AddTypePropertyId at TypeInference-inl.h:413
    7:  js::NativeObject::setDenseElementWithType at NativeObject-inl.h:74
    8:  AddOrChangeProperty(js::ExclusiveContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JSPropertyDescriptor>) at NativeObject.cpp:1157
    9:  js::NativeDefineProperty at NativeObject.cpp:1385
    10:  js::DefineProperty at jsobj.cpp:2661
    11:  js::DefineProperty at jsobj.cpp:2692
    12:  js::DefineElement at jsobj.cpp:2721
    13:  js::InitArrayElemOperation at Interpreter-inl.h:617
    14:  js::jit::DoSetElemFallback at BaselineIC.cpp:2766

So it looks like it's failing to set the type information that IsPackedArray() relies on.

I don't know how to fix this.

Updated

2 years ago
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:bisect]

Comment 2

2 years ago
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Updated

2 years ago
Whiteboard: [jsbugmon:bisect] → [jsbugmon:]
There seems to be JIT on the stack. Since Jon does not know how to fix this as per comment 1, setting needinfo? from Jan as a fallback.
Flags: needinfo?(jdemooij)
Naveed, need an asignee
Flags: needinfo?(nihsanullah)

Comment 5

2 years ago
Hi Decoder,

Is it possible to find what change set caused this regression and back it out?
Flags: needinfo?(choller)
(Assignee)

Comment 6

2 years ago
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #4)
> Naveed, need an asignee

(In reply to Doug Turner (:dougt) from comment #5)
> Is it possible to find what change set caused this regression and back it
> out?

Sorry for the delay here. This is just a bogus assert, I'll post a patch.
Flags: needinfo?(nihsanullah)
Flags: needinfo?(choller)
(Assignee)

Comment 7

2 years ago
Created attachment 8731154 [details] [diff] [review]
Patch

comment 1 is correct. IsPackedArray can return false after we hit OOM doing TI stuff. There are multiple ways to fix this. The patch just gets rid of the IsPackedArray call and checks there are no dense element holes.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8731154 - Flags: review?(arai.unmht)
Comment on attachment 8731154 [details] [diff] [review]
Patch

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

Thanks!
I didn't expect IsPackedArray to be temporal thing.
Attachment #8731154 - Flags: review?(arai.unmht) → review+
(Assignee)

Comment 10

2 years ago
Debug-only, no need to uplift this.
status-firefox46: affected → wontfix

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3151c74f2edb
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.