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.

Sign up for GitHub

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

Jump to bottom

Remove ARC-related Obj-C Code from decompilation #52

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Remove ARC-related Obj-C Code from decompilation #52

wants to merge 5 commits into from

Conversation

0cyn
Copy link
Member

@0cyn 0cyn commented Jan 24, 2023

No description provided.

@0cyn 0cyn self-assigned this Jan 24, 2023
0cyn
0cyn commented Jan 24, 2023
View reviewed changes
MessageHandler.cpp Outdated Show resolved Hide resolved
MessageHandler.cpp Outdated Show resolved Hide resolved
MessageHandler.cpp Outdated Show resolved Hide resolved
MessageHandler.cpp Outdated Show resolved Hide resolved
@0cyn 0cyn marked this pull request as ready for review January 24, 2023 21:16
@0cyn 0cyn requested a review from jonpalmisc January 24, 2023 21:16
0cyn
0cyn commented Jan 24, 2023
View reviewed changes
MessageHandler.cpp Outdated Show resolved Hide resolved
@jonpalmisc
Copy link

Before I do a review, I do believe that this should be placed behind a setting so that it can be enabled/disabled as desired.

@plafosse plafosse self-assigned this Jan 25, 2023
@plafosse plafosse self-requested a review January 25, 2023 19:20
jonpalmisc
jonpalmisc suggested changes Apr 11, 2023
View reviewed changes
Copy link

@jonpalmisc jonpalmisc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursory glance did not reveal any catastrophic mistakes, just minor fixes needed. Not on a computer where I can test this functionally at the moment, however.

Question: What happens when the setting is toggled after initial analysis? My intuition would tell me that functions that end up getting re-analyzed would have the ARC code removed, but old ones wouldn't? Not sure what the "correct" behavior here should be though.

Plugin.cpp Outdated Show resolved Hide resolved
Plugin.cpp Outdated Show resolved Hide resolved
@0cyn
Copy link
Member Author

0cyn commented Jul 18, 2023

This is functional but appears to be partially broken on newer arm64 format binaries which stub code out heavily.

Merge blocked pending addressing Vector35/binaryninja-api#5563

@0cyn 0cyn mentioned this pull request Jul 18, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonpalmisc jonpalmisc jonpalmisc requested changes

@plafosse plafosse Awaiting requested review from plafosse

Assignees

@plafosse plafosse

@0cyn 0cyn

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@0cyn @jonpalmisc @plafosse