Closed Bug 676023 Opened 9 years ago Closed 9 years ago

crash js::StackIter::settleOnNewState

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla8
Tracking Status
firefox7 --- fixed

People

(Reporter: luke, Assigned: luke)

References

Details

(Whiteboard: [inbound][qa-])

Crash Data

Attachments

(2 files)

StackIter::settleOnNewState is showing up in crash-stats for FF7 and up.  The total number is low, but its #28 for FF7.0a2.  The crash happens at part of settleOnNewState that is sensitive to outside code breaking stack invariants.  I'd like to land some telemetry patches to get more details about how its crashing and release-build-assert some stronger invariants that will hopefully turn up a reproducible test-case.
Attached patch telemetry patchSplinter Review
Here's the first patch.  Pushing to try first; maybe we'll get lucky.
Comment on attachment 550130 [details] [diff] [review]
telemetry patch

No luck; try's clean.
Attachment #550130 - Flags: review?(wmccloskey)
Attachment #550130 - Flags: review?(wmccloskey) → review+
http://hg.mozilla.org/mozilla-central/rev/b0e4ea1084d1
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Comment on attachment 550130 [details] [diff] [review]
telemetry patch

>+        JS_ASSERT(false && "About to dereference invalid slot");

Is && really what you mean?
(In reply to comment #5)
> Is && really what you mean?

It is.
Crash Signature: [@ js::StackIter::settleOnNewState()] → [@ js::StackIter::settleOnNewState()] [@ CrashIfInvalidSlot ]
Duplicate of this bug: 676571
Reopening since the patch was telemetry.

So far, it looks like all of the crashes have moved (as of buildid 20110804030732) from StackIter::settleOnNewState to CrashIfInvalidSlot.  I'll try examining a dump on windows.
Attached patch fix and a testSplinter Review
Looking at the OS X crash stacks (the only ones with more than one frame), all the stacks include an error that calls a debugger throw hook than then later throws an exception.

Looking at the crash dumps, the bad 'vp' is pointing into the stack frame.  This could happen if sp_ was too low.  One way sp_ could be too low is if the intepreter popped sp but did not advance the pc.  This is only checked at JSOP_CALL, so I looked there and found one such instance that crashes in CrashIfInvalidSlot.  This patch fixes the bug and hopefully will fix this crash.
Attachment #550878 - Flags: review?(dvander)
Comment on attachment 550878 [details] [diff] [review]
fix and a test

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

nice test case!
Attachment #550878 - Flags: review?(dvander) → review+
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
http://hg.mozilla.org/mozilla-central/rev/6181622382cf
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Looks like that fixes it.  CrashInvalidSlot was hitting >50 times a day, getting to #12 on crash stats.  With the patch, it seems to have disappeared.
Comment on attachment 550878 [details] [diff] [review]
fix and a test

Simple patch, should decrease random crashiness for Firebug users, requesting approval for 7.
Attachment #550878 - Attachment description: a bug fix, maybe the bug fix → fix and a test
Attachment #550878 - Flags: approval-mozilla-aurora?
Comment on attachment 550878 [details] [diff] [review]
fix and a test

Land it this week please, so that it makes it before migration
Attachment #550878 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
qa- as no QA fix verification needed
Whiteboard: [inbound] → [inbound][qa-]
You need to log in before you can comment on or make changes to this bug.