Breakpad symbol dumping for Windows x64

RESOLVED FIXED in mozilla32

Status

()

Toolkit
Crash Reporting
RESOLVED FIXED
8 years ago
3 months ago

People

(Reporter: ted, Assigned: Mikhail I. Izmestev)

Tracking

(Blocks: 2 bugs)

Trunk
mozilla32
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
Breakpad mostly works for 64-bit Windows. The exception handler works, we generate minidumps fine. The symbol dumping even works, except it doesn't include the unwind data necessary for walking the stack on x86-64. The data to do this is present in the executable file, as described here:
http://msdn.microsoft.com/en-us/library/1eyas8tf.aspx

In order to have a fully functioning Breakpad on 64-bit Windows, we'll need to support dumping this data. jimb suggested that we might be able to shoehorn it into the STACK CFI record format that he invented for storing DWARF CFI data, which would mean that no processor-side changes would be necessary to process it.

Comment 1

7 years ago
I have hit this on the automation runs I am running. For now I have disabled breaking on buildsymbols failure.
Blocks: 471090
(Reporter)

Comment 2

7 years ago
Created attachment 449081 [details]
Sample code, read and display unwind info from x86-64 executables

I poked around the Microsoft documentation and spent a little bit of time writing this. It uses the ImageHlp APIs to locate and display the unwind info in the same format as dumpbin -UNWINDINFO (which made it easier to determine that I was interpreting the data correctly).

Implementing actual symbol dumping would involve taking this code and using the unwind data to produce STACK CFI records or STACK WIN records. I'm not terribly familiar with the x86-64 ABI, but I could probably fumble through it.

Updated

7 years ago
Duplicate of this bug: 578321

Updated

7 years ago
Blocks: 636467

Updated

6 years ago
Blocks: 677580
(Assignee)

Comment 4

6 years ago
Created attachment 591754 [details] [diff] [review]
patch adds unwind info for win64

based on Ted's example

not yet implemented registers information
not tested with chained unwinding
not tested with UWOP_PUSH_MACHFRAME and UWOP_SET_FPREG codes

Comment 5

6 years ago
Wow, thanks for the patch! I don't know the Win64 unwinding information format, but I can give you some procedural pointers.

First, you probably want to submit this patch upstream, at the Google Breakpad project, and get it reviewed and landed there. Mozilla tries not to carry local patches to Breakpad if we can avoid it. Their patch tracker is: http://breakpad.appspot.com/

Second, you need to make sure you're conforming to the Google C++ coding standards:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml

For example, you don't want to be sticking definitions in the midst of #includes.
(Assignee)

Comment 6

6 years ago
Thanks Jim, 
I will add support of chained unwinding, because most of system dlls use it.

registers information can be added later
(Assignee)

Comment 7

6 years ago
patch added to review http://breakpad.appspot.com/345002/
(Reporter)

Comment 8

6 years ago
Cool, thanks for working on this!
Assignee: ted.mielczarek → izmmishao5
(Assignee)

Comment 9

6 years ago
Ted, how long patch can wait on review before rejected or applied?
Ted's on paternity leave as far as I know, so the review may be delayed. Sorry.
(Reporter)

Comment 11

6 years ago
Mikhail: sometimes reviews get delayed because everyone working on Breakpad has other tasks as their primary focus. I'll touch base with Mark and try to get this moving forward. I appreciate your work on it!
(Assignee)

Comment 12

6 years ago
Thanks Ted, I found bug with handling UWOP_PUSH_MACHFRAME, fix will tomorrow
(Assignee)

Comment 13

5 years ago
I do not know how long will the review, but I have a few months of successful using of breakpad with Windows x64 on our project.
Comment on attachment 591754 [details] [diff] [review]
patch adds unwind info for win64

If you don't request review it won't happen :-)
Attachment #591754 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 15

5 years ago
(In reply to Ludovic Hirlimann [:Usul] from comment #14)
> Comment on attachment 591754 [details] [diff] [review]
> patch adds unwind info for win64
> 
> If you don't request review it won't happen :-)

review is at http://breakpad.appspot.com/345002/
(Reporter)

Comment 16

5 years ago
Sorry, this fell off my radar. I'll try to get it reviewed and landed soon.
(Reporter)

Comment 17

5 years ago
Comment on attachment 591754 [details] [diff] [review]
patch adds unwind info for win64

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

I've reviewed this upstream.
Attachment #591754 - Flags: review?(ted.mielczarek)
(Reporter)

Comment 18

3 years ago
This is being finished up upstream here:
https://breakpad.appspot.com/1264002/
dump_syms is updated by bug 1003085.  So as long as I check crash reporter, stack trace is correct.

Ted, is there remained issue?
(Reporter)

Comment 20

3 years ago
No, this is fixed, thanks for the reminder!
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.