From ec692429f37cfaf85e923d72183bbe3e95cbe4a6 Mon Sep 17 00:00:00 2001 From: William Mak Date: Thu, 21 Nov 2019 11:36:46 -0500 Subject: [PATCH 1/4] fix(apple-crash-report): Name is an optional field in debug images - This uses `.get` and defaults to `unknown` on an image name, preventing a key error since the name is an optional field and not guaranteed to be part of the debug_image object --- src/sentry/lang/native/applecrashreport.py | 12 ++-- .../lang/native/test_applecrashreport.py | 67 +++++++++++++++++++ 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/src/sentry/lang/native/applecrashreport.py b/src/sentry/lang/native/applecrashreport.py index 413989af0506..c44824a1e7bc 100644 --- a/src/sentry/lang/native/applecrashreport.py +++ b/src/sentry/lang/native/applecrashreport.py @@ -124,7 +124,7 @@ def _convert_frame_to_apple_string(self, frame, next=None, number=0): offset = "" if frame.get("image_addr") is not None and ( not self.symbolicated - or (frame.get("function") or NATIVE_UNKNOWN_STRING) == NATIVE_UNKNOWN_STRING + or frame.get("function", NATIVE_UNKNOWN_STRING) == NATIVE_UNKNOWN_STRING ): offset = " + %s" % ( instruction_addr - slide_value - parse_addr(frame.get("symbol_addr")) @@ -134,17 +134,17 @@ def _convert_frame_to_apple_string(self, frame, next=None, number=0): file = "" if frame.get("filename") and frame.get("lineno"): file = " (%s:%s)" % ( - posixpath.basename(frame.get("filename") or NATIVE_UNKNOWN_STRING), + posixpath.basename(frame.get("filename", NATIVE_UNKNOWN_STRING)), frame["lineno"], ) - symbol = "%s%s" % (frame.get("function") or NATIVE_UNKNOWN_STRING, file) + symbol = "%s%s" % (frame.get("function", NATIVE_UNKNOWN_STRING), file) if next and parse_addr(frame.get("instruction_addr")) == parse_addr( next.get("instruction_addr") ): symbol = "[inlined] " + symbol return "%s%s%s%s%s" % ( str(number).ljust(4, " "), - image_name(frame.get("package") or NATIVE_UNKNOWN_STRING).ljust(32, " "), + image_name(frame.get("package", NATIVE_UNKNOWN_STRING)).ljust(32, " "), hex(instruction_addr).ljust(20, " "), symbol, offset, @@ -173,8 +173,8 @@ def _convert_debug_meta_to_binary_image_row(self, debug_image): return "%s - %s %s %s <%s> %s" % ( hex(image_addr), hex(image_addr + debug_image["image_size"] - 1), - image_name(debug_image["name"]), + image_name(debug_image.get("name", NATIVE_UNKNOWN_STRING)), self.context["device"]["arch"], (debug_image.get("id") or debug_image.get("uuid")).replace("-", "").lower(), - debug_image["name"], + debug_image.get("name", NATIVE_UNKNOWN_STRING), ) diff --git a/tests/sentry/lang/native/test_applecrashreport.py b/tests/sentry/lang/native/test_applecrashreport.py index f3769b635723..2ce8feee5e79 100644 --- a/tests/sentry/lang/native/test_applecrashreport.py +++ b/tests/sentry/lang/native/test_applecrashreport.py @@ -1,5 +1,6 @@ from __future__ import absolute_import +from sentry.constants import NATIVE_UNKNOWN_STRING from sentry.lang.native.applecrashreport import AppleCrashReport @@ -370,6 +371,72 @@ def test__convert_frame_to_apple_string(): ) +def test_apple_string_without_file_name(): + acr = AppleCrashReport( + debug_images=[ + { + "cpu_subtype": 3, + "cpu_type": 16777223, + "image_addr": "0x141c5000", + "image_size": 20480, + "image_vmaddr": "0x0", + "type": "apple", + "uuid": "4B5A054F-B7A1-3AD0-81E1-513B4DBE2A33", + }, + { + "cpu_subtype": 3, + "cpu_type": 16777223, + "image_addr": "0x1400c000", + "image_size": 266240, + "image_vmaddr": "0x0", + "type": "apple", + "uuid": "766DFB14-72EE-32D2-8961-687D32548F2B", + }, + { + "cpu_subtype": 3, + "cpu_type": 16777223, + "image_addr": "0x1406f000", + "image_size": 913408, + "image_vmaddr": "0x0", + "type": "apple", + "uuid": "BE602DC1-D3A0-3389-B8F4-922C37DEA3DC", + }, + ], + context={ + "device": { + "arch": "x86", + "family": "iPhone", + "freeMemory": 169684992, + "memorySize": 17179869184, + "model": "iPhone9,1", + "simulator": True, + "storageSize": 249695305728, + "type": "device", + "usableMemory": 14919622656, + }, + "os": { + "build": "16C67", + "bundleID": "com.rokkincat.SentryExample", + "bundleVersion": "2", + "kernel_version": "Darwin Kernel Version 16.3.0: Thu Nov 17 20:23:58 PST 2016; root:xnu-3789.31.2~1/RELEASE_X86_64", + "name": "iOS", + "type": "os", + "version": "10.2", + }, + }, + ) + binary_images = acr.get_binary_images_apple_string() + assert ( + binary_images + == "Binary Images:\n\ +0x1400c000 - 0x1404cfff {0} x86 <766dfb1472ee32d28961687d32548f2b> {0}\n\ +0x1406f000 - 0x1414dfff {0} x86 {0}\n\ +0x141c5000 - 0x141c9fff {0} x86 <4b5a054fb7a13ad081e1513b4dbe2a33> {0}".format( + NATIVE_UNKNOWN_STRING + ) + ) + + def test_get_binary_images_apple_string(): acr = AppleCrashReport( debug_images=[ From 4a9750596dd289f715f7f3fcecc747b8698174e5 Mon Sep 17 00:00:00 2001 From: William Mak Date: Thu, 21 Nov 2019 17:10:43 -0500 Subject: [PATCH 2/4] Revert "fix(apple-crash-report): Name is an optional field in debug images" This reverts commit ec692429f37cfaf85e923d72183bbe3e95cbe4a6. --- src/sentry/lang/native/applecrashreport.py | 12 ++-- .../lang/native/test_applecrashreport.py | 67 ------------------- 2 files changed, 6 insertions(+), 73 deletions(-) diff --git a/src/sentry/lang/native/applecrashreport.py b/src/sentry/lang/native/applecrashreport.py index c44824a1e7bc..413989af0506 100644 --- a/src/sentry/lang/native/applecrashreport.py +++ b/src/sentry/lang/native/applecrashreport.py @@ -124,7 +124,7 @@ def _convert_frame_to_apple_string(self, frame, next=None, number=0): offset = "" if frame.get("image_addr") is not None and ( not self.symbolicated - or frame.get("function", NATIVE_UNKNOWN_STRING) == NATIVE_UNKNOWN_STRING + or (frame.get("function") or NATIVE_UNKNOWN_STRING) == NATIVE_UNKNOWN_STRING ): offset = " + %s" % ( instruction_addr - slide_value - parse_addr(frame.get("symbol_addr")) @@ -134,17 +134,17 @@ def _convert_frame_to_apple_string(self, frame, next=None, number=0): file = "" if frame.get("filename") and frame.get("lineno"): file = " (%s:%s)" % ( - posixpath.basename(frame.get("filename", NATIVE_UNKNOWN_STRING)), + posixpath.basename(frame.get("filename") or NATIVE_UNKNOWN_STRING), frame["lineno"], ) - symbol = "%s%s" % (frame.get("function", NATIVE_UNKNOWN_STRING), file) + symbol = "%s%s" % (frame.get("function") or NATIVE_UNKNOWN_STRING, file) if next and parse_addr(frame.get("instruction_addr")) == parse_addr( next.get("instruction_addr") ): symbol = "[inlined] " + symbol return "%s%s%s%s%s" % ( str(number).ljust(4, " "), - image_name(frame.get("package", NATIVE_UNKNOWN_STRING)).ljust(32, " "), + image_name(frame.get("package") or NATIVE_UNKNOWN_STRING).ljust(32, " "), hex(instruction_addr).ljust(20, " "), symbol, offset, @@ -173,8 +173,8 @@ def _convert_debug_meta_to_binary_image_row(self, debug_image): return "%s - %s %s %s <%s> %s" % ( hex(image_addr), hex(image_addr + debug_image["image_size"] - 1), - image_name(debug_image.get("name", NATIVE_UNKNOWN_STRING)), + image_name(debug_image["name"]), self.context["device"]["arch"], (debug_image.get("id") or debug_image.get("uuid")).replace("-", "").lower(), - debug_image.get("name", NATIVE_UNKNOWN_STRING), + debug_image["name"], ) diff --git a/tests/sentry/lang/native/test_applecrashreport.py b/tests/sentry/lang/native/test_applecrashreport.py index 2ce8feee5e79..f3769b635723 100644 --- a/tests/sentry/lang/native/test_applecrashreport.py +++ b/tests/sentry/lang/native/test_applecrashreport.py @@ -1,6 +1,5 @@ from __future__ import absolute_import -from sentry.constants import NATIVE_UNKNOWN_STRING from sentry.lang.native.applecrashreport import AppleCrashReport @@ -371,72 +370,6 @@ def test__convert_frame_to_apple_string(): ) -def test_apple_string_without_file_name(): - acr = AppleCrashReport( - debug_images=[ - { - "cpu_subtype": 3, - "cpu_type": 16777223, - "image_addr": "0x141c5000", - "image_size": 20480, - "image_vmaddr": "0x0", - "type": "apple", - "uuid": "4B5A054F-B7A1-3AD0-81E1-513B4DBE2A33", - }, - { - "cpu_subtype": 3, - "cpu_type": 16777223, - "image_addr": "0x1400c000", - "image_size": 266240, - "image_vmaddr": "0x0", - "type": "apple", - "uuid": "766DFB14-72EE-32D2-8961-687D32548F2B", - }, - { - "cpu_subtype": 3, - "cpu_type": 16777223, - "image_addr": "0x1406f000", - "image_size": 913408, - "image_vmaddr": "0x0", - "type": "apple", - "uuid": "BE602DC1-D3A0-3389-B8F4-922C37DEA3DC", - }, - ], - context={ - "device": { - "arch": "x86", - "family": "iPhone", - "freeMemory": 169684992, - "memorySize": 17179869184, - "model": "iPhone9,1", - "simulator": True, - "storageSize": 249695305728, - "type": "device", - "usableMemory": 14919622656, - }, - "os": { - "build": "16C67", - "bundleID": "com.rokkincat.SentryExample", - "bundleVersion": "2", - "kernel_version": "Darwin Kernel Version 16.3.0: Thu Nov 17 20:23:58 PST 2016; root:xnu-3789.31.2~1/RELEASE_X86_64", - "name": "iOS", - "type": "os", - "version": "10.2", - }, - }, - ) - binary_images = acr.get_binary_images_apple_string() - assert ( - binary_images - == "Binary Images:\n\ -0x1400c000 - 0x1404cfff {0} x86 <766dfb1472ee32d28961687d32548f2b> {0}\n\ -0x1406f000 - 0x1414dfff {0} x86 {0}\n\ -0x141c5000 - 0x141c9fff {0} x86 <4b5a054fb7a13ad081e1513b4dbe2a33> {0}".format( - NATIVE_UNKNOWN_STRING - ) - ) - - def test_get_binary_images_apple_string(): acr = AppleCrashReport( debug_images=[ From cea9b87abe9d49cc96ed641853ae8be28890436a Mon Sep 17 00:00:00 2001 From: William Mak Date: Thu, 21 Nov 2019 17:42:11 -0500 Subject: [PATCH 3/4] fix(apple-crash-report): Name was deprecated for code file - Updates use of `name` to `code_file` - Since `code_file` is optional, using the same .get() or unknown pattern. --- src/sentry/lang/native/applecrashreport.py | 4 +- .../lang/native/test_applecrashreport.py | 75 ++++++++++++++++++- 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/src/sentry/lang/native/applecrashreport.py b/src/sentry/lang/native/applecrashreport.py index 413989af0506..e2b64f890f79 100644 --- a/src/sentry/lang/native/applecrashreport.py +++ b/src/sentry/lang/native/applecrashreport.py @@ -173,8 +173,8 @@ def _convert_debug_meta_to_binary_image_row(self, debug_image): return "%s - %s %s %s <%s> %s" % ( hex(image_addr), hex(image_addr + debug_image["image_size"] - 1), - image_name(debug_image["name"]), + image_name(debug_image.get("code_file") or NATIVE_UNKNOWN_STRING), self.context["device"]["arch"], (debug_image.get("id") or debug_image.get("uuid")).replace("-", "").lower(), - debug_image["name"], + debug_image.get("code_file") or NATIVE_UNKNOWN_STRING, ) diff --git a/tests/sentry/lang/native/test_applecrashreport.py b/tests/sentry/lang/native/test_applecrashreport.py index f3769b635723..b2e1665d99a4 100644 --- a/tests/sentry/lang/native/test_applecrashreport.py +++ b/tests/sentry/lang/native/test_applecrashreport.py @@ -1,5 +1,6 @@ from __future__ import absolute_import +from sentry.constants import NATIVE_UNKNOWN_STRING from sentry.lang.native.applecrashreport import AppleCrashReport @@ -379,7 +380,7 @@ def test_get_binary_images_apple_string(): "image_addr": "0x141c5000", "image_size": 20480, "image_vmaddr": "0x0", - "name": "/Users/haza/Library/Developer/CoreSimulator/Devices/DDB32F4C-97CF-4E2B-BD10-EB940553F223/data/Containers/Bundle/Application/8C286977-D498-44FF-B7BE-42BFE3DE38BD/SwiftExample.app/Frameworks/libswiftContacts.dylib", + "code_file": "/Users/haza/Library/Developer/CoreSimulator/Devices/DDB32F4C-97CF-4E2B-BD10-EB940553F223/data/Containers/Bundle/Application/8C286977-D498-44FF-B7BE-42BFE3DE38BD/SwiftExample.app/Frameworks/libswiftContacts.dylib", "type": "apple", "uuid": "4B5A054F-B7A1-3AD0-81E1-513B4DBE2A33", }, @@ -389,7 +390,7 @@ def test_get_binary_images_apple_string(): "image_addr": "0x1400c000", "image_size": 266240, "image_vmaddr": "0x0", - "name": "/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/System/Library/PrivateFrameworks/ContentIndex.framework/ContentIndex", + "code_file": "/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/System/Library/PrivateFrameworks/ContentIndex.framework/ContentIndex", "type": "apple", "uuid": "766DFB14-72EE-32D2-8961-687D32548F2B", }, @@ -399,7 +400,7 @@ def test_get_binary_images_apple_string(): "image_addr": "0x1406f000", "image_size": 913408, "image_vmaddr": "0x0", - "name": "/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/System/Library/PrivateFrameworks/CorePDF.framework/CorePDF", + "code_file": "/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/System/Library/PrivateFrameworks/CorePDF.framework/CorePDF", "type": "apple", "uuid": "BE602DC1-D3A0-3389-B8F4-922C37DEA3DC", }, @@ -437,6 +438,72 @@ def test_get_binary_images_apple_string(): ) +def test_binary_images_without_code_file(): + acr = AppleCrashReport( + debug_images=[ + { + "cpu_subtype": 3, + "cpu_type": 16777223, + "image_addr": "0x141c5000", + "image_size": 20480, + "image_vmaddr": "0x0", + "type": "apple", + "uuid": "4B5A054F-B7A1-3AD0-81E1-513B4DBE2A33", + }, + { + "cpu_subtype": 3, + "cpu_type": 16777223, + "image_addr": "0x1400c000", + "image_size": 266240, + "image_vmaddr": "0x0", + "type": "apple", + "uuid": "766DFB14-72EE-32D2-8961-687D32548F2B", + }, + { + "cpu_subtype": 3, + "cpu_type": 16777223, + "image_addr": "0x1406f000", + "image_size": 913408, + "image_vmaddr": "0x0", + "type": "apple", + "uuid": "BE602DC1-D3A0-3389-B8F4-922C37DEA3DC", + }, + ], + context={ + "device": { + "arch": "x86", + "family": "iPhone", + "freeMemory": 169684992, + "memorySize": 17179869184, + "model": "iPhone9,1", + "simulator": True, + "storageSize": 249695305728, + "type": "device", + "usableMemory": 14919622656, + }, + "os": { + "build": "16C67", + "bundleID": "com.rokkincat.SentryExample", + "bundleVersion": "2", + "kernel_version": "Darwin Kernel Version 16.3.0: Thu Nov 17 20:23:58 PST 2016; root:xnu-3789.31.2~1/RELEASE_X86_64", + "name": "iOS", + "type": "os", + "version": "10.2", + }, + }, + ) + binary_images = acr.get_binary_images_apple_string() + assert ( + binary_images + == "Binary Images:\n\ +0x1400c000 - 0x1404cfff {0} x86 <766dfb1472ee32d28961687d32548f2b> {0}\n\ +0x1406f000 - 0x1414dfff {0} x86 {0}\n\ +0x141c5000 - 0x141c9fff {0} x86 <4b5a054fb7a13ad081e1513b4dbe2a33> {0}".format( + NATIVE_UNKNOWN_STRING + ) + ) + + def test__convert_debug_meta_to_binary_image_row(): acr = AppleCrashReport( context={ @@ -469,7 +536,7 @@ def test__convert_debug_meta_to_binary_image_row(): "image_addr": "0xd69a000", "image_size": 495616, "image_vmaddr": "0x0", - "name": "/Users/haza/Library/Developer/CoreSimulator/Devices/DDB32F4C-97CF-4E2B-BD10-EB940553F223/data/Containers/Bundle/Application/8F8140DF-B25B-4088-B5FB-57F474A49CD6/SwiftExample.app/Frameworks/SentrySwift.framework/SentrySwift", + "code_file": "/Users/haza/Library/Developer/CoreSimulator/Devices/DDB32F4C-97CF-4E2B-BD10-EB940553F223/data/Containers/Bundle/Application/8F8140DF-B25B-4088-B5FB-57F474A49CD6/SwiftExample.app/Frameworks/SentrySwift.framework/SentrySwift", "type": "apple", "uuid": "B427AE1D-BF36-3B50-936F-D78A7D1C8340", } From f2a69224816a79d079f9f518021cc2ddf75b7cae Mon Sep 17 00:00:00 2001 From: William Mak Date: Fri, 22 Nov 2019 16:10:44 -0500 Subject: [PATCH 4/4] ref: remove the other deprecated fields --- .../lang/native/test_applecrashreport.py | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/tests/sentry/lang/native/test_applecrashreport.py b/tests/sentry/lang/native/test_applecrashreport.py index b2e1665d99a4..c0686e73e0c1 100644 --- a/tests/sentry/lang/native/test_applecrashreport.py +++ b/tests/sentry/lang/native/test_applecrashreport.py @@ -375,33 +375,27 @@ def test_get_binary_images_apple_string(): acr = AppleCrashReport( debug_images=[ { - "cpu_subtype": 3, - "cpu_type": 16777223, "image_addr": "0x141c5000", "image_size": 20480, "image_vmaddr": "0x0", "code_file": "/Users/haza/Library/Developer/CoreSimulator/Devices/DDB32F4C-97CF-4E2B-BD10-EB940553F223/data/Containers/Bundle/Application/8C286977-D498-44FF-B7BE-42BFE3DE38BD/SwiftExample.app/Frameworks/libswiftContacts.dylib", - "type": "apple", + "type": "native", "uuid": "4B5A054F-B7A1-3AD0-81E1-513B4DBE2A33", }, { - "cpu_subtype": 3, - "cpu_type": 16777223, "image_addr": "0x1400c000", "image_size": 266240, "image_vmaddr": "0x0", "code_file": "/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/System/Library/PrivateFrameworks/ContentIndex.framework/ContentIndex", - "type": "apple", + "type": "native", "uuid": "766DFB14-72EE-32D2-8961-687D32548F2B", }, { - "cpu_subtype": 3, - "cpu_type": 16777223, "image_addr": "0x1406f000", "image_size": 913408, "image_vmaddr": "0x0", "code_file": "/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator.sdk/System/Library/PrivateFrameworks/CorePDF.framework/CorePDF", - "type": "apple", + "type": "native", "uuid": "BE602DC1-D3A0-3389-B8F4-922C37DEA3DC", }, ], @@ -442,30 +436,24 @@ def test_binary_images_without_code_file(): acr = AppleCrashReport( debug_images=[ { - "cpu_subtype": 3, - "cpu_type": 16777223, "image_addr": "0x141c5000", "image_size": 20480, "image_vmaddr": "0x0", - "type": "apple", + "type": "native", "uuid": "4B5A054F-B7A1-3AD0-81E1-513B4DBE2A33", }, { - "cpu_subtype": 3, - "cpu_type": 16777223, "image_addr": "0x1400c000", "image_size": 266240, "image_vmaddr": "0x0", - "type": "apple", + "type": "native", "uuid": "766DFB14-72EE-32D2-8961-687D32548F2B", }, { - "cpu_subtype": 3, - "cpu_type": 16777223, "image_addr": "0x1406f000", "image_size": 913408, "image_vmaddr": "0x0", - "type": "apple", + "type": "native", "uuid": "BE602DC1-D3A0-3389-B8F4-922C37DEA3DC", }, ],