theos / logos

Preprocessor that simplifies Objective-C hooking.
https://theos.dev/docs/logos
Other
205 stars 34 forks source link

%subclass with protocols crashes in the constructor #9

Open JonasGessner opened 9 years ago

JonasGessner commented 9 years ago

In CCLoader I have %subclass that implements two protocols, a custom one and one defined in SpringBoardUIServices. That subclass (sometimes) crashes in the constructor function. I'm using the latest theos. Here's the crash log, and the %subclass is also in that project: https://github.com/JonasGessner/CCLoader/issues/12

JonasGessner commented 9 years ago

I got the precompiled .mm version of the .xm source file, and in the constructor the two protocols are added to the class like this:

class_addProtocol(_logos_class$_ungrouped$CCSectionViewController, objc_getProtocol("CCSectionDelegate"));
class_addProtocol(_logos_class$_ungrouped$CCSectionViewController, objc_getProtocol("_SBUIWidgetHost"));

I took a look at the output of the two objc_getProtocol calls:

Protocol *widgetHost = objc_getProtocol("_SBUIWidgetHost");
Protocol *sectionDelegate = objc_getProtocol("CCSectionDelegate");

NSLog(@"Protocols: %@ %@", widgetHost, sectionDelegate);

NSLog(@"AAAAAAAAAA 1");
class_addProtocol(_logos_class$_ungrouped$CCSectionViewController, sectionDelegate);
NSLog(@"AAAAAAAAAA 2");
class_addProtocol(_logos_class$_ungrouped$CCSectionViewController, widgetHost);
NSLog(@"AAAAAAAAAA 3");

Output:

Protocols: <Protocol: 0x1742bf860> (null)
AAAAAAAAAA 1
AAAAAAAAAA 2
->SpringBoard crashes

The CCSectionDelegate protocol is nil, because it is not implemented by a class (only the dynamic subclass) and because it is never referenced by a @protocol() call (See: https://developer.apple.com/legacy/library/documentation/Cocoa/Conceptual/ObjectiveC/Chapters/ocProtocols.html -> Protocol Objects).

So changing the code to this:

Protocol *widgetHost = @protocol(_SBUIWidgetHost);
Protocol *sectionDelegate = @protocol(CCSectionDelegate);

NSLog(@"Protocols: %@ %@", widgetHost, sectionDelegate);

NSLog(@"AAAAAAAAAA 1");
class_addProtocol(_logos_class$_ungrouped$CCSectionViewController, sectionDelegate);
NSLog(@"AAAAAAAAAA 2");
class_addProtocol(_logos_class$_ungrouped$CCSectionViewController, widgetHost);
NSLog(@"AAAAAAAAAA 3");

Outputs this:

Protocols: <Protocol: 0x1702a3de0> <Protocol: 0x1702b5600>
AAAAAAAAAA 1
AAAAAAAAAA 2
AAAAAAAAAA 3
->Everything is fine

So to get rid of this issue, the protocols have to be referred to with @protocol() rather than objc_getProtocol() to make sure that the compiler creates a protocol object for custom protocols.

Changes that have to be made are: These lines: https://github.com/DHowett/theos/blob/91f8ed9f1adfb8d01f1b73c1bec5600e245a26b8/bin/lib/Logos/Generator/MobileSubstrate/Subclass.pm#L26 and https://github.com/DHowett/theos/blob/91f8ed9f1adfb8d01f1b73c1bec5600e245a26b8/bin/lib/Logos/Generator/internal/Subclass.pm#L26 have to be changed to:

$return .= "class_addProtocol(".$self->variable($class).", \@protocol($_)); ";

Tested and I can confirm that it works. I'll fork the repo and open a pull request.

rpetrich commented 8 years ago

I'm not sure what the right approach is here. Using @protocol will reference the version of the protocol that is declared in the headers, rather than the runtime version which can be different—especially when one considers the case of protocols that vary between iOS versions. On the other hand, it's very surprising for logos not to do the right thing when one has declared a custom protocol. Certainly it shouldn't crash if the protocol could not be found. A more modest fix would be to check the return of objc_getProtocol for NULL before registering.

JonasGessner commented 8 years ago

Yeah. Checking if the protocol returned from objc_getProtocol is nil and falling back to @protocol would probably be the best solution.