The following testcase asserts on mozilla-central revision e4757379b99a (run with --ion-eager): function pow1(x) { return Math.pow(x, 0.5); } assertEq(pow1(NaN), NaN); function iso(d) { return new Date( NaN, "typeof Number(new Number(Number.NaN))", typeof pow1(new pow1(pow1.NaN))) } function check(s, millis) { description = "Date.parse('"+s+"') == '"+iso(millis)+"'"; } check("2009-07-23T00:00:00.000-07:00", undefined)

Valgrind shows: ==47874== Invalid read of size 4 ==47874== at 0x68CBBA: js::ion::LIRGenerator::visitToDouble(js::ion::MToDouble*) (Lowering.cpp:955) ==47874== by 0x6870BF: js::ion::LIRGenerator::visitInstruction(js::ion::MInstruction*) (Lowering.cpp:1887) ==47874== by 0x6872E9: js::ion::LIRGenerator::visitBlock(js::ion::MBasicBlock*) (Lowering.cpp:1979) ==47874== by 0x6875BE: js::ion::LIRGenerator::generate() (Lowering.cpp:2049) ==47874== by 0x64AE21: js::ion::CompileBackEnd(js::ion::IonBuilder*) (Ion.cpp:854) ==47874== by 0x64B1FF: js::ion::TestCompiler(js::ion::IonBuilder*, js::ion::MIRGraph*, js::ion::AutoDestroyAllocator&) (Ion.cpp:913) ==47874== by 0x64C70D: bool js::ion::IonCompile<&(js::ion::TestCompiler(js::ion::IonBuilder*, js::ion::MIRGraph*, js::ion::AutoDestroyAllocator&))>(JSContext*, JSScript*, JSFunction*, unsigned char*, bool) (Ion.cpp:1022) ==47874== by 0x64C9FC: js::ion::MethodStatus js::ion::Compile<&(js::ion::TestCompiler(js::ion::IonBuilder*, js::ion::MIRGraph*, js::ion::AutoDestroyAllocator&))>(JSContext*, JSScript*, JSFunction*, unsigned char*, bool) (Ion.cpp:1154) ==47874== by 0x64CB74: js::ion::CanEnter(JSContext*, JSScript*, js::StackFrame*, bool) (Ion.cpp:1252) ==47874== by 0x4965EF: js::RunScript(JSContext*, JSScript*, js::StackFrame*) (jsinterp.cpp:297) ==47874== by 0x496819: js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) (jsinterp.cpp:378) ==47874== by 0x496E4C: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::Value*) (jsinterp.h:109) ==47874== Address 0x1b29f47ac is not stack'd, malloc'd or (recently) free'd

Not s-s; in opt builds it will just abort compilation. The problem here is that TI is telling us that an argument to Math.pow() is a double, because it previously saw a double pass through when called from a different context, even though the current argument is an MCreateThis. We follow TI's advice and wind up in a sticky situation. One solution is to attempt to detect whether the MIRType of each argument is actually convertible to a double, and not inline in the case of a mismatch. Another solution is to just remove the JS_NOT_REACHED() and let the compilation fail. The former sounds better, but it is invasive and must be done for a number of these functions.

The crash is from JS_NOT_REACHED affecting opt builds. So far, we have been using JS_NOT_REACHED() as if it were JS_ASSERT() with additional documentation, but JS_NOT_REACHED winds up using __builtin_unreachable() to actually communicate unreachability to the compiler. We occasionally presume graceful fallback to a |return false| in the JS_NOT_REACHED cases -- these should be changed to JS_ASSERT() where relevant for less exploitability.

unhiding based on comment 2.

S-s based on comment 1 and 3, comment 2 is wrong :)

This is half of a fix to this bug -- it fixes the segfault in opt builds without fixing the tripping assertion in debug builds. I would prefer to land this patch first and quickly, since it can manifest as extremely-difficult-to-debug opt crashes in a real browser.

Comment on attachment 667229 [details] [diff] [review] Patch: Prevent segfaults in opt builds. Review of attachment 667229 [details] [diff] [review]: ----------------------------------------------------------------- For some definition of easier to debug - I think the false propagation will be safer but since no error is reported, the callstack will just unwind and disappear.

(In reply to David Anderson [:dvander] from comment #7) > For some definition of easier to debug - I think the false propagation will > be safer but since no error is reported, the callstack will just unwind and > disappear. Instead of segfaulting in opt builds in mysterious places, it will silently fail and propagate error. Makes such bugs perf problems instead of sec-critical issues.

It will also be a correctness issue, since entire JS callstack will unwind with a failure return. That is we wouldn't just fail Ion compilation, we'd fail to run the method. But seeing as how we need to fix the underlying bug anyway, and we've still got asserts, it's not a big deal.

Comment 9 suggests this might even be critical, but dvander can probably tell more.

Fix bug by forcing bailout.

JSBugMon: This bug has been automatically verified fixed.