Skip to content

Conversation

@bdash
Copy link
Contributor

@bdash bdash commented May 16, 2025

The target of the bind opcode has its symbol applied when the dynamic symbol table is parsed.

This was previously adding a symbol at the fixed-up address with the name of the symbol being bound. This was resulting in dozens of _OBJC_CLASS_$_NSObject, _OBJC_METACLASS_$_NSObject, and __objc_empty_cache symbols being created: one for each objc_class that referenced those symbols.

Fixes #5351.

@bdash bdash force-pushed the macho-no-symbols-for-bindings branch 2 times, most recently from cfb5eda to fe48454 Compare May 22, 2025 23:50
@plafosse plafosse requested a review from 0cyn May 27, 2025 12:32
@plafosse plafosse added this to the Helion milestone May 27, 2025
@bdash bdash force-pushed the macho-no-symbols-for-bindings branch from fe48454 to 0e784a1 Compare May 30, 2025 00:30
@0cyn
Copy link
Member

0cyn commented Jun 4, 2025

I am largely in favor of this landing, however we do lose some symbols in IL because of this. The current code is a long-standing holdover for this reason.

Screenshot 2025-06-04 at 10 34 35 AM

Occasionally it is able to resolve the pointers although that is analysis based. I think a good solution here may be to instead define a '__ptr_'+name symbol at this location? That way we can avoid IL quality degrading while making things more accurate.

@bdash
Copy link
Contributor Author

bdash commented Jun 4, 2025

The current IL is wrong so IMO the change shown in the screenshot is an improvement. The argument to dispatch_once isn't __NSConcreteGlobalBlock, it's a pointer to a static struct whose first member is __NSConcreteGlobalBlock.

@0cyn
Copy link
Member

0cyn commented Jun 4, 2025

I picked a bad example 😄 but your point stands.

Still diffing ILs on several binaries for this in different conditions, but assuming that's the only case of this I can find, this should be able to land. Most other cases (e.g. dyld_stub_binder) seem to resolve in HLIL which is fine.

@0cyn 0cyn force-pushed the macho-no-symbols-for-bindings branch 2 times, most recently from 69c80c9 to c20f296 Compare June 4, 2025 17:08
@0cyn 0cyn closed this Jun 4, 2025
@0cyn 0cyn force-pushed the macho-no-symbols-for-bindings branch from c20f296 to 3e7c1b6 Compare June 4, 2025 20:32
@0cyn
Copy link
Member

0cyn commented Jun 4, 2025

Merged with 3e7c1b6.

I missed the git push --force while rebasing this for a few changes that got pushed to dev, so github is marking it as closed/unmerged, however it has landed.

Available in versions >= dev 5.1.7565

@0cyn 0cyn changed the title [MachO] Don't add symbols when processing bind fixups [Merged] [MachO] Don't add symbols when processing bind fixups Jun 4, 2025
@bdash bdash deleted the macho-no-symbols-for-bindings branch June 5, 2025 22:21
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

Successfully merging this pull request may close these issues.

Macho loader treating symbol bindings wrong

3 participants