Assertion failure: MIR instruction returned object with unexpected type, at jit/MacroAssembler.cpp involving objectEmulatingUndefined

VERIFIED FIXED in Firefox 36

Status

()

Core
JavaScript Engine: JIT
--
critical
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: gkw, Assigned: h4writer)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

Trunk
mozilla36
x86_64
Mac OS X
assertion, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox35 unaffected, firefox36 verified, firefox-esr31 unaffected)

Details

(Whiteboard: [jsbugmon:update])

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
function f(x) {
    Math.exp(x ? 0 : 1)
}
f(objectEmulatingUndefined())
f(objectEmulatingUndefined())

asserts js debug shell on m-c changeset c63e741bca2e with --ion-eager --no-threads at Assertion failure: MIR instruction returned object with unexpected type, at jit/MacroAssembler.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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/ef4e7eebec3b
user:        Sushant Dinesh
date:        Tue Nov 25 14:43:31 2014 -0800
summary:     Bug 1082448 - IonMonkey: Add filtering to default case. r=h4writer (in spite of the CLOSED TREE)

Hannes, is bug 1082448 a likely regressor?
Flags: needinfo?(hv1989)
(Reporter)

Updated

3 years ago
status-firefox36: --- → affected
(Reporter)

Comment 1

3 years ago
Locking, this is likely s-s since it involves MIR. Hannes has more details.
Group: core-security
(Reporter)

Comment 2

3 years ago
Created attachment 8529704 [details]
stack

(lldb) bt 5
* thread #1: tid = 0x527ef6, 0x0000000101af2862, queue = 'com.apple.main-thread', stop reason = EXC_BREAKPOINT (code=EXC_I386_BPT, subcode=0x0)
  * frame #0: 0x0000000101af2862
    frame #1: 0x00000001002c2596 js-dbg-opt-64-dm-nsprBuild-darwin-c63e741bca2e`js::jit::IonCannon(JSContext*, js::RunState&) [inlined] EnterIon(data=0x0000000101af2428) + 343 at Ion.cpp:2442
    frame #2: 0x00000001002c243f js-dbg-opt-64-dm-nsprBuild-darwin-c63e741bca2e`js::jit::IonCannon(cx=0x00007fff5fbfe930, state=0x00007fff5fbfe7f8) + 191 at Ion.cpp:2523
    frame #3: 0x0000000100652927 js-dbg-opt-64-dm-nsprBuild-darwin-c63e741bca2e`js::RunScript(cx=0x0000000101c02820, state=0x00007fff5fbfe7f8) + 247 at Interpreter.cpp:412
    frame #4: 0x000000010063ff84 js-dbg-opt-64-dm-nsprBuild-darwin-c63e741bca2e`js::Invoke(cx=0x0000000101c02820, args=CallArgs at 0x00007fff5fbfe880, construct=<unavailable>) + 756 at Interpreter.cpp:501
(lldb)
(Reporter)

Comment 3

3 years ago
Variants cause:

Assertion failure: isTenured(), at gc/Heap.h
Assertion failure: Integer input should be lower or equal than Upperbound., at jit/MacroAssembler.cpp

and also likely:

Assertion failure: Argument check fail., at jit/MacroAssembler.cpp
Summary: Assertion failure: MIR instruction returned object with unexpected type, at jit/MacroAssembler.cpp → Assertion failure: MIR instruction returned object with unexpected type, at jit/MacroAssembler.cpp involving objectEmulatingUndefined
(Assignee)

Comment 4

3 years ago
Created attachment 8529713 [details] [diff] [review]
fixfilter

So there was a path for objectEmulatesUndefined, but it was doing the wrong thing. I should have reviewed this more carefully.
Assignee: nobody → hv1989
Flags: needinfo?(hv1989)
Attachment #8529713 - Flags: review?(jdemooij)
Comment on attachment 8529713 [details] [diff] [review]
fixfilter

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

Stealing review as per request on IRC. So there were 2 nops in the previous code: 1) in the true branch, typesets weren't filtered; 2) in the else branch, nothing was done to prevent objects that might emulated undefined, hence this fuzz bug. Please consider setting security flags and whether adding the test case would be nice or not.

::: js/src/jit/IonBuilder.cpp
@@ +3447,5 @@
>              // According to the standards, we cannot filter out:
>              // Strings, Int32, Double, Booleans, Objects (if they emulate undefined)
>              uint32_t flags = types::TYPE_FLAG_PRIMITIVE;
>  
> +            // If the typeset does emulates undefined. Then we cannot filter out objects.

Please make it a single sentence (and s/emulates/emulate):

If the typeset does emulate undefined, then we cannot filter out objects.

@@ +3449,5 @@
>              uint32_t flags = types::TYPE_FLAG_PRIMITIVE;
>  
> +            // If the typeset does emulates undefined. Then we cannot filter out objects.
> +            if (oldType->maybeEmulatesUndefined())
> +                flags = flags | types::TYPE_FLAG_ANYOBJECT;

style nit: flags |= types::TYPE_FLAG_ANYOBJECT;

@@ +3454,2 @@
>  
>              // Only intersect the typesets if it will generate a more narrow typeset.

I'd even expand this comment into explaining that the first part tests primitive types and the second part tests for objects, but that's probably because I'm a TI beginner, so I let it up to you.

@@ +3459,5 @@
>                  return true;
>              }
>  
>              types::TemporaryTypeSet base(flags, static_cast<types::TypeObjectKey**>(nullptr));
>              type = types::TypeSet::intersectSets(&base, ins->resultTypeSet(), alloc_->lifoAlloc());

while you're here, you can use oldType here as well.
Attachment #8529713 - Flags: review?(jdemooij) → review+
(Assignee)

Comment 6

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/18953298cf06

Normally there is still a merge tomorrow, so this should get in FF36.
But since it isn't 100% sure, I didn't land the tests yet and I NI myself to land the testcase in a couple of days.
Flags: needinfo?(hv1989)
https://hg.mozilla.org/mozilla-central/rev/18953298cf06
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox36: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
(Assignee)

Comment 8

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8bbdcb0f8a67

(adding the test)
Flags: needinfo?(hv1989)
https://hg.mozilla.org/mozilla-central/rev/8bbdcb0f8a67
Flags: in-testsuite+
status-firefox35: --- → unaffected
status-firefox-esr31: --- → unaffected
Status: RESOLVED → VERIFIED
status-firefox36: fixed → verified
status-firefox38: --- → verified
JSBugMon: This bug has been automatically verified fixed.
JSBugMon: This bug has been automatically verified fixed on Fx36
status-firefox38: verified → ---
Group: core-security
You need to log in before you can comment on or make changes to this bug.