Christian Holler (:decoder) Reporter
	Description • 6 years ago

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

gczeal(17, 1);
for (var i = 0; i < 100; ++i) {
    Object.prototype[1012] = "value";
    imports = {}
    for (dmod in imports) {}
}


Backtrace:

received signal SIGSEGV, Segmentation fault.
ShouldMark<JSString*> (str=0xe5e5e5e5e5e5e5e5, gcmarker=0x7ffff5f1a098) at js/src/gc/Marking.cpp:808
#0  ShouldMark<JSString*> (str=0xe5e5e5e5e5e5e5e5, gcmarker=0x7ffff5f1a098) at js/src/gc/Marking.cpp:808
#1  DoMarking<JSString> (gcmarker=0x7ffff5f1a098, thing=0xe5e5e5e5e5e5e5e5) at js/src/gc/Marking.cpp:820
#2  0x0000000000974a12 in js::NativeIterator::<lambda(js::GCPtrFlatString&)>::operator() (prop=..., __closure=<synthetic pointer>) at js/src/vm/Iteration.cpp:74
#3  std::for_each<js::GCPtr<JSFlatString*>*, js::NativeIterator::trace(JSTracer*)::<lambda(js::GCPtrFlatString&)> > (__f=..., __last=<optimized out>, __first=0x7ffff49e8c50) at /usr/include/c++/6/bits/stl_algo.h:3769
#4  js::NativeIterator::trace (this=<optimized out>, trc=0x7ffff5f1a098) at js/src/vm/Iteration.cpp:75
#5  0x0000000000c4cf67 in js::Class::doTrace (this=<optimized out>, obj=0x7ffff4c7f160, trc=0x7ffff5f1a098) at /srv/jenkins/jobs/mozilla-central-build-jsshell/workspace/arch/64/compiler/gcc/instrumentation/none/type/opt/dist/include/js/Class.h:869
#6  CallTraceHook<TraverseObjectFunctor, js::GCMarker*, JSObject*&> (check=CheckGeneration::DoChecks, obj=0x7ffff4c7f160, trc=0x7ffff5f1a098, f=...) at js/src/gc/Marking.cpp:1619
#7  js::GCMarker::processMarkStackTop (this=this@entry=0x7ffff5f1a098, budget=...) at js/src/gc/Marking.cpp:1855
#8  0x0000000000c3d827 in js::GCMarker::drainMarkStack (this=this@entry=0x7ffff5f1a098, budget=...) at js/src/gc/Marking.cpp:1665
#9  0x0000000000bf0750 in js::gc::GCRuntime::drainMarkStack (this=0x7ffff5f194b0, sliceBudget=..., phase=<optimized out>) at js/src/gc/GC.cpp:5908
#10 0x0000000000c0eb0a in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0x7ffff5f194b0, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC, session=...) at js/src/gc/GC.cpp:7121
#11 0x0000000000c0fad7 in js::gc::GCRuntime::gcCycle (this=this@entry=0x7ffff5f194b0, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/gc/GC.cpp:7483
#12 0x0000000000c1005d in js::gc::GCRuntime::collect (this=this@entry=0x7ffff5f194b0, nonincrementalByAPI=nonincrementalByAPI@entry=false, budget=..., reason=reason@entry=JS::gcreason::DEBUG_GC) at js/src/gc/GC.cpp:7626
#13 0x0000000000c110d8 in js::gc::GCRuntime::runDebugGC (this=this@entry=0x7ffff5f194b0) at js/src/gc/GC.cpp:8188
#14 0x0000000000c11298 in js::gc::GCRuntime::gcIfNeededAtAllocation (this=0x7ffff5f194b0, cx=0x7ffff5f14000) at js/src/gc/Allocator.cpp:312
#15 0x0000000000c2c079 in js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> (this=<optimized out>, cx=0x7ffff5f14000, kind=<optimized out>) at js/src/gc/Allocator.cpp:273
#16 0x0000000000c2c6bd in js::AllocateString<JSString, (js::AllowGC)1> (cx=cx@entry=0x7ffff5f14000, heap=heap@entry=js::gc::DefaultHeap) at js/src/gc/Allocator.cpp:179
#17 0x00000000008b7ce2 in js::Allocate<JSThinInlineString, (js::AllowGC)1> (heap=js::gc::DefaultHeap, cx=0x7ffff5f14000) at js/src/gc/Allocator.h:47
#18 JSThinInlineString::new_<(js::AllowGC)1> (cx=0x7ffff5f14000) at js/src/vm/StringType-inl.h:279
#19 js::AllocateInlineString<(js::AllowGC)1, unsigned char> (chars=<synthetic pointer>, len=<optimized out>, cx=0x7ffff5f14000) at js/src/vm/StringType-inl.h:34
#20 js::NewInlineString<(js::AllowGC)1, unsigned char> (chars=..., cx=0x7ffff5f14000) at js/src/vm/StringType-inl.h:60
#21 js::Int32ToString<(js::AllowGC)1> (cx=cx@entry=0x7ffff5f14000, si=1012) at js/src/jsnum.cpp:627
#22 0x0000000000982762 in js::IdToString (id=..., cx=0x7ffff5f14000) at js/src/vm/JSAtom-inl.h:154
#23 js::NativeIterator::NativeIterator (this=0x7ffff49e8c10, cx=0x7ffff5f14000, propIter=..., objBeingIterated=..., props=..., numGuards=2, guardKey=0, hadError=0x7fffffffcd28) at js/src/vm/Iteration.cpp:679
#24 0x0000000000982fa8 in CreatePropertyIterator (guardKey=0, numGuards=2, props=..., objBeingIterated=..., cx=0x7ffff5f14000) at js/src/vm/Iteration.cpp:610
#25 VectorToKeyIterator (numGuards=2, props=..., obj=..., cx=0x7ffff5f14000) at js/src/vm/Iteration.cpp:747
#26 js::GetIterator (cx=cx@entry=0x7ffff5f14000, obj=obj@entry=...) at js/src/vm/Iteration.cpp:910
#27 0x00000000009847ee in js::ValueToIterator (cx=0x7ffff5f14000, vp=...) at js/src/vm/Iteration.cpp:1150
[...]
#38 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:9339
rax	0xe5e5e5e5e5e00000	-1880844493790380032
rbx	0x7ffff49e8c58	140737297419352
rcx	0xfe2c	65068
rdx	0xf23e05	15875589
rsi	0xe5e5e5e5e5e5e5e5	-1880844493789993499
rdi	0x7ffff5f1a098	140737319641240
rbp	0x7ffff49e8c70	140737297419376
rsp	0x7fffffffc7b8	140737488340920
r8	0x1	1
r9	0x100000000000	17592186044416
r10	0x104104104100	17871427092736
r11	0x0	0
r12	0x7ffff5f1a098	140737319641240
r13	0xf23e05	15875589
r14	0x7ffff4c7f160	140737300132192
r15	0x7fffffffcaf0	140737488341744
rip	0xc41909 <DoMarking<JSString>(js::GCMarker*, JSString*)+9>
=> 0xc41909 <DoMarking<JSString>(js::GCMarker*, JSString*)+9>:	mov    0xffff8(%rax),%rdx
   0xc41910 <DoMarking<JSString>(js::GCMarker*, JSString*)+16>:	cmp    %rdx,(%rdi)



Marking s-s and sec-high due to GC crash/assert and dangerous 0xe5e5 pattern in crash.

	Fuzzing Team
	Comment 1 • 6 years ago

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/6f7c84e815c0
user:        Jeff Walden
date:        Wed May 23 10:14:05 2018 -0700
summary:     Bug 1462939.  r=jandem

This iteration took 246.504 seconds to run.

	Jeff Walden [:Waldo] Assignee
	Comment 2 • 6 years ago

Simple bug when you look at it.  Tricky thing is figuring out a fix.  Should have one shortly nonetheless.

<3 fuzzers, and also I am a dumb still.

	Jeff Walden [:Waldo] Assignee
	Comment 3 • 6 years ago

Sigh.

	Jan de Mooij [:jandem]
	Comment 5 • 6 years ago

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

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

Ugh, good find. Properties must be initialized before guards because of the guardKey thing? It's pretty weird but pre-existing.

::: js/src/jit/BaselineCacheIRCompiler.cpp
@@ +1979,5 @@
>      masm.loadPtr(iterAddr, output);
>      masm.loadObjPrivate(output, JSObject::ITER_CLASS_NFIXED_SLOTS, niScratch);
>  
> +    // Ensure the iterator is reusable: see NativeIterator::isReusable.
> +    masm.branchTestNativeIteratorNotReusable(niScratch, failure->label());

Nit: maybe s/branchTest/branchIf/ ?

	Jan de Mooij [:jandem]
	Comment 6 • 6 years ago

Comment on attachment 8980905 [details] [diff] [review]
Add some comments and the test

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

Nice comments.

::: js/src/vm/Iteration.h
@@ +67,5 @@
> +        // of being constructed.  At such time |guardsEnd_| accounts only for
> +        // guards that have been initialized -- potentially none of them.
> +        // Instead, |propertyCursor_| is initialized to the ultimate/actual
> +        // start of properties and must be used instead of |propertiesBegin()|,
> +        // which asserts that thsi flag is present to guard against misuse.

s/thsi/this/

	Jeff Walden [:Waldo] Assignee
	Comment 7 • 6 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad6e1c06f4d76667e00ae0b520eff0225292396a

Holding off on test/comments for a week or so to give nightly users time to update, as usual.

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 8 • 6 years ago

Backed out (bug 1435828, bug 1464472) for spidermonkey failures and jsreftest bustage:

https://hg.mozilla.org/integration/mozilla-inbound/rev/2948af44622f481f704636556e43da92ff6bde99

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=ad6e1c06f4d76667e00ae0b520eff0225292396a&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log example: https://treeherder.mozilla.org/logviewer.html#?job_id=180627931&repo=mozilla-inbound

 Assertion failure: (chars[0] == '(' && chars[len - 1] == ')') || (chars[0] == '[' && chars[len - 1] == ']'), at /builds/worker/workspace/build/src/js/src/builtin/Eval.cpp:171

	Jeff Walden [:Waldo] Assignee
	Comment 9 • 6 years ago

Whoops, typo in the other patches landed then, nothing wrong with this patch.  Relanded:

https://hg.mozilla.org/integration/mozilla-inbound/rev/1268d562bda0

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 10 • 6 years ago

https://hg.mozilla.org/mozilla-central/rev/1268d562bda0

	Fuzzing Team
	Comment 11 • 6 years ago

JSBugMon: This bug has been automatically verified fixed.

	Pulsebot
	Comment 12 • 6 years ago

Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8add80f35d02
Add some comments and a test.  r=jandem

	Eliza Balazs [:ebalazs_]
	Comment 13 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/8add80f35d02