Skip to content
This repository has been archived by the owner on Feb 15, 2022. It is now read-only.

Add support for macOS __crash_info data #29

Merged
merged 1 commit into from May 5, 2021

Conversation

steven-michaud
Copy link
Contributor

@steven-michaud steven-michaud commented May 3, 2021

I'm creating this pull request to follow up https://bugzilla.mozilla.org/show_bug.cgi?id=1577886, a fix for which landed a few days ago. This fix needs a followup fix, but the patch in my pull request has already been updated along these lines.

I've tested this patch using make build and make shell. It seems to work fine. But I do have one question about the output of stackwalker <MINIDUMPFILE> or stackwalker --pretty <MINIDUMPFILE>. As my patch currently stands, the output looks like this:

    "mac_crash_info" : {
       "num_records" : 2,
       "records" : [
          {
             "message" : "CryptKit fatal error: Raise test exception from _pthread_cond_wait(2)",
             "module" : "/System/Library/Frameworks/Security.framework/Versions/A/Security"
          },
          {
             "message" : "abort() called",
             "module" : "/usr/lib/system/libsystem_c.dylib"
          }
       ]
    },

But I could also make it look like this:

    "mac_crash_info" : {
       "num_records" : 2,
       {
           "message" : "CryptKit fatal error: Raise test exception from _pthread_cond_wait(2)",
           "module" : "/System/Library/Frameworks/Security.framework/Versions/A/Security"
       },
       {
           "message" : "abort() called",
           "module" : "/usr/lib/system/libsystem_c.dylib"
       }
    },

Or this:

    "mac_crash_info" : {
       {
           "message" : "CryptKit fatal error: Raise test exception from _pthread_cond_wait(2)",
           "module" : "/System/Library/Frameworks/Security.framework/Versions/A/Security"
       },
       {
           "message" : "abort() called",
           "module" : "/usr/lib/system/libsystem_c.dylib"
       }
    },

Do you have a preference?

@willkg
Copy link
Collaborator

willkg commented May 3, 2021

I was thinking about this and how the data could get used in Socorro and I don't think I have a preference. Having a "results" intermediary key allows us to expand in the future with additional bits. I don't know what other bits could be interesting.

I think I'm fine with what's implemented.

I'll try to find some time to test this out this week. Do we have any crash reports that have been submitted to Crash Stats that I can test with?

@steven-michaud
Copy link
Contributor Author

steven-michaud commented May 3, 2021

Do we have any crash reports that have been submitted to Crash Stats that I can test with?

Here are two that I just made, using today's mozilla-central nightly and my HookCase hook library from https://bugzilla.mozilla.org/show_bug.cgi?id=1577886#c12. Both should contain some __crash_info data:

https://crash-stats.mozilla.org/report/index/1bc9e790-d009-4419-a87a-e4af50210503
https://crash-stats.mozilla.org/report/index/366161a6-34a0-4411-b016-0fad30210503

@willkg
Copy link
Collaborator

willkg commented May 4, 2021

Awesome--that helps a lot!

Copy link
Collaborator

@willkg willkg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me!

@willkg
Copy link
Collaborator

willkg commented May 4, 2021

@gabrielesvelto Any objections to this?

@gabrielesvelto
Copy link
Collaborator

No objections. I asked Steven to get the patch into Socorro after having deployed it in m-c.

@steven-michaud
Copy link
Contributor Author

As best I can tell, I can't merge this myself. So someone please do it for me :-)

@steven-michaud
Copy link
Contributor Author

And once this has merged, there's still another step (or steps) to take:

We need to get a summary of "mac_crash_info" onto the "details" page of crash reports at https://crash-stats.mozilla.org/. And we need to make this information searchable. I have ideas about both of these. Should I open a bmo bug to discuss them?

@gabrielesvelto
Copy link
Collaborator

And once this has merged, there's still another step (or steps) to take:

We need to get a summary of "mac_crash_info" onto the "details" page of crash reports at https://crash-stats.mozilla.org/. And we need to make this information searchable. I have ideas about both of these. Should I open a bmo bug to discuss them?

Yes, open a bug under the Socorro component. Since we don't know exactly what these strings might contain we cannot make them publicly visible right away. We first have to make sure that they do not contain arbitrary data about the user because if they do they'll have to be protected content. Also we might need a data review before adding new fields to Socorro.

@steven-michaud
Copy link
Contributor Author

@willkg willkg merged commit 9a55067 into mozilla-services:main May 5, 2021
@willkg
Copy link
Collaborator

willkg commented May 5, 2021

Now that this is merged, I need to do a release and then pull it into Socorro and then do a prod deploy of that. Typically that takes me a couple of days to work through.

@steven-michaud
Copy link
Contributor Author

Now that this is merged, I need to do a release and then pull it into Socorro and then do a prod deploy of that. Typically that takes me a couple of days to work through.

I just triggered another crash with __crash_info data. It's still not showing up on the "raw data" page of the crash report. Have you done this yet?

https://crash-stats.mozilla.org/report/index/2e7cb5a1-832d-4fea-8fa7-67fc20210510

@willkg
Copy link
Collaborator

willkg commented May 10, 2021

No, I haven't. I'm going to update minidump-stackwalk in Socorro along with the rest of the work in bug 1709658.

@steven-michaud
Copy link
Contributor Author

OK, that makes sense.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants