Closed Bug 1098961 Opened 10 years ago Closed 10 years ago

Crash [@ js::TypeDescrIsSimpleType] or Assertion failure: args[0].isObject(), at builtin/TypedObject.cpp

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox33 --- unaffected
firefox34 --- unaffected
firefox35 --- unaffected
firefox36 --- affected
firefox-esr31 --- unaffected

People

(Reporter: gkw, Assigned: bhackett1024)

References

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Crash Data

Attachments

(3 files)

Array.prototype[Symbol.iterator] = function() {
    for (var i = 3; --i >= 0;) {
        yield this[i]
    }
}
new TypedObject.ArrayType(TypedObject.int32, 0).build(x => 1)

asserts js debug shell on m-c changeset 7f0d92595432 with --no-ion --no-threads at Assertion failure: args[0].isObject(), at builtin/TypedObject.cpp.

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-central/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 jit-tests together with jsfunfuzz, the specific files are:

http://hg.mozilla.org/mozilla-central/file/7f0d92595432/js/src/jit-test/tests/for-of/semantics-03.js
http://hg.mozilla.org/mozilla-central/file/7f0d92595432/js/src/jit-test/tests/ion/inlining/TypedObject-TypeDescrIsArrayType-unknown.js

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/ceca39a1a154
user:        Brian Hackett
date:        Fri Nov 07 08:37:21 2014 -0700
summary:     Bug 1092318 - Remove unsized array typed objects, r=nmatsakis.

Brian, is bug 1092318 a possible regressor?
Flags: needinfo?(bhackett1024)
Attached file stack
(lldb) bt 5
* thread #1: tid = 0x1b27b1, 0x00000001000c7c4c js-dbg-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::TypeDescrIsSimpleType((null)=<unavailable>, argc=<unavailable>, vp=<unavailable>) + 396 at TypedObject.cpp:2713, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x00000001000c7c4c js-dbg-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::TypeDescrIsSimpleType((null)=<unavailable>, argc=<unavailable>, vp=<unavailable>) + 396 at TypedObject.cpp:2713
    frame #1: 0x0000000100631492 js-dbg-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) [inlined] js::CallJSNative(native=0x00000001006df5b0)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 166 at jscntxtinlines.h:231
    frame #2: 0x00000001006313ec js-dbg-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::Invoke(cx=0x0000000101c01b90, args=CallArgs at 0x00007fff5fbfcad0, construct=<unavailable>) + 428 at Interpreter.cpp:482
    frame #3: 0x000000010064e491 js-dbg-opt-64-dm-nsprBuild-darwin-7f0d92595432`Interpret(cx=<unavailable>, state=<unavailable>) + 43393 at Interpreter.cpp:2527
    frame #4: 0x0000000100643ae9 js-dbg-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::RunScript(cx=0x0000000101c01b90, state=0x00007fff5fbfd918) + 345 at Interpreter.cpp:432
(lldb)
This also causes a null de-ref in js opt shells compiled using:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --disable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests
Crash Signature: [@ js::TypeDescrIsSimpleType]
Keywords: crash
Summary: Assertion failure: args[0].isObject(), at builtin/TypedObject.cpp → Crash [@ js::TypeDescrIsSimpleType] or Assertion failure: args[0].isObject(), at builtin/TypedObject.cpp
Attached file stack for opt crash
(lldb) bt 5
* thread #1: tid = 0x1b2b23, 0x0000000100095bbe js-dbgDisabled-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::TypeDescrIsSimpleType(js::ThreadSafeContext*, unsigned int, JS::Value*) [inlined] js::BarrieredBase<js::types::TypeObject*>::operator->(this=<unavailable>) const at Barrier.h:455, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x8)
  * frame #0: 0x0000000100095bbe js-dbgDisabled-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::TypeDescrIsSimpleType(js::ThreadSafeContext*, unsigned int, JS::Value*) [inlined] js::BarrieredBase<js::types::TypeObject*>::operator->(this=<unavailable>) const at Barrier.h:455
    frame #1: 0x0000000100095bbe js-dbgDisabled-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::TypeDescrIsSimpleType(js::ThreadSafeContext*, unsigned int, JS::Value*) [inlined] JSObject::getClass(this=0x0000000000000000) const at jsobj.h:138
    frame #2: 0x0000000100095bbe js-dbgDisabled-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::TypeDescrIsSimpleType(js::ThreadSafeContext*, unsigned int, JS::Value*) [inlined] js::ValueOperations<JS::MutableHandle<JS::Value> >::toObject(this=0x0000000000000000, i=<unavailable>) const + 4 at TypedObject.h:1054
    frame #3: 0x0000000100095bba js-dbgDisabled-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::TypeDescrIsSimpleType((null)=0x0000000101613c90, argc=1, vp=0x000000010187fa70) + 10 at TypedObject.cpp:2716
    frame #4: 0x0000000100456830 js-dbgDisabled-opt-64-dm-nsprBuild-darwin-7f0d92595432`js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) [inlined] js::CallJSNative(native=0x00000001004bafa0, args=0x000000010187fa80)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 65 at jscntxtinlines.h:231
(lldb)
This isn't a regression from bug 1092318.  The testcase below crashes in 527d5142b7c5, which is the parent revision to bug 1092318:

Array.prototype[Symbol.iterator] = function() {
    for (var i = 3; --i >= 0;) {
        yield this[i]
    }
}
new TypedObject.ArrayType(TypedObject.int32, 0).build(1, x => 1)

The problem here is that the typed object self hosting code uses destructuring internally, and when that destructuring code runs it accesses this iteration stuff whose behavior can be changed in arbitrary ways by the referring script.  I guess this could be fixed by either banning array-style destructuring in self hosted code, or modifying its semantics so it uses the builtin array iterator.
Flags: needinfo?(bhackett1024)
Attached patch patchSplinter Review
After talking with Till we decided that banning operations which can invoke these iteration hooks is the best course of action here.  This patch adds an assert to that effect and fixes up the places in TypedObject.js where this bites.  Other self hosted code doesn't use array style destructuring.
Assignee: nobody → bhackett1024
Attachment #8524062 - Flags: review?(nmatsakis)
Comment on attachment 8524062 [details] [diff] [review]
patch

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

Nice find by the fuzzers.  (And reason 1 of N that I am not a huge fan of destructuring sugar.)

::: js/src/frontend/BytecodeEmitter.cpp
@@ +3426,5 @@
>  EmitIteratorNext(ExclusiveContext *cx, BytecodeEmitter *bce, ParseNode *pn=nullptr)
>  {
> +    // Disallow use of .next() iteration in self hosted code, as this can
> +    // access user modifiable iteration hooks.
> +    MOZ_ASSERT(bce->emitterMode != BytecodeEmitter::SelfHosting);

Put the rationale in the assertion itself so that people who encounter this know immediately why this is prohibited without having to find the assertion in source code:

    MOZ_ASSERT(bce->emitterMode != BytecodeEmitter::SelfHosting,
               ".next() iteration is prohibited in self-hosted code because it "
               "could run user-modifiable iteration code");
Comment on attachment 8524062 [details] [diff] [review]
patch

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

I agree with Jeff's comment regarding having a message in the assertion.
Attachment #8524062 - Flags: review?(nmatsakis) → review+
https://hg.mozilla.org/mozilla-central/rev/52f500342c88
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.