Closed
Bug 732806
Opened 14 years ago
Closed 14 years ago
Profiler Crash [@ mozilla::FramePointerStackWalk ]
Categories
(Core :: Gecko Profiler, defect)
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: BenWa, Assigned: jrmuizel)
Details
Attachments
(2 files)
|
3.92 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
|
1.08 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
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•14 years ago
|
||
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•14 years ago
|
||
Attachment #603014 -
Flags: review?(ehsan)
Comment 3•14 years ago
|
||
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+
Comment 4•14 years ago
|
||
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•14 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.
Comment 6•14 years ago
|
||
Assignee: nobody → jmuizelaar
Target Milestone: --- → mozilla13
| Reporter | ||
Comment 7•14 years ago
|
||
I think you got the stack start/end backwards, we were always returning.
Attachment #604711 -
Flags: review?(jmuizelaar)
| Assignee | ||
Updated•14 years ago
|
Attachment #604711 -
Flags: review?(jmuizelaar) → review+
| Reporter | ||
Comment 8•14 years ago
|
||
Comment 9•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•