-
Notifications
You must be signed in to change notification settings - Fork 346
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
Rust demangle #112
base: main
Are you sure you want to change the base?
Rust demangle #112
Conversation
Nice! I asked about demangling on Rust Internals: https://internals.rust-lang.org/t/symbol-mangling-of-rust-vs-c/7222 I got a reply that said that Rust symbols always end in a 16 byte hash. So perhaps we could detect Rust symbols by simply looking for the string "17h" 16 byte before the end of the string? Want to see if that works as intended? I'd really like to do this in a way that doesn't require Rust-specific options if possible. This would be more user-friendly, and also would gracefully support mixed C++/Rust binaries. |
I'll try to that out. It is bit hacky, but I can see the benefit of avoiding Rust specific modes. What would be the desired behavior if we detect Rust symbol and we don't have rustc-demangle compiled in? Try to demangle with C++ demangler or use the mangled symbol as-is? Btw, do we want to have those hashes in demangled symbols? Right now I have opted to not have them, but rustc-demangle can do either way. I think (but honestly I'm not sure) the hashes are there to make symbols unique in situations where we have multiple versions of a library linked into one binary, so I guess leaving the hashes out leads to different results in such cases. |
Good question. I'd be inclined to say use C++ demangling. Users can always get raw symbols with What do you think as a Rust user? |
I think using C++ demangling sounds pretty reasonable, maybe emit a warning to stderr also? |
Sounds good, as long as it's just a single warning per invocation. |
Just checking in, I think this is in your court? I wanted to make sure you're not waiting on me for anything. |
Rust is switching to a new symbol mangling scheme: https://github.com/rust-lang/rfcs/blob/master/text/2603-symbol-name-mangling-v2.md It has been implemented but is not turned on by default yet. In the new scheme, all Rust symbols start with I saw a comment that they also wrote a complete C implementation of the new scheme, but I can't find a link to it anywhere. |
Cool, if/when this is available as a C library I'd be happy to integrate it. |
@msizanoen1's link is correct, although I wish I got pinged on this issue (gist links aren't enough to do that). I'm in the process of finally upstreaming the whole thing to GCC (which sadly holds the primary copy of The current GCC/binutils/gdb implementation already avoids demangling Rust legacy symbols via the C++ demangler, but instead it handles them more like |
I added the upstreaming effort status to rust-lang/rust#60705, and I'll post updates as comments on there, so you can subscribe to it if you want to track my progress. |
Resolves #110
Not quite yet ready for merge, but posting it early for feedback
CMake build scripts do not work right, for some reason I get this error if the rust lib hasn't been built
As a workaround building the library manually resolves the error. This is bit confusing to me as the documentation for
add_custom_command
says "In makefile terms this creates a new target..", so the target should be there? I would appreciate any help/hints here.Not sure if rustc-demangle should also be built in debug mode when bloaty is built in debug mode. Now for simplicity's sake I'm building rustc-demangle always in release mode.
Need to add tests for this new feature.
Now I've only added Rust demangling to src/elf.cc, it should be also added similarly to src/macho.cc. I don't have a Mac at hand though, so need to tread bit more carefully there.
Currently the submodule is pointing to my fork of rustc-demangle, but that can be changed to refer upstream once rust-lang/rustc-demangle#14 is merged
Should document this feature in README.md.
Does Travis need some new configuration for testing also this feature in CI?