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

Add demangling as plugins and libdemangle as external dependency #1656

Merged
merged 18 commits into from
Oct 1, 2021
Merged

Add demangling as plugins and libdemangle as external dependency #1656

merged 18 commits into from
Oct 1, 2021

Conversation

wargio
Copy link
Member

@wargio wargio commented Sep 11, 2021 •
edited by XVilka
Loading

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the documentation and the rizin book with the relevant information (if needed)

Detailed description

Now the demanglers are all handled as external libs (https://github.com/rizinorg/rz-libdemangle).

Also https://github.com/rizinorg/rz-libswift is now a plugin

Closes #1600

@wargio wargio added this to the 0.4.0 milestone Sep 12, 2021
@github-actions github-actions bot added the RzBin label Sep 12, 2021
XVilka
XVilka requested changes Sep 13, 2021
View reviewed changes
Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

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

Looks much better and cleaner this way 👍 🥇

librz/bin/bin.c Show resolved Hide resolved
librz/bin/bin.c Outdated Show resolved Hide resolved
librz/bin/bin.c Show resolved Hide resolved
@wargio wargio requested a review from XVilka September 13, 2021 10:24
@wargio wargio force-pushed the libdemangler-asan branch 2 times, most recently from ebfce37 to 2c03342 Compare September 17, 2021 10:14
@XVilka
Copy link
Member

XVilka commented Sep 18, 2021

@wargio please rebase again to fix the Windows builds.

@XVilka XVilka mentioned this pull request Sep 23, 2021
Closed
3 tasks
ret2libc
ret2libc reviewed Sep 23, 2021
View reviewed changes
librz/bin/bin.c Show resolved Hide resolved
librz/bin/bin_language.c Show resolved Hide resolved
librz/core/cmd_descs/cmd_info.yaml Show resolved Hide resolved
librz/core/cmd_info.c Show resolved Hide resolved
meson_options.txt Show resolved Hide resolved
ret2libc
ret2libc requested changes Sep 27, 2021
View reviewed changes
Copy link
Member

@ret2libc ret2libc left a comment

Choose a reason for hiding this comment

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

I think most things look good, apart from the following:
Most librz/main binaries should not have to care about rz_demanglaer_plugin_init/fini IMHO. Maybe only rz-bin needs it.

  • TODO: create issue to create new RZ_ARG_TYPE_LANG with autocompletion for iD

librz/main/rizin.c Outdated Show resolved Hide resolved
ret2libc
ret2libc requested changes Sep 27, 2021
View reviewed changes
librz/demangler/demangler.c Outdated Show resolved Hide resolved
@XVilka XVilka added mangling refactor Refactoring requests labels Sep 30, 2021
@wargio
Copy link
Member Author

wargio commented Sep 30, 2021 •
edited
Loading

requires a version bump once rizinorg/rz-libdemangle#10 is merged
edit: merged.

ret2libc
ret2libc reviewed Oct 1, 2021
View reviewed changes
Copy link
Member

@ret2libc ret2libc left a comment

Choose a reason for hiding this comment

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

LGTM

librz/demangler/demangler.c Outdated Show resolved Hide resolved
librz/include/rz_demangler.h Outdated Show resolved Hide resolved
librz/main/rz-gg.c Outdated Show resolved Hide resolved
librz/main/rz-find.c Outdated Show resolved Hide resolved
librz/main/rz-diff.c Outdated Show resolved Hide resolved
librz/main/rz-sign.c Outdated Show resolved Hide resolved
@wargio wargio enabled auto-merge (squash) October 1, 2021 10:19
@wargio wargio requested a review from ret2libc October 1, 2021 10:20
@wargio wargio merged commit e224ee7 into dev Oct 1, 2021
@wargio wargio deleted the libdemangler-asan branch October 1, 2021 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XVilka XVilka XVilka approved these changes

@ret2libc ret2libc ret2libc approved these changes

@kazarmy kazarmy Awaiting requested review from kazarmy kazarmy is a code owner

@thestr4ng3r thestr4ng3r Awaiting requested review from thestr4ng3r thestr4ng3r is a code owner

Assignees

@ret2libc ret2libc

Labels
high-priority mangling merge-when-green refactor Refactoring requests RzBin
Projects
None yet
Milestone
0.4.0
Development

Successfully merging this pull request may close these issues.

Extract demangling code into the separate library
3 participants
@wargio @XVilka @ret2libc