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

Rewrite Of C++ Demanglers #69

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

brightprogrammer
Copy link
Contributor

@brightprogrammer brightprogrammer commented Jan 3, 2025

Remove old GPL code from libiberty and completely rewrite both GNU v2 and GNU v3 demanglers with LGPL license.

Work plan :

Test plan :

  • Already added in Add More GNU v2/v3 Tests #68
  • Write a fuzzer (in less than 100 lines) as implemented in GCC?
  • Build old GCC version (maybe using Qemu) and then compile and get [mangled, original] name pairs for testing.

- Common types like int, signed char, etc... are being demangled in
function params now
- Support for custom types in function param list needs to be added.
- Support for repeated parameter types.
Now 180 tests pass out of all. Two tests seem to be wrong at the moment,
but will take a look later.

TODO
- Sequence of identical types
- Templates
- Function params that have qualifier list in their type
src/cp/vec.h Outdated Show resolved Hide resolved
src/cp/param.h Outdated
Comment on lines 60 to 65
((p) ? (dem_string_free ((p)->name), \
dem_string_free ((p)->suffix), \
dem_string_free ((p)->prefix), \
memset ((p), 0, sizeof (Param)), \
(p)) : \
NULL)
Copy link
Member

Choose a reason for hiding this comment

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

maybe just use a do {} while(0) with an if inside ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like them having as expressions so I can do an if(!expr) {error} on them to debug sometimes.

src/cp/param.h Outdated
Comment on lines 45 to 49
((p) ? (((p)->name = dem_string_new()), \
((p)->suffix = dem_string_new()), \
((p)->prefix = dem_string_new()), \
(p)) : \
NULL)
Copy link
Member

Choose a reason for hiding this comment

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

same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can convert these after the rewrite is complete, it won't hurt to change them and the code will be going into static mode.

src/cp/param.h Outdated
Comment on lines 30 to 34
((param_init (p_dst) && (p_src)) ? (dem_string_concat ((p_dst)->name, (p_src)->name), \
dem_string_concat ((p_dst)->suffix, (p_src)->suffix), \
dem_string_concat ((p_dst)->prefix, (p_src)->prefix), \
(p_dst)) : \
NULL)
Copy link
Member

@wargio wargio Jan 14, 2025

Choose a reason for hiding this comment

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

if the purpose is to call this, then maybe just define a function that does this directly in the demangler ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I'll do that. I used this init then concat pattern some other places as well. It'll fix those as well...

Comment on lines +8 to +13
#define nearest_power_of_two(v) \
(((v)--), \
((v) |= (v) >> 1), \
((v) |= (v) >> 2), \
((v) |= (v) >> 4), \
((v) |= (v) >> 8), \
Copy link
Member

Choose a reason for hiding this comment

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

instead of reinventing the wheel, why not copying the code from rizin side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, sorry about that 😅 I have a habit of doing that. The plus point is that src/cp will be a completely independent module on it's own if we just provide a DemString to it. Maybe that's good...?

Copy link
Member

Choose a reason for hiding this comment

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

just define one like DemString (which is an extract of RzStrBuf) which any other demangler can use.

@brightprogrammer
Copy link
Contributor Author

We do need to find a way to test the GNU v2 code more like we do for GNU v3. There are very few examples. Also due to lack of a precise grammar, things like special naming conventions are hard for me to cover. Any suggestions?

@brightprogrammer brightprogrammer marked this pull request as ready for review January 17, 2025 10:34
@brightprogrammer
Copy link
Contributor Author

There are some function parameter types for which tests don't exist. I haven't added them for now, because I won't have any way to test my implementation.

@@ -11,6 +11,10 @@ Files: .clang-format
Copyright: 2020 RizinOrg <[email protected]>
License: LGPL-3.0-only

Files: src/cp/.clang-format
Copy link
Member

Choose a reason for hiding this comment

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

Maybe move this out of cp and make it global in the root dir?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doing that in a separate PR might be a good idea though. If introduced right now, it might make the diff noisy.

Copy link
Member

Choose a reason for hiding this comment

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

yes you can submit this in another PR where you patch all the diffs.

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.

2 participants