The following testcase crashes on mozilla-central revision d57a813c77a4 (run with --ion-eager): function g(f) {} function f(b) { g.apply(null, arguments); f(false); } f(true);

Crash trace: ==60679== Invalid read of size 4 ==60679== at 0x5E03D2: js::types::TypeSet::constraintsPurged() (jsinfer.h:505) ==60679== by 0x5E0433: js::types::TypeSet::toStackTypeSet() (jsinfer.h:724) ==60679== by 0x5E347C: js::types::TypeScript::ArgTypes(JSScript*, unsigned int) (jsinferinlines.h:820) ==60679== by 0xC3BEE5: js::ion::IonBuilder::makeInliningDecision(js::AutoObjectVector&) (IonBuilder.cpp:3213) ==60679== by 0xC418D4: js::ion::IonBuilder::jsop_funapplyarguments(unsigned int) (IonBuilder.cpp:4062) ==60679== by 0xC40A24: js::ion::IonBuilder::jsop_funapply(unsigned int) (IonBuilder.cpp:3962) ==60679== by 0xC2C346: js::ion::IonBuilder::inspectOpcode(JSOp) (IonBuilder.cpp:938) ==60679== by 0xC2B04F: js::ion::IonBuilder::traverseBytecode() (IonBuilder.cpp:689) ==60679== by 0xC2A71D: js::ion::IonBuilder::buildInline(js::ion::IonBuilder*, js::ion::MResumePoint*, js::ion::CallInfo&) (IonBuilder.cpp:488) ==60679== by 0xC38A39: js::ion::IonBuilder::inlineScriptedCall(JS::Handle<JSFunction*>, js::ion::CallInfo&) (IonBuilder.cpp:2962) ==60679== by 0xC3D88A: js::ion::IonBuilder::inlineScriptedCalls(js::AutoObjectVector&, js::AutoObjectVector&, js::ion::CallInfo&) (IonBuilder.cpp:3478) ==60679== by 0xC42334: js::ion::IonBuilder::jsop_call(unsigned int, bool) (IonBuilder.cpp:4118) ==60679== Address 0x48 is not stack'd, malloc'd or (recently) free'd

JSBugMon: Bisection requested, result: autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 122313:64a2c3fb2052 user: Hannes Verschore date: Tue Feb 19 11:33:42 2013 +0100 summary: Bug 836274: Disable funapply inlining when typeset of callee is tighter than caller, r=nbp This iteration took 121.091 seconds to run.

Bug is introduced by 837014. We don't have have TI information for the empty function, but still do the inlining. Funapply tests the types and fails on retrieving the type information. We can fix it by making sure an empty function always has TI information by running inference on it. Not sure if this is allowed, but testing it locally it just works. Fixes this assert + keeps score of deltablue. (Note: the changes to IonBuilder are a precaution. I noticed I didn't set targetScript correct. This is not exploitable, because when inlining funapply there is only 1 target at max. But still it is plain wrong. Therefore fixing this too!)

Comment on attachment 718325 [details] [diff] [review] Run inference before deciding to inline empty function Review of attachment 718325 [details] [diff] [review]: ----------------------------------------------------------------- Good catch. ::: js/src/ion/IonBuilder.cpp @@ +3163,4 @@ > if (!target->isInterpreted()) > return false; > > + RootedScript targetScript(cx, target->nonLazyScript()); Nit: as Sean mentioned on IRC, these can be JSFunction * and JSScript * since we suppress GC during compilation. @@ +3199,5 @@ > > JSOp op = JSOp(*pc); > for (size_t i = 0; i < targets.length(); i++) { > + RootedFunction target(cx, targets[i]->toFunction()); > + RootedScript targetScript(cx, target->nonLazyScript()); Same here.

Patch, like it was committed to inbound for asking aurora-approval

Comment on attachment 719028 [details] [diff] [review] Run inference before deciding to inline empty function [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 837014 User impact if declined: crashes when ionmonkey tries to inline empty function Testing completed (on m-c, etc.): 2 days on m-i, 1 day on m-c Risk to taking this patch (and alternatives if risky): Almost none. String or UUID changes made by this patch: /