Skip to content

Commit

Permalink
fix: Missing UIViewController traces with Xcode 16 (#4523)
Browse files Browse the repository at this point in the history
Xcode 16 introduces a new flag ENABLE_DEBUG_DYLIB. If this flag is
enabled, debug builds of app and app extension targets on supported
platforms and SDKs will be built with the main binary code in a separate
“NAME.debug.dylib”. Our swizzling logic for UIViewControllers sometimes
skipped swizzling the UIViewControllers in that debug.dylib binary. This
is fixed now, by swizzling all matching inAppInclude binary images that
the binary image cache returns and not only the first one.
  • Loading branch information
philipphofmann authored Nov 11, 2024
1 parent 4509e9c commit a34f08c
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 53 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
### Fixes

- Keep PropagationContext when cloning scope (#4518)
- UIViewController with Xcode 16 in debug (#4523). The Xcode 16 build setting [ENABLE_DEBUG_DYLIB](https://developer.apple.com/documentation/xcode/build-settings-reference#Enable-Debug-Dylib-Support), which is turned on by default only in debug, could lead to missing UIViewController traces.
- Concurrency crash with Swift 6 (#4512)

## 8.40.1
Expand Down
8 changes: 5 additions & 3 deletions Sources/Sentry/SentryBinaryImageCache.m
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,18 @@ - (NSInteger)indexOfImage:(uint64_t)address
return -1; // Address not found
}

- (nullable NSString *)pathForInAppInclude:(NSString *)inAppInclude
- (NSSet<NSString *> *)imagePathsForInAppInclude:(NSString *)inAppInclude
{
NSMutableSet<NSString *> *imagePaths = [NSMutableSet new];

@synchronized(self) {
for (SentryBinaryImageInfo *info in _cache) {
if ([SentryInAppLogic isImageNameInApp:info.name inAppInclude:inAppInclude]) {
return info.name;
[imagePaths addObject:info.name];
}
}
}
return nil;
return imagePaths;
}

@end
Expand Down
8 changes: 5 additions & 3 deletions Sources/Sentry/SentrySubClassFinder.m
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ - (void)actOnSubclassesOfViewControllerInImage:(NSString *)imageName block:(void
copyClassNamesForImage:[imageName cStringUsingEncoding:NSUTF8StringEncoding]
amount:&count];

SENTRY_LOG_DEBUG(@"Found %u number of classes in image: %@.", count, imageName);

// Storing the actual classes in an NSArray would call initializer of the class, which we
// must avoid as we are on a background thread here and dealing with UIViewControllers,
// which assume they are running on the main thread. Therefore, we store the class name
Expand Down Expand Up @@ -87,9 +89,9 @@ - (void)actOnSubclassesOfViewControllerInImage:(NSString *)imageName block:(void
block(NSClassFromString(className));
}

SENTRY_LOG_DEBUG(
@"The following UIViewControllers will generate automatic transactions: %@",
[classesToSwizzle componentsJoinedByString:@", "]);
SENTRY_LOG_DEBUG(@"The following UIViewControllers for image: %@ will generate "
@"automatic transactions: %@",
imageName, [classesToSwizzle componentsJoinedByString:@", "]);
}];
}];
}
Expand Down
15 changes: 10 additions & 5 deletions Sources/Sentry/SentryUIViewControllerSwizzling.m
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,17 @@ - (instancetype)initWithOptions:(SentryOptions *)options
- (void)start
{
for (NSString *inAppInclude in self.inAppLogic.inAppIncludes) {
NSString *pathToImage = [self.binaryImageCache pathForInAppInclude:inAppInclude];
if (pathToImage != nil) {
[self swizzleUIViewControllersOfImage:pathToImage];
NSSet<NSString *> *imagePathsToInAppInclude =
[self.binaryImageCache imagePathsForInAppInclude:inAppInclude];

if (imagePathsToInAppInclude.count > 0) {
for (NSString *imagePath in imagePathsToInAppInclude) {
[self swizzleUIViewControllersOfImage:imagePath];
}
} else {
SENTRY_LOG_WARN(@"Failed to find the binary image for inAppInclude <%@> and, therefore "
@"can't instrument UIViewControllers in that binary",
SENTRY_LOG_WARN(
@"Failed to find the binary image(s) for inAppInclude <%@> and, therefore "
@"can't instrument UIViewControllers in these binaries.",
inAppInclude);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ NS_ASSUME_NONNULL_BEGIN

- (nullable SentryBinaryImageInfo *)imageByAddress:(const uint64_t)address;

- (nullable NSString *)pathForInAppInclude:(NSString *)inAppInclude;
- (NSSet<NSString *> *)imagePathsForInAppInclude:(NSString *)inAppInclude;

+ (NSString *_Nullable)convertUUID:(const unsigned char *const)value;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase {
cString: class_getImageName(SentryUIViewControllerSwizzlingTests.self)!,
encoding: .utf8)! as NSString
options.add(inAppInclude: imageName.lastPathComponent)

let externalImageName = String(
cString: class_getImageName(ExternalUIViewController.self)!,
encoding: .utf8)! as NSString
options.add(inAppInclude: externalImageName.lastPathComponent)
}

var sut: SentryUIViewControllerSwizzling {
Expand Down Expand Up @@ -167,13 +162,59 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase {
}

func testSwizzlingOfExternalLibs() {
let externalImageName = String(
cString: class_getImageName(ExternalUIViewController.self)!,
encoding: .utf8)! as NSString
fixture.options.add(inAppInclude: externalImageName.lastPathComponent)

let sut = fixture.sut
sut.start()
let controller = ExternalUIViewController()
controller.loadView()
XCTAssertNotNil(SentrySDK.span)
}

func testSwizzleInAppIncludes_WithShortenedInAppInclude() throws {
let imageName = try XCTUnwrap(String(
cString: class_getImageName(ExternalUIViewController.self)!,
encoding: .utf8) as? NSString)

let lastPathComponent = String(imageName.lastPathComponent)
let shortenedLastPathComponent = String(lastPathComponent.prefix(5))

fixture.options.add(inAppInclude: shortenedLastPathComponent)

let sut = fixture.sut
sut.start()
let controller = ExternalUIViewController()
controller.loadView()
XCTAssertNotNil(SentrySDK.span)
}

/// Xcode 16 introduces a new flag ENABLE_DEBUG_DYLIB (https://developer.apple.com/documentation/xcode/build-settings-reference#Enable-Debug-Dylib-Support)
/// If this flag is enabled, debug builds of app and app extension targets on supported platforms and SDKs
/// will be built with the main binary code in a separate “NAME.debug.dylib”.
/// This test adds this debug.dylib and checks if it gets swizzled.
func testSwizzle_DebugDylib_GetsSwizzled() {
let imageName = String(
cString: class_getImageName(SentryUIViewControllerSwizzlingTests.self)!,
encoding: .utf8)! as NSString

let debugDylib = "\(imageName).debug.dylib"

var image = createCrashBinaryImage(0, name: debugDylib)
SentryDependencyContainer.sharedInstance().binaryImageCache.start()
SentryDependencyContainer.sharedInstance().binaryImageCache.binaryImageAdded(&image)

let sut = fixture.sut
sut.start()

let subClassFinderInvocations = fixture.subClassFinder.invocations
let result = subClassFinderInvocations.invocations.filter { $0.imageName == debugDylib }

XCTAssertEqual(1, result.count)
}

func testSwizzle_fromScene_invalidNotification_NoObject() {
let swizzler = fixture.testableSut

Expand Down
71 changes: 35 additions & 36 deletions Tests/SentryTests/SentryBinaryImageCacheTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -122,17 +122,17 @@ class SentryBinaryImageCacheTests: XCTestCase {
sut.binaryImageAdded(&binaryImage)
sut.binaryImageAdded(&binaryImage2)

let path = sut.pathFor(inAppInclude: "Expected Name at 0")
XCTAssertEqual(path, "Expected Name at 0")
let paths = sut.imagePathsFor(inAppInclude: "Expected Name at 0")
XCTAssertEqual(paths.first, "Expected Name at 0")

let path2 = sut.pathFor(inAppInclude: "Expected Name at 1")
XCTAssertEqual(path2, "Expected Name at 1")
let paths2 = sut.imagePathsFor(inAppInclude: "Expected Name at 1")
XCTAssertEqual(paths2.first, "Expected Name at 1")

let path3 = sut.pathFor(inAppInclude: "Expected")
XCTAssertEqual(path3, "Expected Name at 0")
let bothPaths = sut.imagePathsFor(inAppInclude: "Expected")
XCTAssertEqual(bothPaths, ["Expected Name at 0", "Expected Name at 1"])

let didNotFind = sut.pathFor(inAppInclude: "Name at 0")
XCTAssertNil(didNotFind)
let didNotFind = sut.imagePathsFor(inAppInclude: "Name at 0")
XCTAssertTrue(didNotFind.isEmpty)
}

func testBinaryImageWithNULLName_DoesNotAddImage() {
Expand Down Expand Up @@ -193,7 +193,7 @@ class SentryBinaryImageCacheTests: XCTestCase {

for i in 0..<count {
DispatchQueue.global().async {
var binaryImage0 = self.createCrashBinaryImage(UInt(i * 10))
var binaryImage0 = createCrashBinaryImage(UInt(i * 10))
self.sut.binaryImageAdded(&binaryImage0)

self.sut.stop()
Expand All @@ -204,32 +204,31 @@ class SentryBinaryImageCacheTests: XCTestCase {

waitForExpectations(timeout: 1)
}
}

func createCrashBinaryImage(_ address: UInt, vmAddress: UInt64 = 0) -> SentryCrashBinaryImage {
let name = "Expected Name at \(address)"
let nameCString = name.withCString { strdup($0) }

var uuidPointer = UnsafeMutablePointer<UInt8>(nil)
let uuidAsCharArray: [UInt8] = [132, 186, 235, 218, 173, 26, 51, 244, 179, 93, 138, 69, 245, 218, 243, 34]
uuidPointer = UnsafeMutablePointer<UInt8>.allocate(capacity: uuidAsCharArray.count)
uuidPointer?.initialize(from: uuidAsCharArray, count: uuidAsCharArray.count)

let binaryImage = SentryCrashBinaryImage(
address: UInt64(address),
vmAddress: vmAddress,
size: 100,
name: nameCString,
uuid: uuidPointer,
cpuType: 1,
cpuSubType: 1,
majorVersion: 1,
minorVersion: 0,
revisionVersion: 0,
crashInfoMessage: nil,
crashInfoMessage2: nil
)

return binaryImage
}

func createCrashBinaryImage(_ address: UInt, vmAddress: UInt64 = 0, name: String? = nil) -> SentryCrashBinaryImage {
let imageName = name ?? "Expected Name at \(address)"
let nameCString = imageName.withCString { strdup($0) }

var uuidPointer = UnsafeMutablePointer<UInt8>(nil)
let uuidAsCharArray: [UInt8] = [132, 186, 235, 218, 173, 26, 51, 244, 179, 93, 138, 69, 245, 218, 243, 34]
uuidPointer = UnsafeMutablePointer<UInt8>.allocate(capacity: uuidAsCharArray.count)
uuidPointer?.initialize(from: uuidAsCharArray, count: uuidAsCharArray.count)

let binaryImage = SentryCrashBinaryImage(
address: UInt64(address),
vmAddress: vmAddress,
size: 100,
name: nameCString,
uuid: uuidPointer,
cpuType: 1,
cpuSubType: 1,
majorVersion: 1,
minorVersion: 0,
revisionVersion: 0,
crashInfoMessage: nil,
crashInfoMessage2: nil
)

return binaryImage
}

0 comments on commit a34f08c

Please sign in to comment.