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

Theos should raise an exception when hooking a non-implemented selector #49

Open
codyd51 opened this issue Jul 4, 2019 · 4 comments
Open

Comments

@codyd51
Copy link

codyd51 commented Jul 4, 2019

It's easy to make a mistake with theos by placing a method-hook under the wrong %hook class, or adding a typo to a selector name. In this case, the hooks will silently fail, and the methods will not be swizzled, leading to confusion of whether the target methods are being invoked at all.

Instead, theos should loudly fail when I provide an 'invalid hook'. This would catch this bug much earlier in development and reduce headaches.

@leptos-null
Copy link
Member

Logging for this was removed to avoid initializing the class.

@uroboro uroboro transferred this issue from theos/theos Dec 10, 2019
@uroboro
Copy link
Member

uroboro commented Dec 25, 2019

If you mean at runtime, it’s coverec by the previous comment.
If you mean at compile time this would require some heavy lifting to parse the code, the sdk and figure out if there’s an error somewhere. It would be even more complex if a tweak supports multiple versions where classes or methods change on major or minor updates.

@codyd51
Copy link
Author

codyd51 commented Dec 26, 2019

Of course I agree that doing this statically is a non-starter.

But, it seems totally doable dynamically and a valuable QoL improvement. I understand the justification to want to avoid the hooking platform initializing a class, but this isn't a serious blocker and can be worked around:

Ctrl-f CLS_INITIALIZED

Personally, I struggle to think of a bug caused by a class being initialized too early - its dependency chain of other classes should already have been +loaded, for example. But, if you're sure you want to handle the edge case, either of these might work:

  1. Hooking a class auto-creates an +initialize hook which does the MSHookMessageEx

  2. The code above might be wrapped by if (CLS_GETINFO(objc_getClass("classToHook"), CLS_INITIALIZED)) { ... }

@codyd51
Copy link
Author

codyd51 commented Dec 26, 2019

I now see bugs caused by the behavior mentioned in the commit message. I think both approaches above would avoid it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants