ios: Fix crash in ObjcExceptionPreprocessor. [crashpad/crashpad : main]
Justin Cohen (Gerrit)
Attention is currently required from: Robert Sesek.
Patch set 5:Commit-Queue +1
1 comment:
Patchset:
ptal!
To view, visit change 4317110. To unsubscribe, or for help writing mail filters, visit settings.
Ben Hamilton (Gerrit)
Attention is currently required from: Justin Cohen, Robert Sesek.
4 comments:
File client/ios_handler/exception_processor.mm:
Patch Set #5, Line 141:
if ([exception respondsToSelector:@selector(name)]) {You don't need the respondsToSelector: tests now that you've verified at runtime that `exception` is an `NSException*`.
Patch Set #5, Line 228:
NSArray<NSNumber*>* address_array = [exception callStackReturnAddresses];You can only do this if the -isKindOfClass: test passes.
Patch Set #5, Line 273:
std::atomic<NSException*> last_exception_ = nil;Should we make this a `void *` so we know the only safe operation is pointer comparison?
if ([exception respondsToSelector:@selector(name)] &&
[exception respondsToSelector:@selector(reason)]) {
NSString* value = [NSString`if ([exception isKindOfClass:[NSException class]) {` ?
To view, visit change 4317110. To unsubscribe, or for help writing mail filters, visit settings.
Justin Cohen (Gerrit)
Attention is currently required from: Ben Hamilton, Robert Sesek.
4 comments:
File client/ios_handler/exception_processor.mm:
Patch Set #5, Line 141:
if ([exception respondsToSelector:@selector(name)]) {
You don't need the respondsToSelector: tests now that you've verified at runtime that `exception` is […]
Done
Patch Set #5, Line 228:
NSArray<NSNumber*>* address_array = [exception callStackReturnAddresses];You can only do this if the -isKindOfClass: test passes.
Done
Patch Set #5, Line 273:
std::atomic<NSException*> last_exception_ = nil;Should we make this a `void *` so we know the only safe operation is pointer comparison?
Good idea.
if ([exception respondsToSelector:@selector(name)] &&
[exception respondsToSelector:@selector(reason)]) {
NSString* value = [NSString`if ([exception isKindOfClass:[NSException class]) {` ?
Done
To view, visit change 4317110. To unsubscribe, or for help writing mail filters, visit settings.
Ben Hamilton (Gerrit)
Attention is currently required from: Justin Cohen, Robert Sesek.
Patch set 7:Code-Review +1
1 comment:
File client/ios_handler/exception_processor.mm:
if ([exception isKindOfClass:[NSException class]]) {
NSString* value = [NSString
stringWithFormat:@"%@ reason %@", [exception name], [exception reason]];
key->Set(base::SysNSStringToUTF8(value));
}Worth adding an `else` block that just appends `[exception description]`?
To view, visit change 4317110. To unsubscribe, or for help writing mail filters, visit settings.
Justin Cohen (Gerrit)
Attention is currently required from: Robert Sesek.
1 comment:
File client/ios_handler/exception_processor.mm:
if ([exception isKindOfClass:[NSException class]]) {
NSString* value = [NSString
stringWithFormat:@"%@ reason %@", [exception name], [exception reason]];
key->Set(base::SysNSStringToUTF8(value));
}Worth adding an `else` block that just appends `[exception description]`?
Done
To view, visit change 4317110. To unsubscribe, or for help writing mail filters, visit settings.
Robert Sesek (Gerrit)
Attention is currently required from: Justin Cohen.
Patch set 11:Code-Review +1
1 comment:
File client/ios_handler/exception_processor.mm:
Patch Set #11, Line 2:
/as /revert
To view, visit change 4317110. To unsubscribe, or for help writing mail filters, visit settings.
Justin Cohen (Gerrit)
Patch set 12:Commit-Queue +2
1 comment:
File client/ios_handler/exception_processor.mm:
Patch Set #11, Line 2:
/as /revert
Done
To view, visit change 4317110. To unsubscribe, or for help writing mail filters, visit settings.
Crashpad LUCI CQ (Gerrit)
Crashpad LUCI CQ submitted this change.
11 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: client/ios_handler/exception_processor.mm
Insertions: 1, Deletions: 1.
@@ -1,5 +1,5 @@
// Copyright 2020 The Crashpad Authors
-/as /
+//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
```
ios: Fix crash in ObjcExceptionPreprocessor.
ObjcExceptionPreprocessor is a 'reasonable effort' attempt to catch an
NSException minidump at time the exception is thrown as opposed to when the application terminates due to the exception. If multiple
exceptions are thrown at the same time, Crashpad should correctly
report the final uncaught exception, but the minidump may not
represent the full `caught-at-thrown` minidump.
- Don't assume ObjcExceptionPreprocessor throws an NSException.
- Don't retain/release the exception. Instead of calling isEqual,
just use a simple pointer comparison.
- Make last_exception atomic.
Bug: crashpad: 445, 446
Change-Id: I9f2f2041e96aa9818c63937025e507487ae9d03d
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/4317110
Reviewed-by: Ben Hamilton <benha...@google.com>
Commit-Queue: Justin Cohen <justi...@chromium.org>
Reviewed-by: Robert Sesek <rse...@chromium.org>
---
M client/ios_handler/exception_processor.mm
M test/ios/crash_type_xctest.mm
M test/ios/host/cptest_application_delegate.mm
M test/ios/host/cptest_shared_object.h
4 files changed, 85 insertions(+), 16 deletions(-)