Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR-9188] +[NSBundle bundleForClass:] swizzle crashes on nil argument #51679

Closed
lilyball mannequin opened this issue Nov 5, 2018 · 9 comments
Closed

[SR-9188] +[NSBundle bundleForClass:] swizzle crashes on nil argument #51679

lilyball mannequin opened this issue Nov 5, 2018 · 9 comments

Comments

@lilyball
Copy link
Mannequin

@lilyball lilyball mannequin commented Nov 5, 2018

Previous ID SR-9188
Radar rdar://problem/45849924
Original Reporter @lilyball
Type Bug
Status Resolved
Resolution Done
Environment

Xcode 10.1 (10B61)
Apple Swift version 4.2.1 (swiftlang-1000.11.42 clang-1000.11.45.1)
Target: x86_64-apple-darwin18.2.0

Additional Detail from JIRA
Votes 1
Component/s Standard Library
Labels Bug, Runtime
Assignee @mikeash
Priority Medium

md5: b3c7aea07f0f18ab24ef0c093ede11d2

Issue Description:

The Swift stdlib is swizzling +[NSBundle bundleForClass:] in order to handle Swift classes properly. Unfortunately it crashes if the passed class is nil. The API itself is documented as taking a nonnull parameter, but the actual observed behavior at runtime of [NSBundle bundleForClass:nil] without Swift is to return the same thing as [NSBundle mainBundle].

Unfortunately, this crash is a showstopper for us for using Xcode 10.1 because a closed-source dependency of ours is apparently calling [NSBundle bundleForClass:nil].

@lilyball
Copy link
Mannequin Author

@lilyball lilyball mannequin commented Nov 5, 2018

The crashing method in question is patchedBundleForClass().

@lilyball
Copy link
Mannequin Author

@lilyball lilyball mannequin commented Nov 5, 2018

In general, any swizzles performed by Swift should be written as defensively as possible, including not making any assumptions that the nullability annotations can be relied upon. The existing framework methods frequently actually handle null objects passed to nonnull parameters because these APIs were written long before nullability annotations were a thing.

@belkadan
Copy link
Contributor

@belkadan belkadan commented Nov 6, 2018

Oops. Thanks for reporting. cc @mikeash

@swift-ci create

@mikeash
Copy link
Contributor

@mikeash mikeash commented Nov 6, 2018

Oops indeed. My bad.

@belkadan
Copy link
Contributor

@belkadan belkadan commented Nov 6, 2018

Let's not encode any particular behavior for Nil; we can just check for it when deciding whether to do Special Swift Things.

@belkadan
Copy link
Contributor

@belkadan belkadan commented Nov 6, 2018

(You can see what era I learned Objective-C in from my use of Nil.)

@lilyball
Copy link
Mannequin Author

@lilyball lilyball mannequin commented Nov 6, 2018

Agreed. I think the proper solution is just replacing

if (getImageNameFromSwiftClass(objcClass, &imageName)) {

with

if (objcClass != Nil && getImageNameFromSwiftClass(objcClass, &imageName)) {

@mikeash
Copy link
Contributor

@mikeash mikeash commented Nov 6, 2018

I think the check should go in getImageNameFromSwiftClass instead. This will also handle the override of class_getImageName, which also fails gracefully on Nil.

@mikeash
Copy link
Contributor

@mikeash mikeash commented Nov 15, 2018

For your viewing pleasure: #20608

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants