Skip to content

Add demangle support using external rustfilt tool#2

Merged
chrysn merged 1 commit intoRIOT-OS:masterfrom
chrysn-pull-requests:demangle
Dec 13, 2024
Merged

Add demangle support using external rustfilt tool#2
chrysn merged 1 commit intoRIOT-OS:masterfrom
chrysn-pull-requests:demangle

Conversation

@chrysn
Copy link
Copy Markdown
Member

@chrysn chrysn commented Dec 2, 2024 •
edited
Loading

This PR adds support for an external demangler: If rustfilt is present, symbol names are shown in a demangled way. This should cover not only Rust but also C++ symbols.

This is moving haukepetersen#12 in here; marked as a draft as I'll have yet to review whether this is still all current.

@chrysn chrysn mentioned this pull request Dec 6, 2024
Closed
@chrysn
Copy link
Copy Markdown
Member Author

chrysn commented Dec 6, 2024

Rebased and re-tested: Symbols that previously looked like _ZN123_$LT$coap_handler_implementations..wkc_implementation..WellKnownCore$LT$H$GT$$u20$as$u20$coap_handler..handler..Handler$GT$14build_response17hdc5d4b38dfaaba18E now look like <coap_handler_implementations::wkc_implementation::WellKnownCore<H> as coap_handler::handler::Handler>::build_response (which is just as long but at least readable).

This does emit a warning about not having been able to run the demangler even when there are no Rust symbols, but then I don't know how to reliably recognize them, it's just a warning, and it's not like there's certainty that there are no surprise Rust modules in RIOT any more ;-)

Please review.

@chrysn chrysn marked this pull request as ready for review December 6, 2024 15:52
@chrysn chrysn changed the title Add demangle support using external rustfix tool Add demangle support using external rustfilt tool Dec 9, 2024
@chrysn chrysn mentioned this pull request Dec 10, 2024
Merged
mguetschow
mguetschow reviewed Dec 13, 2024
View reviewed changes
Copy link
Copy Markdown

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

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

Tested locally (or rather with BUILD_IN_DOCKER=1) with examples/rust-gcoap and can confirm this to work nicely.

All rust symbols are grouped under "unspecified", but that was already the case before so out of scope for this PR, I'd say.

Just a minor suggestion below.

mguetschow
mguetschow approved these changes Dec 13, 2024
View reviewed changes
Copy link
Copy Markdown

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

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

Thanks!

mguetschow
mguetschow previously requested changes Dec 13, 2024
View reviewed changes
Copy link
Copy Markdown

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

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

Oh wait, the commit message should be changed with s/rustfix/rustfilt/

@chrysn
Copy link
Copy Markdown
Member Author

chrysn commented Dec 13, 2024

Thanks for checking, fixed.

@chrysn chrysn dismissed mguetschow’s stale review December 13, 2024 12:51

Applied the commit message suggestion

@chrysn chrysn merged commit 1bdcea7 into RIOT-OS:master Dec 13, 2024
@chrysn chrysn deleted the demangle branch December 13, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mguetschow mguetschow mguetschow approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@chrysn @mguetschow