feat: Added support for arm64e

[brustolin]

📜 Description

A developer may choose the arm64e architecture for their iOS app to benefit from enhanced security features.
Sentry was not compiling for this architecture due to issues with accessing machine context properties

By using arm_thread_state64_get_* macros, we can read the necessary information.

💡 Motivation and Context

closes #3344

💚 How did you test it?

Samples

📝 Checklist

You have to check all boxes before merging:

• 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 change or entry added to the changelog.
• No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

[github-actions]

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

Generated by 🚫 dangerJS against 2ad5f7c

[brustolin]

Although the project is now compiling, when we run profiling it crashes, I couldn't figure it out the problem.

@armcknight maybe you would like to take a look at this?
Try the iOS13-Swift sample. It needs to run on a real device.

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1227.51 ms	1245.79 ms	18.28 ms
Size	21.58 KiB	418.44 KiB	396.86 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
4977fbc	1217.26 ms	1239.82 ms	22.56 ms
3a01b17	1212.12 ms	1221.80 ms	9.68 ms
3297d6e	1228.57 ms	1255.04 ms	26.47 ms
5e78d2b	1229.90 ms	1252.94 ms	23.04 ms
a423161	1234.41 ms	1246.41 ms	12.00 ms
2124551	1246.10 ms	1254.84 ms	8.74 ms
3f366ee	1244.49 ms	1257.28 ms	12.79 ms
c5232ea	1226.06 ms	1246.57 ms	20.50 ms
4afae53	1217.65 ms	1229.27 ms	11.62 ms
7bb0873	1215.65 ms	1235.00 ms	19.35 ms

App size

Revision	Plain	With Sentry	Diff
4977fbc	20.76 KiB	419.85 KiB	399.10 KiB
3a01b17	20.76 KiB	436.33 KiB	415.57 KiB
3297d6e	21.58 KiB	418.45 KiB	396.86 KiB
5e78d2b	22.85 KiB	411.17 KiB	388.32 KiB
a423161	22.85 KiB	406.69 KiB	383.85 KiB
2124551	22.85 KiB	411.69 KiB	388.84 KiB
3f366ee	20.76 KiB	427.84 KiB	407.08 KiB
c5232ea	22.85 KiB	413.98 KiB	391.13 KiB
4afae53	22.84 KiB	402.08 KiB	379.24 KiB
7bb0873	22.85 KiB	407.09 KiB	384.24 KiB

Previous results on branch: fix/arm64e-lib__fp

Startup times

Revision	Plain	With Sentry	Diff
3930ded	1211.16 ms	1228.40 ms	17.24 ms

App size

Revision	Plain	With Sentry	Diff
3930ded	22.85 KiB	414.71 KiB	391.86 KiB

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.455%. Comparing base (4eea4d1) to head (2ad5f7c).
Report is 4 commits behind head on main.

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #3398       +/-   ##
=============================================
+ Coverage   91.424%   91.455%   +0.030%     
=============================================
  Files          628       629        +1     
  Lines        50530     50619       +89     
  Branches     18255     18338       +83     
=============================================
+ Hits         46197     46294       +97     
+ Misses        4240      4233        -7     
+ Partials        93        92        -1     

Files with missing lines	Coverage Δ
Sources/Sentry/SentryBacktrace.cpp	85.714% <ø> (ø)
Sources/Sentry/SentryProfiler.mm	89.000% <100.000%> (+1.000%)	⬆️
Sources/Sentry/SentrySamplingProfiler.cpp	83.544% <100.000%> (ø)
Sources/Sentry/include/SentryThreadState.hpp	90.476% <100.000%> (+4.761%)	⬆️
...SentryCrash/Recording/Tools/SentryCrashCPU_arm64.c	100.000% <100.000%> (ø)
Tests/SentryProfilerTests/SentryBacktraceTests.mm	96.794% <100.000%> (ø)

... and 15 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

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

[philipphofmann]

The changes look good, but CI is complaining.

[armcknight]

Having a look. For reference, here is the crash:

It's happening when creating a profiler instance when there's already one in memory which gets destructed.

[brustolin]

It's happening when creating a profiler instance when there's already one in memory which gets destructed.

Thanks! Do we have plans to fix this already?

[armcknight]

Sorry for the late reply @brustolin ... we're currently investigating the root cause.

[brustolin]

Any update on this one @armcknight

[indragiek]

@armcknight @brustolin how do I repro this? I'm running on device with the iOS-Swift sample app and starting/ending profiles and I can't get it to crash.

[brustolin]

@armcknight @brustolin how do I repro this? I'm running on device with the iOS-Swift sample app and starting/ending profiles and I can't get it to crash.

Are you running it on a real device? Simulators don't crash. Run the app and start a transaction, it crashes for me.

[indragiek]

@brustolin I can't repro on an iPhone 14 Pro Max running iOS-Swift at the head of this branch. Are you testing with an arm64e or arm64 device? I tried with multiple concurrent transactions as well and everything seems to work fine. @armcknight do you have an older device you can test with?

[brustolin]

@brustolin I can't repro on an iPhone 14 Pro Max running iOS-Swift at the head of this branch. Are you testing with an arm64e or arm64 device? I tried with multiple concurrent transactions as well and everything seems to work fine. @armcknight do you have an older device you can test with?

Ow, sorry. iOS-Swift is not configured to run with arm64e, I must have roll it back. It only crashes when I compile the sample for arm64e. I will commit the change, you should have both options now.

[indragiek]

@brustolin Thanks, able to repro now. It appears this is crashing within profiling code any place where shared_ptr is used
and free'd (QueueMetadata, ThreadMetadataCache, and SamplingProfiler) so it isn't directly related to any specific piece of code. Replacing shared_ptr with unique_ptr in all of these places fixes the crash but I don't think it's the right way to solve the problem. Still looking into it.

[indragiek]

There is some relevant documentation on Apple's page about arm64e pointer authentication:

Pointer authentication can also expose latent bugs in existing code. In C++, it’s incorrect to call a virtual method using a declaration that differs from its definition. In practice, such calls typically succeed in arm64, but trigger a pointer authentication failure in arm64e. You might encounter this bug when using OS_OBJECT types like dispatch_queue_t and xpc_connection_t. You can’t pass instances of these types from C++ code to an Objective-C++ function (or vice versa) because they’re defined differently in Objective-C++ to support automatic reference counting (ARC).

I suspect something like this is causing the crash.

[armcknight]

I don't think [replacing shared_ptr with unique_ptr in all of these places is] the right way to solve the problem

Why not? We would potentially have to use std::move in places that create new references to these, if any, if my understanding of std::move is correct (based on my reading of https://stackoverflow.com/a/6876833). Are there other considerations?

[indragiek]

@armcknight No doubt we can make the change, I just would not be confident that it actually fixes the issue without understanding why using a shared_ptr here causes a crash, and only on a single architecture. If this is urgent and we need to ship a quick patch then we certainly can do that though.

[brustolin]

@armcknight @indragiek Any progress to solve this crash?

[armcknight]

@brustolin check out the changes Indragie pushed up recently. Do you still see the crash with those?

[brustolin]

Still getting an error at SamplingProfiler::~SamplingProfiler:

[armcknight]

@brustolin Would you mind pulling the commit I just added and trying again? That was a nasty merge you had to do, and it overwrote some of indragie's changes. Sorry about that. I just tried my test and it no longer crashes. The profiles I created both appeared in the dash, so things appear to still function normally.

[brustolin]

Same error I believe.

[armcknight]

@brustolin would you mind capturing your repro steps in a UI test so we can eliminate any variation between our machines and see what happens when we run it in CI?

[brustolin]

@armcknight I added a test, but this only crashes when running in the real device (its not even possible to run on a simulator).

[armcknight]

@brustolin Oh, I forgot that you were originally testing with iOS13-Swift. Is there no way to do it in the normal iOS-Swift sample app so we can test it in a simulator? That sample is still set up to build for the arm64e target. Is there something special about iOS13-Swift?

[brustolin]

The problem only occurs when the target is arm64e. This is the whole issue.

I don't want to change iOS-Swift because of this; we would have to test on real devices anyway.

[brustolin]

Hello @armcknight and @indragiek.

This have a higher priority now. It's affecting Xcode SwiftUI visualiser for macOS projects as described in here.

Do you have any suggestions what could we do?
Maybe turn of profiling for arm64e?

[brustolin]

Awesome, Thanks @indragiek for the fix!!

[philipphofmann]

I leave the review of the CPP code to @armcknight cause I'm not that knowledgeable in CPP.

[armcknight]

I'm not clear on why we can change std::shared_ptr to naked pointers vs using unique_ptr everywhere (partly because it's used in some places (https://github.com/getsentry/sentry-cocoa/pull/3398/files#diff-0397b590fd7ff98685a61fa9a99cceccf30bdf224754ba5f92448abd0852885bR80)... does C++ autobox naked pointers?)

[armcknight]

I'm satisfied with the answers to my last questions, let's ship it!