Closed Bug 673789 Opened 9 years ago Closed 9 years ago

minidump_generator.cc uses mach/ppc/thread_status.h unconditionally

Categories

(Toolkit :: Crash Reporting, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: espindola, Assigned: espindola)

Details

(Whiteboard: fixed-in-bs)

Attachments

(1 file, 2 obsolete files)

mach/ppc/thread_status.h has been removed from the 10.7 sdk. We should add an ifdef for it or just drop it if no breakpad user supports ppc anymore.
Assignee: nobody → respindola
Attached patch drop support for ppc (obsolete) — Splinter Review
Let me know if I should add a configure check instead.
Attachment #548028 - Flags: review?(ted.mielczarek)
Comment on attachment 548028 [details] [diff] [review]
drop support for ppc

I don't think we're going to drop support for PPC upstream, so we'll probably want to #ifdef this. Something like MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_7 ?

Also, if you could generate this against upstream Breakpad SVN that would be helpful for me:
http://code.google.com/p/google-breakpad/source/checkout
Attachment #548028 - Flags: review?(ted.mielczarek) → review-
Attached patch Only use ppc on mac os < 10.7 (obsolete) — Splinter Review
Attachment #548028 - Attachment is obsolete: true
Attachment #548354 - Flags: review?(ted.mielczarek)
Attachment #548354 - Attachment is obsolete: true
Attachment #548354 - Flags: review?(ted.mielczarek)
Attachment #548355 - Flags: review?(ted.mielczarek)
Comment on attachment 548355 [details] [diff] [review]
define HAS_PPC_SUPPORT in the right place

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

Okay, this looks good. I'll land it upstream for you.
Attachment #548355 - Flags: review?(ted.mielczarek) → review+
Thanks!
Should we backport or just update breakpad?
We can land your patch in mozilla-central as well, I still have a few more patches to upstream before we can update our Breakpad snapshot.
>+#ifdef HAS_PPC_SUUPORT
>     case CPU_TYPE_POWERPC:
>       return WriteStackPPC(state, stack_location);
>     case CPU_TYPE_POWERPC64:
>       return WriteStackPPC64(state, stack_location);
>+#endif

Looks like there is typo in patch. s/SUUPORT/SUPPORT
http://hg.mozilla.org/mozilla-central/rev/3a7ad3683fbf
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.