1473256 - Assertion failure: !obj->lookup(cx, lengthId), at js/src/builtin/Array.cpp:1016

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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

var of = Reflect.construct(Array, [], Object);
var g = newGlobal({
    sameCompartmentAs: this
});
new g.Array(1, 2, 3);


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x00000000004c7ec8 in AddLengthProperty (cx=<optimized out>, obj=obj@entry=...) at js/src/builtin/Array.cpp:1016
#0  0x00000000004c7ec8 in AddLengthProperty (cx=<optimized out>, obj=obj@entry=...) at js/src/builtin/Array.cpp:1016
#1  0x00000000004d1120 in CreateArrayPrototype (cx=<optimized out>, key=<optimized out>) at js/src/builtin/Array.cpp:3683
#2  0x0000000000b91da2 in js::GlobalObject::resolveConstructor (cx=<optimized out>, cx@entry=0x7ffff5f17000, global=global@entry=..., key=key@entry=JSProto_Array) at js/src/vm/GlobalObject.cpp:202
#3  0x00000000005b59b9 in js::GlobalObject::ensureConstructor (cx=0x7ffff5f17000, global=..., key=JSProto_Array) at js/src/vm/GlobalObject.h:155
#4  0x0000000000a49562 in JS_ResolveStandardClass (cx=0x7ffff5f17000, obj=..., id=..., resolved=0x7fffffffc85f) at js/src/jsapi.cpp:1050
#5  0x0000000000bf6a4c in js::CallResolveOp (recursedp=<synthetic pointer>, propp=..., id=..., obj=..., cx=<optimized out>) at js/src/vm/NativeObject-inl.h:789
#6  js::LookupOwnPropertyInline<(js::AllowGC)1> (cx=<optimized out>, obj=..., id=..., propp=..., donep=0x7fffffffc97f) at js/src/vm/NativeObject-inl.h:861
#7  0x0000000000bf7043 in NativeGetPropertyInline<(js::AllowGC)1> (cx=<optimized out>, cx@entry=0x7ffff5f17000, obj=..., receiver=..., id=..., nameLookup=nameLookup@entry=NotNameLookup, vp=...) at js/src/vm/NativeObject.cpp:2402
#8  0x0000000000bf79d0 in js::NativeGetProperty (cx=cx@entry=0x7ffff5f17000, obj=..., receiver=..., receiver@entry=..., id=..., id@entry=..., vp=..., vp@entry=...) at js/src/vm/NativeObject.cpp:2450
#9  0x00000000005b8ef4 in js::GetProperty (cx=0x7ffff5f17000, obj=..., receiver=..., id=..., vp=...) at js/src/vm/NativeObject.h:1688
#10 0x000000000059d4be in js::GetProperty (vp=..., name=<optimized out>, receiver=..., obj=..., cx=<optimized out>) at js/src/vm/JSObject.h:787
#11 js::GetProperty (cx=<optimized out>, v=..., name=..., vp=...) at js/src/vm/Interpreter.cpp:4527
#12 0x00000000005a3d64 in GetPropertyOperation (vp=..., lval=..., pc=<optimized out>, script=..., fp=<optimized out>, cx=0x7ffff5f17000) at js/src/vm/Interpreter.cpp:217
#13 Interpret (cx=0x7ffff5f17000, state=...) at js/src/vm/Interpreter.cpp:2912
#14 0x00000000005afdb6 in js::RunScript (cx=0x7ffff5f17000, state=...) at js/src/vm/Interpreter.cpp:423
[...]
#23 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:9461
rax	0x0	0
rbx	0x7fffffffc5a0	140737488340384
rcx	0x7ffff6c282ad	140737333330605
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffc4f0	140737488340208
rsp	0x7fffffffc470	140737488340080
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4780	140737354024832
r10	0x58	88
r11	0x7ffff6b9e7a0	140737332766624
r12	0x7fffffffc4a0	140737488340128
r13	0x7fffffffc530	140737488340272
r14	0x7fffffffc490	140737488340112
r15	0x7fffffffc5a0	140737488340384
rip	0x4c7ec8 <AddLengthProperty(JSContext*, js::HandleArrayObject)+600>
=> 0x4c7ec8 <AddLengthProperty(JSContext*, js::HandleArrayObject)+600>:	movl   $0x0,0x0
   0x4c7ed3 <AddLengthProperty(JSContext*, js::HandleArrayObject)+611>:	ud2

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/6bbae91a2eaf
user:        Jan de Mooij
date:        Thu Jun 07 10:02:10 2018 +0200
summary:     Bug 1466501 part 1 - Refactor ZoneSpecifier and add a sameCompartmentAs option to newGlobal in the shell. r=luke

This iteration took 295.084 seconds to run.

Comment 2

[Jan de Mooij [:jandem]]

This is actually a pre-existing bug, the following asserts too:

var a1 = Reflect.construct(Array, [], Object);
var g = newGlobal({sameZoneAs: this});
var a2 = new g.Array(1, 2, 3);

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Patch

Comment 4

[André Bargull [:anba]]

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

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

Looks reasonable.

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking to make sure this lands. 
Jan, do you think this is OK to uplift to beta 62 (once it lands on m-c, that is)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/238948fa4dd6
Don't add a length property in AddLengthProperty if it's already present. r=anba

Comment 7

[Jan de Mooij [:jandem]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #5)
> Tracking to make sure this lands. 
> Jan, do you think this is OK to uplift to beta 62 (once it lands on m-c,
> that is)

Yes I think we should uplift this to 62. Keeping the NI to request approval in a few days.

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/238948fa4dd6

Comment 9

[Jan de Mooij [:jandem]]

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

Approval Request Comment
[Feature/Bug causing the regression]: Not sure but likely pretty old.
[User impact if declined]: Buggy behavior.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Low risk.
[Why is the change risky/not risky?]: It's just adding some code to handle a (very unlikely) case.
[String changes made/needed]: None.

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

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

Fix for possibly long standing minor bug, adds a test fix as well. 
Let's uplift for beta 10.

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/0e7242cbb1ce