1330662 - Assertion failure: end[0] == ZeroLengthArrayData, at js/src/vm/TypedArrayObject.cpp:148

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 97d6f7364394 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off):

for (i=0;i<10000;++i) {
    a = inIon() ? 0 : 300;
    try {
        buf = new Uint8ClampedArray(a);
        assertEq(buf.length, 300);
    } catch ({}) {}
}



Backtrace:

 received signal SIGSEGV, Segmentation fault.
js::TypedArrayObject::assertZeroLengthArrayData (this=0x7ffff0128f40) at js/src/vm/TypedArrayObject.cpp:148
#0  js::TypedArrayObject::assertZeroLengthArrayData (this=0x7ffff0128f40) at js/src/vm/TypedArrayObject.cpp:148
#1  0x0000000000c246aa in js::TypedArrayObject::finalize (fop=<optimized out>, obj=<optimized out>) at js/src/vm/TypedArrayObject.cpp:166
#2  0x00000000009ad0f3 in js::Class::doFinalize (this=<optimized out>, obj=0x7ffff0128f40, fop=0x7fffffffd0b0) at dist/include/js/Class.h:770
#3  JSObject::finalize (this=this@entry=0x7ffff0128f40, fop=fop@entry=0x7fffffffd0b0) at js/src/jsobjinlines.h:87
#4  0x00000000009ad4b2 in js::gc::Arena::finalize<JSObject> (this=this@entry=0x7ffff0128000, fop=fop@entry=0x7fffffffd0b0, thingKind=thingKind@entry=js::gc::AllocKind::OBJECT4_BACKGROUND, thingSize=thingSize@entry=64) at js/src/jsgc.cpp:458
#5  0x0000000000973eed in FinalizeTypedArenas<JSObject> (fop=0x7fffffffd0b0, src=0x7fffffffbfa8, dest=..., thingKind=js::gc::AllocKind::OBJECT4_BACKGROUND, budget=..., keepArenas=js::gc::ArenaLists::KEEP_ARENAS) at js/src/jsgc.cpp:516
#6  0x0000000000975c61 in js::gc::ArenaLists::backgroundFinalize (fop=fop@entry=0x7fffffffd0b0, listHead=0x7ffff0129000, empty=empty@entry=0x7fffffffd068) at js/src/jsgc.cpp:2769
#7  0x000000000097605f in js::gc::GCRuntime::sweepBackgroundThings (this=this@entry=0x7ffff695fad0, zones=..., freeBlocks=...) at js/src/jsgc.cpp:3167
#8  0x0000000000976cef in js::gc::GCRuntime::sweepBackgroundThings (freeBlocks=..., zones=..., this=0x7ffff695fad0) at js/src/jsgc.cpp:3155
#9  js::gc::GCRuntime::endSweepingZoneGroup (this=this@entry=0x7ffff695fad0) at js/src/jsgc.cpp:5175
#10 0x0000000000977445 in js::gc::GCRuntime::sweepPhase (this=this@entry=0x7ffff695fad0, sliceBudget=..., lock=...) at js/src/jsgc.cpp:5388
#11 0x000000000098bd75 in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0x7ffff695fad0, budget=..., reason=reason@entry=JS::gcreason::DESTROY_RUNTIME, lock=...) at js/src/jsgc.cpp:5900
#12 0x000000000098d358 in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffff695fad0, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DESTROY_RUNTIME) at js/src/jsgc.cpp:6176
#13 0x000000000098db99 in js::gc::GCRuntime::collect (this=this@entry=0x7ffff695fad0, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DESTROY_RUNTIME) at js/src/jsgc.cpp:6320
#14 0x000000000098de1b in js::gc::GCRuntime::gc (this=this@entry=0x7ffff695fad0, gckind=gckind@entry=GC_NORMAL, reason=reason@entry=JS::gcreason::DESTROY_RUNTIME) at js/src/jsgc.cpp:6381
#15 0x0000000000b7b3bf in JSRuntime::destroyRuntime (this=this@entry=0x7ffff695f208) at js/src/vm/Runtime.cpp:414
#16 0x0000000000915773 in JSContext::~JSContext (this=0x7ffff695f000, __in_chrg=<optimized out>) at js/src/jscntxt.cpp:937
#17 0x00000000009165aa in js_delete_poison<JSContext> (p=0x7ffff695f000) at dist/include/js/Utility.h:393
#18 js::DestroyContext (cx=0x7ffff695f000) at js/src/jscntxt.cpp:137
#19 0x000000000088df4a in JS_DestroyContext (cx=<optimized out>) at js/src/jsapi.cpp:486
#20 0x0000000000438482 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:7966
rax	0x2061520	33953056
rbx	0x7ffff0128f40	140737221136192
rcx	0x126f808	19331080
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffbd80	140737488338304
rsp	0x7fffffffbd70	140737488338288
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x7ffff0128f40	140737221136192
r13	0x7fffffffd0b0	140737488343216
r14	0x203f660	33814112
r15	0x7fffffffbe60	140737488338528
rip	0xc1b567 <js::TypedArrayObject::assertZeroLengthArrayData() const+103>
=> 0xc1b567 <js::TypedArrayObject::assertZeroLengthArrayData() const+103>:	movl   $0x0,0x0
   0xc1b572 <js::TypedArrayObject::assertZeroLengthArrayData() const+114>:	ud2    


This bug as everything an s-s bug needs, GC involved, TypedArrays and an assertion that looks like you don't want to meet it at night in a dark corner.

Comment 1

[Hannes Verschore [:h4writer]]

Taking. Assert was added in bug 1312480

Comment 2

[Jeff Walden [:Waldo]]

Please for the love of all that is holy don't land comment 0's test verbatim: catching assertEq exceptions is really not cool form!

Comment 3

[Hannes Verschore [:h4writer]]

Opening bug. This is a too narrow assert.
In the IonMonkey fastpath we create a TypedArrayObject which isn't big enough,
which we discard immediately and never use. But the finalizer gets called and triggers this assert.

Comment 4

[Hannes Verschore [:h4writer]]

Attachment: Patch

Comment 5

[Jan de Mooij [:jandem]]

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

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

::: js/src/vm/TypedArrayObject.cpp
@@ +163,5 @@
>      MOZ_ASSERT(!IsInsideNursery(obj));
>      TypedArrayObject* curObj = &obj->as<TypedArrayObject>();
>  
> +    // Template objects or discarded objects (which didn't have enough room
> +    // for inner elements). Don't have anything to free.

s/. Don't/ don't/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/22426fbd559b
IonMonkey: Don't check the size of a zero TypedArrayObject when not used, r=jandem

Comment 7

[Hannes Verschore [:h4writer]]

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

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1312480

[User impact if declined]:
Nothing. This asserts a bit too eagerly on debug builds. This would mostly be for fuzzers not hitting this, in their search for other bugs

[Is this code covered by automated tests?]:
Yes

[Has the fix been verified in Nightly?]:
I verified that the testcase is fine and jit-tests run fine too. It just landed on inbound.

[Needs manual test from QE? If yes, steps to reproduce]: 
/

[List of other uplifts needed for the feature/fix]:
/

[Is the change risky?]:
[Why is the change risky/not risky?]:
No. It makes us not call the asserting debug code in DEBUG builds.


[String changes made/needed]:
/

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/22426fbd559b

Comment 9

[Gerry Chang [:gchang]]

Track 51- as no significant user impact and we've started to build 51 RC. Let this ride the train on 52.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a66dd0e983f2