Closed Bug 1105684 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla36
Tracking Status
firefox35 --- unaffected
firefox36 --- verified
firefox-esr31 --- unaffected

People

(Reporter: gkw, Assigned: h4writer)

References

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])

Attachments

(2 files)

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)
Locking, this is likely s-s since it involves MIR. Hannes has more details.
Group: core-security
Attached file 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)
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
Attached patch fixfilterSplinter Review
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+
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
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Flags: in-testsuite+
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
JSBugMon: This bug has been automatically verified fixed on Fx36
Group: core-security
You need to log in before you can comment on or make changes to this bug.