-
Couldn't load subscription status.
- Fork 10.6k
Runtime: expose demangle() in Runtime module #84788
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
base: main
Are you sure you want to change the base?
Conversation
|
@swift-ci please smoke test |
6402000 to
899d2e9
Compare
|
@swift-ci please smoke test |
ff6de75 to
41eb337
Compare
|
@swift-ci please smoke test |
6352e39 to
d392c27
Compare
d392c27 to
9db938e
Compare
|
|
||
| // Demangle the given raw name (supports Swift and C++) | ||
| // | ||
| // The demangled string IS a null-terminated c-string. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't appear to be true with the current implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, good catch... this is a behavior change actually hm...
_swift_backtrace_demangle used to null-terminate, and the only user of that was in SymbolicatedBacktrace demangleRawName which now delegates directly to demangle() -> String but only in Swift 6.3...
Null terminating in the new one isn't so great since they're aimed to be used with spans which rely on the explicit count rather than strlen-ing the result hm.
I'll think how to handle this in the Backtrace lib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we return a NUL terminated string from the C interface, but provide a span which doesn't cover the terminator? Slightly wasteful, but convenient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah maybe that way around would be easier
32382c5 to
5626571
Compare
|
Thanks for the review! Adjusted things and fixed up the \0 story a bit, need to determine if we keep the old backtrace entrypoint at all. I also added tests for the C++ mangling. |
7f91a50 to
b469d3e
Compare
|
@swift-ci please smoke test |
We should expose the demangle functionality; It's been widely used by calling into internal _swift_demangle and instead we should offer a real API. It's also already used in the Runtime module already when forming backtraces.
Previous discussions happened between 2019 and 2024, and just never materialized in a complete implementation and proposal.
Right now, even more tools are in need of this API, as we are building continious profiling solutions etc, so it is a good time to revisit this topic.
This PR is roughly based off @Azoy's
https://github.com/swiftlang/swift/pull/25314/files#diff-fd379a721cc9a1c9ef6486eae713e945da842b42170d4d069029a57334371eba from 2019, however it brings it over to the new Runtime module which is a great place to put this functionality - even Backtrace had to recently reinvent calling the demangling infra in this module.
Pending SE review, proposal here.
cc @Azoy @al45tair