Sentry tests from 04/03/2017 #15
Conversation
|
@HazAT, It will cover your contributions to all Microsoft-managed open source projects. |
|
@HazAT, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request. |
|
If someone already checking this ... |
|
Hi, thanks for providing the data and congrats on the good results. We'll wait for your additional tests then and will check the details once you got back. Best, |
|
Perfect! |
|
I've updated all the reports and provided direct links to the crashes because there were already so many crashes in that account that it was kind of hard to find the ones in the JSON files. |
|
Would you mind letting us know which Xcode version you were using? We want to add this information in the future as it makes quite a difference too |
|
Of course it was ... |
|
Hey, just wanted to ask if there is anything where we could help to get this merged? |
|
@HazAT no, I just need some time over here to go over it. Planning to do this later this week. Sorry for the delay :( |
ios/11/_sentry_arm64.crash
Outdated
| Thread 0 name: | ||
| Thread 0 Crashed: | ||
| 0 CrashLibiOS 0x1000cfd24 -[CRLCrashGarbage crash] (CRLCrashGarbage.m:52) | ||
| 1 CrashLibiOS 0x1000cfd20 -[CRLCrashGarbage crash] (CRLCrashGarbage.m:41) |
There was a problem hiding this comment.
Reporting the same method twice with different lines and the method doesn't call itself recursively results in this being not complete. So this frame should be marked as invalid frame and the result for 64bit here is incomplete.
ios/19/_sentry_arm64.crash
Outdated
| Thread 0 name: | ||
| Thread 0 Crashed: | ||
| 0 CrashLibiOS 0x10004c2dc -[CRLCrashOverwriteLinkRegister crash] (CRLCrashOverwriteLinkRegister.m:53) | ||
| 1 CrashLibiOS 0x10004c2c0 -[CRLCrashOverwriteLinkRegister crash] (CRLCrashOverwriteLinkRegister.m:42) |
There was a problem hiding this comment.
Reporting the same method twice with different lines and the method doesn't call itself recursively results in this being not complete. So this frame should be marked as invalid frame and the result for 64bit here is incomplete.
There was a problem hiding this comment.
You are absolutely correct. Going to get this checked out.
ios/19/_sentry_armv7.crash
Outdated
| Thread 0 name: | ||
| Thread 0 Crashed: | ||
| 0 CrashLibiOS 0x219e52 -[CRLCrashOverwriteLinkRegister crash] (CRLCrashOverwriteLinkRegister.m:53) | ||
| 1 CrashLibiOS 0x219e4f -[CRLCrashOverwriteLinkRegister crash] (CRLCrashOverwriteLinkRegister.m:49) |
There was a problem hiding this comment.
Reporting the same method twice with different lines and the method doesn't call itself recursively results in this being not complete. So this frame should be marked as invalid frame and the result for 32bit here is incomplete.
ios/22/_sentry_arm64.crash
Outdated
| Thread 0 name: | ||
| Thread 0 Crashed: | ||
| 0 CrashLibiOS 0x1000bc500 @objc CRLCrashSwift.crash() -> () | ||
| 1 CrashLibiOS 0x1000bc500 CRLCrashSwift.crash() -> () (CRLCrashSwift.swift:36) |
There was a problem hiding this comment.
Reporting the same method twice with different lines and the method doesn't call itself recursively results in this being not complete. So this frame should be marked as invalid frame and the result for 64bit here is incomplete.
There was a problem hiding this comment.
This is actually contained like this in the DWARF file as an expanded inline frame.
There was a problem hiding this comment.
There can't be two frames with the same memory address and different results. This should be shown as one frame with the combined result. Otherwise this should be read by the user as frame 0 is called by frame 1. Which is impossible with the given data.
There was a problem hiding this comment.
I will see if I can find a better visualization for these things. We currently show the same address for expanded inline frames because we cannot recover the actual address where the call to the inlined frame happened.
There was a problem hiding this comment.
I just looked into this a bit more in detail to see what xcode does. First of all there is definitely a bug in our implementation which I will fix. Inline frames were expanded the wrong way around.
However I did compare to xcode how it displays inline frames and generally it's not very different to what we do. The main difference is that it mentions the inline expansion in the output:
* frame #0: 0x000000010012052c CrashLibiOS`@objc CRLCrashSwift.crash() -> () [inlined] CrashLibiOS.CRLCrashSwift.crash () -> () at CRLCrashSwift.swift:36 [opt]
frame #1: 0x000000010012052c CrashLibiOS`@objc CRLCrashSwift.crash() -> () at CRLCrashSwift.swift:0 [opt]
We can add an [inline] into to the stacktrace output. Would that be acceptable?
There was a problem hiding this comment.
I changed the rendering to be this now:
0 CrashLibiOS 0x10004052c [inlined] CRLCrashSwift.crash() -> () (CRLCrashSwift.swift:36)
1 CrashLibiOS 0x10004052c @objc CRLCrashSwift.crash() -> ()
Let me know if that works for you.
There was a problem hiding this comment.
I am not sure what you are doing, but there should be only one single frame. This is what the iOS generated crash report looks like, not sure how you are referring to Xcode otherwise. Maybe your crash reporting library is reporting too many frames?
0 CrashLibiOS 0x00000001000f052c @objc CRLCrashSwift.crash() -> () (CRLCrashSwift.swift:0)
1 CrashProbeiOS 0x000000010003b840 -[CRLDetailViewController doCrash] (CRLDetailViewController.m:58)
So showing two frames is wrong for the same reasons I mentioned before.
There was a problem hiding this comment.
Our goal was to get as close as possible to the backtrace you get when you see the crash in xcode/lldb. The way to do that is to also handle inline frames from the DWARF data.
This is indeed intentional. The crash reporting library is only producing one frame which with the data contained in the dsym DWARF data can expand to multiple. In this case the apple conpiler stores an inline function call in the debug data and we support that. This is consistent with backtraces generated by xcode/lldb. We do want to keep this feature enabled because it gives signficiantly better stacktraces at higher optimization levels in case the compiler inlines small functions.
There was a problem hiding this comment.
From a users and user source code respective and marking it as correct I expect the resulting crash report to have one single line. Showing what you have here should result in the report categorized as incomplete and the second line as being wrong.
There was a problem hiding this comment.
Then we will mark the report accordingly.
ios/22/_sentry_armv7.crash
Outdated
| Thread 0 name: | ||
| Thread 0 Crashed: | ||
| 0 CrashLibiOS 0x189fe8 @objc CRLCrashSwift.crash() -> () | ||
| 1 CrashLibiOS 0x189fe8 CRLCrashSwift.crash() -> () (CRLCrashSwift.swift:36) |
There was a problem hiding this comment.
Reporting the same method twice with different lines and the method doesn't call itself recursively results in this being not complete. So this frame should be marked as invalid frame and the result for 32bit here is incomplete.
|
All tests have been updated and I've marked the swift test as incomplete 😢 |
|
Thanks a lot guys, looks good and congrats to the great results too! Looks like CrashProbe was of good help for you too :) |
ghost
merged commit
|
The data is now available on the website. Thanks again for providing the data and using the test suite :) |
|
Thank you 👍 |
|
The data is updated and the Swift test case now reports as correct. I forgot to update the comment in the report itself. Do you want to provide updated tests also with the NX test case and include a fix for that? |
|
Hey, |
|
Sounds good. Regarding the Swift frames, however you want. I don't think it is critical as I made the rating "accurate", so only the hint if someone is looking into the details may confuse. It's up to you :) |
Provider name: Sentry
Repository URL: https://github.com/getsentry/CrashProbe
URL to the data: https://sentry.io/sentry-jj/crashprobe/
Username and password for the account:
daniel+crashprobe@sentry.io:9p7U{sB64h7n2{]igCq&*/gwDate of testing: 04/03/2017
SDK-Version: 2.1.7 (Swift 3)
Logo: https://sentry.io/branding/
Xcode Version 8.2.1 (8C1002)
Test environment per architecture:
arm7: iPad mini (P107AP), iOS 9.3.5
arm64: iPod Touch 6G (N102AP), iOS 10.2.1