Closed Bug 694200 Opened 8 years ago Closed 8 years ago

Crash [@ js::mjit::ic::BaseIC::disable]

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
Tracking Status
firefox8 --- unaffected
firefox9 + fixed
firefox10 + fixed
status1.9.2 --- unaffected

People

(Reporter: decoder, Assigned: bhackett)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [sg:critical][qa-])

Crash Data

Attachments

(2 files)

The attached test crashes on mozilla-central revision 866b2b1793cd (see README for running instructions).

Valgrind trace:

==15832== Invalid read of size 8
==15832==    at 0x7846B5: js::mjit::ic::BaseIC::disable(JSContext*, char const*, void*) (PolyIC.cpp:2330)
==15832==    by 0x78CD95: PICStubCompiler::disable(JSContext*, char const*) (PolyIC.cpp:184)
==15832==    by 0x78CD5E: PICStubCompiler::disable(char const*) (PolyIC.cpp:180)
==15832==    by 0x783694: js::mjit::ic::GetProp(js::VMFrame&, js::mjit::ic::PICInfo*) (PolyIC.cpp:1992)
==15832==    by 0x6E7C75: ??? (MethodJIT.cpp:164)
==15832==    by 0x404FB4F: ???
==15832==  Address 0x40 is not stack'd, malloc'd or (recently) free'd


S-s because crashes with this signature have been previously critical.
Attached patch patchSplinter Review
On error paths, the IC code sometimes tried to disable the IC before returning an error.  This is a problem when the error was generated by a VM operation, as the error is checked before recompilation.  Patch does a sweep and removes all places where ICs are disabled on error paths.
Assignee: general → bhackett1024
Attachment #566829 - Flags: review?(dvander)
Attachment #566829 - Flags: review?(dvander) → review+
Attachment #566829 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/3783a31eda47
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: js-triage-needed → [sg:critical]
Brian, is bug 695875 revolving around something similar to this bug?
(In reply to Naoki Hirata :nhirata from comment #4)
> Brian, is bug 695875 revolving around something similar to this bug?

Possibly, yeah.  The crash in that bug predates the fix here, so it would be good to see if that crash can be reproduced on a current nightly.
Duplicate of this bug: 694872
Comment on attachment 566829 [details] [diff] [review]
patch

Approved per today's triage meeting.
Attachment #566829 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Duplicate of this bug: 695875
Do we need this for 8?
No, this is a TI bug.
Whiteboard: [sg:critical] → [sg:critical][qa+]
Brian, is there another way to verify that the issue has been solved as running the testcase through js? I have problems to compile Firefox on OS X at least with the given changeset from comment 0.
Whiteboard: [sg:critical][qa+] → [sg:critical][qa?]
Confirmed fixed on trunk, aurora and beta.

@Henrik: You don't need to compile Firefox to verify tests like this, a JS shell suffices (which is much easier). If you need help on how to compile these, let me know (just ping me on IRC or email) :)
[qa-] since QA didn't verify this fix. Marking it VERIFIED based on comment 13.

Thanks Christian
Status: RESOLVED → VERIFIED
Whiteboard: [sg:critical][qa?] → [sg:critical][qa-]
(In reply to Christian Holler (:decoder) from comment #13)
> @Henrik: You don't need to compile Firefox to verify tests like this, a JS
> shell suffices (which is much easier). If you need help on how to compile
> these, let me know (just ping me on IRC or email) :)

You came me before now. Yesterday I have already seen this by checking the tinderbox folders on FTP. All the builds up there have the jsshell included. But thanks for the verification.
Group: core-security
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.