Closed
Bug 1108007
Opened 10 years ago
Closed 10 years ago
Assertion failure: (ptrBits & 0x7) == 0, at dist/include/js/Value.h
Categories
(Core :: JavaScript: GC, defect)
Tracking
()
VERIFIED
FIXED
mozilla38
People
(Reporter: gkw, Assigned: jonco)
References
Details
(4 keywords, Whiteboard: [jsbugmon:update][b2g-adv-main2.2-])
Attachments
(2 files, 1 obsolete file)
7.36 KB,
text/plain
|
Details | |
2.77 KB,
patch
|
jonco
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
bkerensa
:
approval-mozilla-esr31+
|
Details | Diff | Splinter Review |
// |jit-test| --no-threads; --no-ion
// From a CLI flag: --gc-zeal=14
gczeal(14);
// Randomly chosen test: js/src/jit-test/tests/basic/bug1057571.js
(function() {
evaluate(cacheEntry((function() {
return "".toSource()
})()), Object.create({}, {
saveBytecode: {
value: true
}
}))
})();
// Randomly chosen test: js/src/tests/ecma_6/Math/asinh-approx.js
[
[0],
[0],
[0],
[0],
[0],
[0],
[0],
[0],
[0],
[0],
[0],
[0],
[0],
[0],
[0],
[0],
[0],
]
asserts js debug shell on m-i changeset 69b7229fa7a0 with --no-threads --no-ion at Assertion failure: (ptrBits & 0x7) == 0, at dist/include/js/Value.h and crashes some builds (probably 32-bit ones?) with js::ArrayObject::createArrayInternal on the stack.
Debug configure options:
CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-inbound/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests
This was found by combining random js tests together with jsfunfuzz, the specific file(s) is/are:
http://hg.mozilla.org/mozilla-central/file/69b7229fa7a0/js/src/jit-test/tests/basic/bug1057571.js
http://hg.mozilla.org/mozilla-central/file/69b7229fa7a0/js/src/tests/ecma_6/Math/asinh-approx.js
autoBisect shows this is probably related to the following changeset:
The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/34859490061a
user: Hannes Verschore
date: Tue Nov 18 15:53:58 2014 +0100
summary: Bug 1052839 - Selfhost substr/slice/substring, r=waldo,till,jonco
Setting s-s because GC is on the stack.
Hannes and I looked at this and we don't think this is accurate. This seems to point to compacting GC instead, so setting needinfo? from Jon.
Flags: needinfo?(jcoppeard)
Reporter | ||
Comment 1•10 years ago
|
||
(lldb) bt 5
* thread #1: tid = 0x429a00, 0x000000010001d5a2 js-dbg-opt-64-dm-nsprBuild-darwin-69b7229fa7a0`JS::Value::toObject() const [inlined] JSVAL_IS_OBJECT_IMPL(jsval_layout) + 28 at Value.h:807, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
* frame #0: 0x000000010001d5a2 js-dbg-opt-64-dm-nsprBuild-darwin-69b7229fa7a0`JS::Value::toObject() const [inlined] JSVAL_IS_OBJECT_IMPL(jsval_layout) + 28 at Value.h:807
frame #1: 0x000000010001d586 js-dbg-opt-64-dm-nsprBuild-darwin-69b7229fa7a0`JS::Value::toObject() const [inlined] JS::Value::isObject(this=<unavailable>) const at Value.h:1148
frame #2: 0x000000010001d586 js-dbg-opt-64-dm-nsprBuild-darwin-69b7229fa7a0`JS::Value::toObject(this=<unavailable>) const + 182 at Value.h:1243
frame #3: 0x000000010018630a js-dbg-opt-64-dm-nsprBuild-darwin-69b7229fa7a0`js::GCMarker::processMarkStackTop(this=0x0000000102026b30, budget=0x00007fff5fbfe250) + 714 at Marking.cpp:1777
frame #4: 0x0000000100173b35 js-dbg-opt-64-dm-nsprBuild-darwin-69b7229fa7a0`js::GCMarker::drainMarkStack(this=0x0000000102026b30, budget=0x00007fff5fbfe250) + 69 at Marking.cpp:1916
(lldb)
Reporter | ||
Comment 2•10 years ago
|
||
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #0)
> Hannes and I looked at this and we don't think this is accurate. This seems
> to point to compacting GC instead, so setting needinfo? from Jon.
I misspoke - it seems to point to *GC, not compacting GC, since this is not a build compiled with --enable-gccompacting.
Reporter | ||
Comment 3•10 years ago
|
||
Confirmed, it also reproduces when I replace the call to gczeal(14) with gczeal(2).
Assignee | ||
Comment 4•10 years ago
|
||
Reproduced.
(lldb) p/x ptrBits
(uint64_t) $5 = 0x0000000000000042
This is the poison value from Debug_SetValueRangeToCrashOnTouch() called from NativeObject::growElements().
js::DeepCloneObjectLiteral() is setting the initialized length of a new array before populating the elements. If we GC before all elements have been initialized we observe the poison value and crash.
Flags: needinfo?(jcoppeard)
Assignee | ||
Comment 5•10 years ago
|
||
Patch to only set the array's initialized length when we have a value to initialize the new element.
Assignee: nobody → jcoppeard
Attachment #8532654 -
Flags: review?(nicolas.b.pierron)
Reporter | ||
Comment 6•10 years ago
|
||
Jon, do you have an idea of how far back this goes?
Flags: needinfo?(jcoppeard)
Updated•10 years ago
|
Attachment #8532654 -
Flags: review?(nicolas.b.pierron) → review+
Updated•10 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
Comment 7•10 years ago
|
||
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 78fe054f6737).
Assignee | ||
Comment 8•10 years ago
|
||
This fuzz test case has unrelated test failures on tbpl, for which I filed bug 1108603.
Flags: needinfo?(jcoppeard)
Comment 9•10 years ago
|
||
jonco, can you reproduce the test case if you add a new global to the argument of the evaluate function?
{ global : newGlobal(), saveBytecode: { value: true } }
Assignee | ||
Comment 10•10 years ago
|
||
This produces the same results as before with the new global added.
Comment 11•10 years ago
|
||
How far back does this go?
Updated•10 years ago
|
Blocks: 1052839
status-firefox35:
--- → unaffected
status-firefox36:
--- → affected
status-firefox-esr31:
--- → unaffected
tracking-firefox36:
--- → +
tracking-firefox37:
--- → +
Comment 12•10 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #11)
> How far back does this go?
As far as I know, this code is not exercise out-side of the JS Shell, as the call to the DeepCloneObject function is guarded by cloneSingleton:
- http://dxr.mozilla.org/mozilla-central/search?q=%2Bfunction-ref%3AJS%3A%3ACompartmentOptions%3A%3AsetCloneSingletons%28_Bool%29
- http://dxr.mozilla.org/mozilla-central/search?q=%2Bvar-ref%3AJS%3A%3ACompartmentOptions%3A%3AcloneSingletons_
Comment 13•10 years ago
|
||
Jon can this land? And I guess if this is really shell-only it can be sec-other instead of sec-high?
Flags: needinfo?(jcoppeard)
Updated•10 years ago
|
status-firefox38:
--- → affected
tracking-firefox38:
--- → +
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #13)
> Jon can this land? And I guess if this is really shell-only it can be
> sec-other instead of sec-high?
I've updated the test case not to trigger bug 1108603 so yes this can land now.
Yes it's shell only so marking sec-other.
Assignee | ||
Comment 15•10 years ago
|
||
Comment 16•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Updated•10 years ago
|
Status: RESOLVED → VERIFIED
Comment 17•10 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Reporter | ||
Updated•10 years ago
|
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update]
Comment 18•10 years ago
|
||
Jon, could you fill the uplift requests to aurora & beta? Thanks
Flags: needinfo?(jcoppeard)
Assignee | ||
Comment 19•10 years ago
|
||
DeepCloneObjectLiteral was added in FF 29 by bug 920322.
Blocks: 920322
Flags: needinfo?(jcoppeard)
Assignee | ||
Comment 20•10 years ago
|
||
Updating patch with fixed test code, carrying r+.
Attachment #8532654 -
Attachment is obsolete: true
Attachment #8554434 -
Flags: review+
Assignee | ||
Comment 21•10 years ago
|
||
Comment on attachment 8554434 [details] [diff] [review]
bug1108007-clone-fuzz-crash v2
Approval Request Comment
[Feature/regressing bug #]: Bug 920322
[User impact if declined]: None, shell only.
[Describe test coverage new/current, TreeHerder]: Change has been on m-c for 3 days.
[Risks and why]: Low risk, shell only change.
[String/UUID change made/needed]: None.
Attachment #8554434 -
Flags: approval-mozilla-beta?
Attachment #8554434 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8554434 -
Flags: approval-mozilla-beta?
Attachment #8554434 -
Flags: approval-mozilla-beta+
Attachment #8554434 -
Flags: approval-mozilla-aurora?
Attachment #8554434 -
Flags: approval-mozilla-aurora+
Comment 22•10 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #19)
> DeepCloneObjectLiteral was added in FF 29 by bug 920322.
So, ESR is impacted, right?
Updated•10 years ago
|
Assignee | ||
Comment 23•10 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #22)
> So, ESR is impacted, right?
Yep.
Updated•10 years ago
|
Attachment #8554434 -
Flags: approval-mozilla-esr31?
Updated•10 years ago
|
status-b2g-v1.4:
--- → wontfix
status-b2g-v2.0:
--- → wontfix
status-b2g-v2.0M:
--- → wontfix
status-b2g-v2.1:
--- → wontfix
status-b2g-v2.1S:
--- → wontfix
status-b2g-v2.2:
--- → affected
status-b2g-master:
--- → fixed
Comment 24•10 years ago
|
||
Comment 25•10 years ago
|
||
Updated•10 years ago
|
Attachment #8554434 -
Flags: approval-mozilla-esr31? → approval-mozilla-esr31+
Comment 26•10 years ago
|
||
Comment 27•10 years ago
|
||
esr31 doesn't support --no-threads. Rather than mess around with the flags on the test, I just removed the test.
https://hg.mozilla.org/releases/mozilla-esr31/rev/50daa62a5cb9
Updated•10 years ago
|
Group: core-security
Updated•10 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:update][b2g-adv-main2.2-]
You need to log in
before you can comment on or make changes to this bug.
Description
•