676023 - crash js::StackIter::settleOnNewState

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

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.

Comment 1

[Luke Wagner [:luke]]

Attachment: telemetry patch

Here's the first patch.  Pushing to try first; maybe we'll get lucky.

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 550130 [details] [diff] [review]
telemetry patch

No luck; try's clean.

Comment 3

[Luke Wagner [:luke]]

Landed telemetry patch:
http://hg.mozilla.org/integration/mozilla-inbound/rev/b0e4ea1084d1

Comment 4

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/b0e4ea1084d1

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 550130 [details] [diff] [review]
telemetry patch

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

Is && really what you mean?

Comment 6

[Luke Wagner [:luke]]

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

It is.

Comment 8

[Luke Wagner [:luke]]

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.

Comment 9

[Luke Wagner [:luke]]

Attachment: fix and a test

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.

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 550878 [details] [diff] [review]
fix and a test

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

nice test case!

Comment 11

[Luke Wagner [:luke]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/6181622382cf

Comment 12

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/6181622382cf

Comment 13

[Luke Wagner [:luke]]

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 14

[Luke Wagner [:luke]]

Comment on attachment 550878 [details] [diff] [review]
fix and a test

Simple patch, should decrease random crashiness for Firebug users, requesting approval for 7.

Comment 15

[Johnathan Nightingale [:johnath]]

Comment on attachment 550878 [details] [diff] [review]
fix and a test

Land it this week please, so that it makes it before migration

Comment 16

[Luke Wagner [:luke]]

http://hg.mozilla.org/releases/mozilla-aurora/rev/800e20b4890f

Comment 17

[u279076]

qa- as no QA fix verification needed

Comment 18

[Loic]

Still crashing in FF7.0.1: https://crash-stats.mozilla.com/report/index/815c5157-fe01-449d-896e-898222111028