Closed Bug 1108007 Opened 5 years ago Closed 5 years ago

Assertion failure: (ptrBits & 0x7) == 0, at dist/include/js/Value.h

Categories

(Core :: JavaScript: GC, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla38
Tracking Status
firefox35 --- unaffected
firefox36 + fixed
firefox37 + fixed
firefox38 + verified
firefox-esr31 --- fixed
b2g-v1.4 --- wontfix
b2g-v2.0 --- wontfix
b2g-v2.0M --- wontfix
b2g-v2.1 --- wontfix
b2g-v2.1S --- wontfix
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: gkw, Assigned: jonco)

References

(Blocks 2 open bugs)

Details

(4 keywords, Whiteboard: [jsbugmon:update][b2g-adv-main2.2-])

Attachments

(2 files, 1 obsolete file)

// |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)
Attached file stack
(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)
(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.
Confirmed, it also reproduces when I replace the call to gczeal(14) with gczeal(2).
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)
Attached patch bug1108007-clone-fuzz-crash (obsolete) — Splinter Review
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)
Jon, do you have an idea of how far back this goes?
Flags: needinfo?(jcoppeard)
Attachment #8532654 - Flags: review?(nicolas.b.pierron) → review+
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 78fe054f6737).
This fuzz test case has unrelated test failures on tbpl, for which I filed bug 1108603.
Flags: needinfo?(jcoppeard)
Depends on: 1108603
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 } }
This produces the same results as before with the new global added.
Keywords: sec-high
How far back does this go?
(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_
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)
(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.
Flags: needinfo?(jcoppeard)
Keywords: sec-highsec-other
No longer depends on: 1108603
https://hg.mozilla.org/mozilla-central/rev/6e2a8cd91dfd
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:update]
Jon, could you fill the uplift requests to aurora & beta? Thanks
Flags: needinfo?(jcoppeard)
DeepCloneObjectLiteral was added in FF 29 by bug 920322.
Blocks: 920322
Flags: needinfo?(jcoppeard)
Updating patch with fixed test code, carrying r+.
Attachment #8532654 - Attachment is obsolete: true
Attachment #8554434 - Flags: review+
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?
Attachment #8554434 - Flags: approval-mozilla-beta?
Attachment #8554434 - Flags: approval-mozilla-beta+
Attachment #8554434 - Flags: approval-mozilla-aurora?
Attachment #8554434 - Flags: approval-mozilla-aurora+
(In reply to Jon Coppeard (:jonco) from comment #19)
> DeepCloneObjectLiteral was added in FF 29 by bug 920322.
So, ESR is impacted, right?
(In reply to Sylvestre Ledru [:sylvestre] from comment #22)
> So, ESR is impacted, right?

Yep.
Attachment #8554434 - Flags: approval-mozilla-esr31?
Attachment #8554434 - Flags: approval-mozilla-esr31? → approval-mozilla-esr31+
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
Group: core-security
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.