Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add EXC_BAD_ACCESS subtypes to events #2667

Merged
merged 5 commits into from
Feb 6, 2023

Conversation

philipphofmann
Copy link
Member

@philipphofmann philipphofmann commented Jan 30, 2023

📜 Description

Add EXC_BAD_ACCESS subtypes to unhandled errors on arm CPUs. Previously the CrashDoctor always diagnosed a memory crash as Attempted to dereference garbage pointer at 0x13fd4582e. Now the SDK replaces the generic message with the specific subtype.

This change has no impact on grouping.

💡 Motivation and Context

Came up while investigating #2662. This is also related to #2665.

💚 How did you test it?

Generating a crash report with a subtype EXC_ARM_DA_ALIGN and unit tests.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

🔮 Next steps

Add EXC_BAD_ACCESS subtypes to unhandled errors on arm CPUs.
Previously the CrashDoctor always diagnosed a memory crash as
Attempted to dereference garbage pointer at 0x13fd4582e. Now the SDK
replaces the generic message with the specific subtype.
@philipphofmann philipphofmann changed the title Feat/mach exception codes feat: Add EXC_BAD_ACCESS subtypes to events Jan 30, 2023
@github-actions
Copy link

github-actions bot commented Jan 30, 2023

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 3b50631

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

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

Changes LGTM.

Tests need fixing and we're good to go.

@codecov
Copy link

codecov bot commented Feb 3, 2023

Codecov Report

Merging #2667 (eb0833e) into main (075a044) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head eb0833e differs from pull request most recent head 3b50631. Consider uploading reports for the commit 3b50631 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2667      +/-   ##
==========================================
- Coverage   80.61%   80.57%   -0.05%     
==========================================
  Files         247      247              
  Lines       22856    22869      +13     
  Branches    10120    10125       +5     
==========================================
+ Hits        18426    18427       +1     
- Misses       3972     3981       +9     
- Partials      458      461       +3     
Impacted Files Coverage Δ
...rces/SentryCrash/Recording/Tools/SentryCrashMach.c 35.65% <ø> (ø)
Sources/SentryCrash/Recording/SentryCrashDoctor.m 49.04% <100.00%> (+1.68%) ⬆️
Sources/Sentry/SentryProfilingLogging.mm 0.00% <0.00%> (-51.86%) ⬇️
Sources/Sentry/include/SentryProfilingLogging.hpp 45.45% <0.00%> (-9.10%) ⬇️
Sources/Sentry/SentryThreadMetadataCache.cpp 91.30% <0.00%> (-4.35%) ⬇️
Sources/Sentry/SentryThreadHandle.cpp 69.94% <0.00%> (-2.74%) ⬇️
Sources/Sentry/SentryBacktrace.cpp 88.39% <0.00%> (-1.79%) ⬇️
Sources/Sentry/SentryFileManager.m 95.06% <0.00%> (-0.42%) ⬇️
Sources/Sentry/SentryCrashIntegration.m 98.66% <0.00%> (+0.03%) ⬆️
Sources/Sentry/SentrySerialization.m 87.97% <0.00%> (+0.12%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 075a044...3b50631. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Feb 6, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1250.65 ms 1268.24 ms 17.59 ms
Size 20.76 KiB 420.55 KiB 399.79 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b2f82fa 1237.78 ms 1256.02 ms 18.24 ms
ddc9b9a 1201.71 ms 1226.70 ms 24.99 ms
dc0db9e 1222.10 ms 1240.90 ms 18.80 ms
4977fbc 1231.55 ms 1239.80 ms 8.25 ms
156e771 1228.06 ms 1242.64 ms 14.58 ms
9faf217 1268.86 ms 1274.82 ms 5.96 ms
15b8c61 1223.16 ms 1244.83 ms 21.67 ms
83d2d84 1211.31 ms 1227.34 ms 16.03 ms
1bd0055 1207.57 ms 1223.10 ms 15.53 ms
443723a 1205.24 ms 1220.52 ms 15.28 ms

App size

Revision Plain With Sentry Diff
b2f82fa 20.76 KiB 419.62 KiB 398.86 KiB
ddc9b9a 20.76 KiB 420.40 KiB 399.65 KiB
dc0db9e 20.76 KiB 419.62 KiB 398.86 KiB
4977fbc 20.76 KiB 419.86 KiB 399.10 KiB
156e771 20.76 KiB 419.70 KiB 398.94 KiB
9faf217 20.76 KiB 419.70 KiB 398.94 KiB
15b8c61 20.76 KiB 419.67 KiB 398.91 KiB
83d2d84 20.76 KiB 419.66 KiB 398.90 KiB
1bd0055 20.76 KiB 420.22 KiB 399.46 KiB
443723a 20.76 KiB 414.44 KiB 393.68 KiB

Previous results on branch: feat/mach-exception-codes

Startup times

Revision Plain With Sentry Diff
260fe58 1217.55 ms 1240.36 ms 22.81 ms

App size

Revision Plain With Sentry Diff
260fe58 20.76 KiB 420.55 KiB 399.79 KiB

@philipphofmann philipphofmann merged commit bd2afa6 into main Feb 6, 2023
@philipphofmann philipphofmann deleted the feat/mach-exception-codes branch February 6, 2023 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants