732806 - Profiler Crash [@ mozilla::FramePointerStackWalk ]

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Benoit Girard (:BenWa)]

I'm able to reliably reproduce this crash using this applet:
http://janus.astro.umd.edu/orbits/nbdy/rstar.html

Just play around, it takes me about 1-2 minutes to crash while playing with the applet and interacting with the scroll bar.

bp-21d12f9a-0c74-493d-bfd3-aabd02120304
bp-4e22924e-c48b-4cc4-abe7-85a4b2120304
f5d3ad97-f015-4955-8de5-dc30f2120304
500c0ffa-1444-4238-83c6-d62f32120304

Comment 1

[Markus Stange [:mstange]]

The crash address is 0x0 and the crashing line is "void **next = (void**)*bp", so it looks like bp is null there. I think all we need is a null check of bp at the beginning of the function. If bp isn't null at the beginning, it can't become null during the loop because the "next <= bp" condition would break us out of the loop.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Attachment: This patch should help

Comment 3

[(no longer active)]

Comment on attachment 603014 [details] [diff] [review]
This patch should help

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

Looks good, but please make sure that you also modify the other FramePointerStackWalk implementation I added yesterday.  r=me with that.

Comment 4

[Markus Stange [:mstange]]

Wouldn't the crash address in Benoit's reports be different from 0x0 if bp were too large (which is what this patch protects from)?

Comment 5

[Jeff Muizelaar [:jrmuizel]]

(In reply to Markus Stange from comment #4)
> Wouldn't the crash address in Benoit's reports be different from 0x0 if bp
> were too large (which is what this patch protects from)?

Quite. It seems like we need to check that the bp we start with is sane.

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/00afd602bf72

Comment 7

[Benoit Girard (:BenWa)]

Attachment: Regression

I think you got the stack start/end backwards, we were always returning.

Comment 8

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7de086f7aca0

Comment 9

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/mozilla-central/rev/7de086f7aca0