Profiler Crash [@ mozilla::FramePointerStackWalk ]

RESOLVED FIXED in mozilla13

Status

()

Core
Gecko Profiler
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: BenWa, Assigned: jrmuizel)

Tracking

unspecified
mozilla13
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
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
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.
(Assignee)

Comment 2

5 years ago
Created attachment 603014 [details] [diff] [review]
This patch should help
Attachment #603014 - Flags: review?(ehsan)
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.
Attachment #603014 - Flags: review?(ehsan) → review+
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)?
(Assignee)

Comment 5

5 years ago
(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.
https://hg.mozilla.org/mozilla-central/rev/00afd602bf72
Assignee: nobody → jmuizelaar
Target Milestone: --- → mozilla13
(Reporter)

Comment 7

5 years ago
Created attachment 604711 [details] [diff] [review]
Regression

I think you got the stack start/end backwards, we were always returning.
Attachment #604711 - Flags: review?(jmuizelaar)
(Assignee)

Updated

5 years ago
Attachment #604711 - Flags: review?(jmuizelaar) → review+
(Reporter)

Comment 8

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7de086f7aca0
https://hg.mozilla.org/mozilla-central/rev/7de086f7aca0
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.