Closed
Bug 1464472
Opened 7 years ago
Closed 6 years ago
Crash [@ ShouldMark<JSString*>] or Assertion failure: addr % CellAlignBytes == 0, at gc/Cell.h:242
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox-esr60 | --- | unaffected |
firefox60 | --- | unaffected |
firefox61 | --- | unaffected |
firefox62 | --- | verified |
People
(Reporter: decoder, Assigned: Waldo)
References
Details
(6 keywords, Whiteboard: [jsbugmon:update])
Crash Data
Attachments
(2 files)
(deleted),
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
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.
Updated•7 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
Comment 1•7 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.
Assignee | ||
Comment 2•7 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.
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•7 years ago
|
||
Attachment #8980905 -
Flags: review?(jdemooij)
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/ ?
Attachment #8980904 -
Flags: review?(jdemooij) → review+
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/
Attachment #8980905 -
Flags: review?(jdemooij) → review+
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.
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
Flags: needinfo?(jwalden+bmo)
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
Flags: needinfo?(jwalden+bmo)
Comment 10•6 years ago
|
||
Group: javascript-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Updated•6 years ago
|
Status: RESOLVED → VERIFIED
Comment 11•6 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Updated•6 years ago
|
Blocks: 1462939
status-firefox60:
--- → unaffected
status-firefox61:
--- → unaffected
status-firefox-esr52:
--- → unaffected
status-firefox-esr60:
--- → unaffected
Updated•6 years ago
|
Group: core-security-release
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
Comment 13•6 years ago
|
||
bugherder |
You need to log in
before you can comment on or make changes to this bug.
Description
•