optimize internal generator

[0xilis]

What does this implement/fix? Explain your changes.

I mentioned this on the theos discord but I'm going to restate here:
For cases where we do need to call objective-c runtime apis directly (ex compiling a substrate-less tweak for Simulator), I noticed that for handling the superclass behavior as kirb described, Logos internal generator seems to copy the method list and cycle through until it finds the method name, and uses method_setImplementation and class_addMethod directly.

But to my testing and as the Objective-C runtime documentation states, class_replaceMethod does seem to do so.

To my testing, this always seems to be faster than the internal generator's current implementation and I believe should behave the same way. Even in the area where it's least impactful (the method is already implemented by the class directly), on my x86_64 macbook running macOS 12.6, this is about 4 times faster than it is currently. When testing when a method was implemented by a superclass of the superclass of the class, it was about 10 times faster. (plus results in a very slightly smaller binary, though really only a couple of bytes, but hey still an improvement nonetheless)

Does this close any currently open issues?

Nope!

Any relevant logs, error output, etc?

This doesn't apply to this commit.

Any other comments?

…

Where has this been tested?

Operating System: macOS

Platform: x86_64

[kirb]

This is great, thanks heaps for looking into this. We need to find out if there was some reason it wasn’t implemented the obvious way with class_replaceMethod though. It could be that this logic handles some specific edge cases, or that replaceMethod wasn’t sufficient or didn’t exist in older runtimes.

[L1ghtmann]

Relevant commit for reference 493a4a0

[leptos-null]

Attaching my test.

Makefile

TARGET := macosx:clang:latest:14.0

include $(THEOS)/makefiles/common.mk

TOOL_NAME = HookTest

HookTest_FILES = main.x
HookTest_CFLAGS = -fobjc-arc
HookTest_INSTALL_PATH = /usr/local/bin

include $(THEOS_MAKE_PATH)/tool.mk

test:: HookTest
	$(THEOS_OBJ_DIR)/HookTest

main.x

%config(generator=internal);

#import <objc/NSObject.h>
#import <stdio.h>

@interface TestA : NSObject

+ (int)sampleClass;
- (int)sampleInstance;

@end

@interface TestB : TestA
@end

@interface TestC : TestA
@end

@implementation TestA

+ (int)sampleClass {
	return 'A';
}

- (int)sampleInstance {
	return 'a';
}

@end

@implementation TestB
@end

@implementation TestC
@end

%hook TestB

+ (int)sampleClass {
	return 'B';
}

%end

%hook TestC

- (int)sampleInstance {
	return 'c';
}

%end

#if DEBUG
#	define test_case(truthy, message) ((!(truthy)) ? (void)fprintf(stderr, "Test: `" #truthy "` failed; " message "\n") : (void)fprintf(stderr, "Test: `" #truthy "` passed; " message "\n"))
#else
#	define test_case(truthy, message) ((!(truthy)) ? (void)fprintf(stderr, "Test: `" #truthy "` failed; " message "\n") : (void)0)
#endif

int main() {
	TestA *const aInst = [TestA new];
	TestB *const bInst = [TestB new];
	TestC *const cInst = [TestC new];

	test_case([TestA sampleClass] == 'A', "Base");
	test_case([TestB sampleClass] == 'B', "Hooked B");
	test_case([TestC sampleClass] == 'A', "No hook should call super class");

	test_case([aInst sampleInstance] == 'a', "Base");
	test_case([bInst sampleInstance] == 'a', "No hook should call super class");
	test_case([cInst sampleInstance] == 'c', "Hooked C");

	test_case([TestA sampleClass] != 'Z', "Counter case");

	test_case(0, "Expected fail");
}

All tests passing.

Any additional cases anyone has concerns about?

[leptos-null]

Per Apple documentation, this function is available since iOS 2.0, so availability looks good to me.

[0xilis]

I should note that it is possible to inline method_getTypeEncoding which should save a tiny bit of speed, but that would rely on the struct element being at offset 0x8 (making it much less future proof, breaking it if it ever changes) so I'm not going to do that, it would have barely made an impact anyway and would make the code much more ugly...

[leptos-null]

Thank you for opening theos/orion#31 and thanks to Kabir for the tests on that repo.

The unit test attached here (using same Makefile as above) demonstrates why this method does not work.
Specifically, class_replaceMethod only returns the original implementation when it was implemented on the class specified; i.e. it will return NULL when the method we're swizzling is on a class higher in the inheritance hierarchy.

In conclusion, class_replaceMethod is able to swizzle methods in the runtime in the way we expect, however it does not provide %orig in the way we expect.

main.x

%config(generator=internal);

#import <objc/NSObject.h>
#import <stdio.h>

@interface TestA : NSObject

+ (int)sampleClass;

@end

@interface TestB : TestA
@end

@implementation TestA

+ (int)sampleClass {
	return 'A';
}

@end

@implementation TestB
@end

%hook TestB

+ (int)sampleClass {
    return %orig + 1;
}

%end

#if DEBUG
#	define test_case(truthy, message) ((!(truthy)) ? (void)fprintf(stderr, "Test: `" #truthy "` failed; " message "\n") : (void)fprintf(stderr, "Test: `" #truthy "` passed; " message "\n"))
#else
#	define test_case(truthy, message) ((!(truthy)) ? (void)fprintf(stderr, "Test: `" #truthy "` failed; " message "\n") : (void)0)
#endif

int main() {
	test_case([TestA sampleClass] == 'A', "Base");
	test_case([TestB sampleClass] == 'B', "Hooked B");

	test_case([TestA sampleClass] != 'Z', "Counter case");

	test_case(0, "Expected fail");
}

[leptos-null]

Adding my formal rejection to this PR to avoid an accidental merge.
Will leave this PR open for 1 week for discussion before closing.