Re: [PATCH] libiberty rust-demangle, ignore .suffix
Apologies for the delay, the email fell through the cracks somehow. The updated patch looks like it would work alright, only needs a couple tests, e.g.: https://github.com/rust-lang/rustc-demangle/blob/2811a1ad6f7c8bead2ef3671e4fdc10de1553e96/src/lib.rs#L413-L422 https://github.com/rust-lang/rustc-demangle/blob/2811a1ad6f7c8bead2ef3671e4fdc10de1553e96/src/v0.rs#L1442-L1444 Thanks, - Eddy B. On Tue, Dec 7, 2021, at 21:16, Mark Wielaard wrote: > Hi Eddy, > > On Fri, 2021-12-03 at 01:14 +0200, Eduard-Mihai Burtescu wrote: >> On Fri, Dec 3, 2021, at 00:07, Mark Wielaard wrote: >> > On Thu, Dec 02, 2021 at 07:35:17PM +0200, Eduard-Mihai Burtescu >> > wrote: >> > > That also means that for consistency, suffixes like these should >> > > be >> > > handled uniformly for both v0 and legacy (as rustc-demangle >> > > does), >> > > since LLVM doesn't distinguish. >> > >> > The problem with the legacy mangling is that dot '.' is a valid >> > character. That is why the patch only handles the v0 mangling case >> > (where dot '.' isn't valid). >> >> Thought so, that's an annoying complication - however, see later down >> why that's still not a blocker to the way rustc-demangle handles it. >> >> > > You may even be able to get Clang to generate C++ mangled symbols >> > > with ".llvm." suffixes, with enough application of LTO. This is >> > > not >> > > unlike GCC ".clone" suffixes, AFAIK. Sadly I don't think there's >> > > a >> > > way to handle both as "outside the symbol", without hardcoding >> > > ".llvm." in the implementation. >> > >> > We could use the scheme used by c++ where the .suffix is added as " >> > [clone .suffix]", it even handles multiple dots, where something >> > like >> > _Z3fooi.part.9.165493.constprop.775.31805 >> > demangles to >> > foo(int) [clone .part.9.165493] [clone .constprop.775.31805] >> > >> > I just don't think that is very useful and a little confusing. >> >> Calling it "clone" is a bit weird, but I just checked what rustc- >> demangle >> does for printing suffixes back out and it's not great either: >> - ".llvm." (and everything after it) is completely removed >> - any left-over suffixes (after demangling), if they start with ".", >> are >> not considered errors, but printed out verbatim after the >> demangling >> >> > > I don't recall the libiberty demangling API having any provisions >> > > for the demangler deciding that a mangled symbol "stops early", >> > > which would maybe allow for a more general solution. >> > >> > No, there indeed is no interface. We might introduce a new option >> > flag >> > for treating '.' as end of symbol. But do we really need that >> > flexibility? >> >> That's not what I meant - a v0 or legacy symbol is self-terminating >> in >> its parsing (or at the very least there are not dots allowed outside >> of a length-prefixed identifier), so that when you see the start of >> a valid mangled symbol, you can always find its end in the string, >> even when that end is half-way through (and is followed by suffixes >> or any other unrelated noise). >> >> What I was imagining is a way to return to the caller the number of >> chars from the start of the original string, that were demangled, >> letting the caller do something else with the rest of that string. >> (see below for how rustc-demangle already does something similar) >> >> > > Despite all that, if it helps in practice, I would still not mind >> > > this patch landing in its current form, I just wanted to share my >> > > perspective on the larger issue. >> > >> > Thanks for that. Do you happen to know what other rust demanglers >> > do? >> >> rustc-demangle's internal API returns a pair of the demangler and the >> "leftover" parts of the original string, after the end of the symbol. >> You can see here how that suffix is further checked, and kept: >> https://github.com/rust-lang/rustc-demangle/blob/2811a1ad6f7c8bead2ef3671e4fdc10de1553e96/src/lib.rs#L108-L138 > > Yes, returning a struct that returns the style detected, the demangled > string and the left over chars makes sense. But we don't have an > interface like that at the moment, and I am not sure we (currently) > have users who want this. > >> As mentioned above, ".llvm." is handled differently, just above the snippet >> linked - perhaps it was deemed too common to let it pollute the output. > > But that also makes it a slightly odd interface. I would imagine that > people would be interested in the .llvm. part. Now that just gets > dropped. > > Since we don't have an interface to return the suffix (and I find the > choice of dropping .llvm. but not other suffixes odd), I think we > should just simply always drop the .suffix. I understand now how to do > that for legacy symbols too, thanks for the hints. > > See the attached update to the patch. What do you think? > > Thanks, > > Mark > > Attachments: > * 0001-libiberty-rust-demangle-ignore-.suffix.patch
Re: [PATCH] libiberty rust-demangle, ignore .suffix
On Fri, Dec 3, 2021, at 00:07, Mark Wielaard wrote: > Hi Eddy, > > On Thu, Dec 02, 2021 at 07:35:17PM +0200, Eduard-Mihai Burtescu wrote: >> On Thu, Dec 2, 2021, at 19:17, Mark Wielaard wrote: >> > Rust v0 symbols can have a .suffix because if compiler transformations. >> >> For some context, the suffix comes from LLVM (I believe as part of >> its LTO). If this were a semantic part of a Rust symbol, it would >> have an encoding within v0 (as we already do for e.g. shims). > > The same is true for gccrs or the libgccjit backend, gcc might clone > the symbol name and make it unique by appending some .suffix name. > >> That also means that for consistency, suffixes like these should be >> handled uniformly for both v0 and legacy (as rustc-demangle does), >> since LLVM doesn't distinguish. > > The problem with the legacy mangling is that dot '.' is a valid > character. That is why the patch only handles the v0 mangling case > (where dot '.' isn't valid). Thought so, that's an annoying complication - however, see later down why that's still not a blocker to the way rustc-demangle handles it. >> You may even be able to get Clang to generate C++ mangled symbols >> with ".llvm." suffixes, with enough application of LTO. This is not >> unlike GCC ".clone" suffixes, AFAIK. Sadly I don't think there's a >> way to handle both as "outside the symbol", without hardcoding >> ".llvm." in the implementation. > > We could use the scheme used by c++ where the .suffix is added as " > [clone .suffix]", it even handles multiple dots, where something like > _Z3fooi.part.9.165493.constprop.775.31805 > demangles to > foo(int) [clone .part.9.165493] [clone .constprop.775.31805] > > I just don't think that is very useful and a little confusing. Calling it "clone" is a bit weird, but I just checked what rustc-demangle does for printing suffixes back out and it's not great either: - ".llvm." (and everything after it) is completely removed - any left-over suffixes (after demangling), if they start with ".", are not considered errors, but printed out verbatim after the demangling >> I don't recall the libiberty demangling API having any provisions >> for the demangler deciding that a mangled symbol "stops early", >> which would maybe allow for a more general solution. > > No, there indeed is no interface. We might introduce a new option flag > for treating '.' as end of symbol. But do we really need that > flexibility? That's not what I meant - a v0 or legacy symbol is self-terminating in its parsing (or at the very least there are not dots allowed outside of a length-prefixed identifier), so that when you see the start of a valid mangled symbol, you can always find its end in the string, even when that end is half-way through (and is followed by suffixes or any other unrelated noise). What I was imagining is a way to return to the caller the number of chars from the start of the original string, that were demangled, letting the caller do something else with the rest of that string. (see below for how rustc-demangle already does something similar) >> Despite all that, if it helps in practice, I would still not mind >> this patch landing in its current form, I just wanted to share my >> perspective on the larger issue. > > Thanks for that. Do you happen to know what other rust demanglers do? rustc-demangle's internal API returns a pair of the demangler and the "leftover" parts of the original string, after the end of the symbol. You can see here how that suffix is further checked, and kept: https://github.com/rust-lang/rustc-demangle/blob/2811a1ad6f7c8bead2ef3671e4fdc10de1553e96/src/lib.rs#L108-L138 As mentioned above, ".llvm." is handled differently, just above the snippet linked - perhaps it was deemed too common to let it pollute the output. > Cheers, > > Mark Thanks, - Eddy B.
Re: [PATCH] libiberty rust-demangle, ignore .suffix
On Thu, Dec 2, 2021, at 19:17, Mark Wielaard wrote: > Rust v0 symbols can have a .suffix because if compiler transformations. For some context, the suffix comes from LLVM (I believe as part of its LTO). If this were a semantic part of a Rust symbol, it would have an encoding within v0 (as we already do for e.g. shims). That also means that for consistency, suffixes like these should be handled uniformly for both v0 and legacy (as rustc-demangle does), since LLVM doesn't distinguish. You may even be able to get Clang to generate C++ mangled symbols with ".llvm." suffixes, with enough application of LTO. This is not unlike GCC ".clone" suffixes, AFAIK. Sadly I don't think there's a way to handle both as "outside the symbol", without hardcoding ".llvm." in the implementation. I don't recall the libiberty demangling API having any provisions for the demangler deciding that a mangled symbol "stops early", which would maybe allow for a more general solution. Despite all that, if it helps in practice, I would still not mind this patch landing in its current form, I just wanted to share my perspective on the larger issue. Thanks, - Eddy B.
Re: [PATCH] Support the new ("v0") mangling scheme in rust-demangle.
Hi everyone, Apologies again for the delay on my end, the past few weeks have been hectic and exhausting. The changes look good and pass the testing I was doing for my version of the patch. Feel free to commit Nikhil's latest patch for us. Thanks, - Eddy B. On Fri, Nov 13, 2020, at 08:42, Nikhil Benesch wrote: > On 11/6/20 12:09 PM, Jeff Law wrote: > > So I think the best path forward is to let you and Eduard-Mihai make the > > technical decisions about what bits are ready for the trunk. When y'all > > think something is ready, let's go ahead and get it installed and > > iterate on things that aren't quite ready yet. > > > > > > For bits y'all think are ready, ISTM that Eduard-Mihai should commit the > > changes. > > I've attached an updated version of the patch that contains some > additional unit tests that eddyb noticed I lost. From my perspective, > this is now ready for commit. > > Neither eddyb nor I have write access, so someone else will need to > commit. (But please wait for eddyb to sign off too.) > > > It's better to get it in sooner, but there is some degree of freedom > > depending on the impact of the changes. Changes in the rust demangler > > aren't likely to trigger codegen or ABI breakages in the compiler itself > > -- so with that in mind I think we should give this code a higher degree > > of freedom to land after the stage1 close deadline. > > Got it. Thanks. That's very helpful context. > > Nikhil > > Attachments: > * rust-demangle.patch
Re: [PATCH] Support the new ("v0") mangling scheme in rust-demangle.
Reading the diff patch, the v0 changes look great. I wouldn't be too worried
about the "printable character" aspect, there are similar Unicode-related
issues elsewhere, e.g. the "non-control ASCII" comment in decode_legacy_escape
(I suppose we could make it also go through the "print a non-control ASCII
character or some escape sequence" logic you added, if you think that helps).
However, I'm not sure about the legacy changes. Or rather, the .llvm. one, it's
not really Rust-specific, it's only in the rustc-demangle crate for convenience,
but C++ code compiled with Clang could run into the same problem - ideally,
stripping that suffix could be done uniformly in cplus-dem.c, but I decided
against making that change myself, for now.
I'm especially not comfortable removing the fast path, since that was the
condition under which I was able to make Rust demangling be attempted first,
before C++, in order to implement the Rust legacy demangling standalone,
separately from C++ demangling, so that it could be together with the v0 one.
It should be possible to keep the fast path if stripping .llvm.* suffixes is
done before either Rust or C++ demangling is attempted, but even if that would
be nice to have, IMO it should be a separate patch and not block v0 demangling.
As for the dataset, it doesn't include .llvm. because it's collected by rustc
itself, before LLVM had any chance to add any suffixes. This was done in order
to have several different mangling formats dumped at once for every symbol.
(It also contains symbols for e.g. functions that LLVM would've inlined away)
I can test the patch and upload the dataset tomorrow, but if you want to get
something committed sooner (is there a deadline for the next release?), feel
free to land the v0 changes (snprintf + const values) without the legacy ones.
Thanks,
- Eddy B.
On Sun, Nov 1, 2020, at 11:18, Nikhil Benesch wrote:
>
>
> On 10/29/20 12:16 AM, Nikhil Benesch wrote:
> > On Wed, Oct 28, 2020 at 7:53 PM Eduard-Mihai Burtescu
> > wrote:
> >> I agree that landing the extra changes later on top should be fine anyway,
> >> though when I make the sprintf -> snprintf change, I could provide the
> >> extra
> >> changes as well (either as a combined patch, or as a second tiny patch).
> >>
> >> Sadly I don't think I can get to either until next week, hope that's okay.
> >
> > I can make the sprintf -> snprintf change as early as tomorrow.
> >
> > I suspect I can also make the substantive const generics change, based
> > on the Rust implementation, though that's less of a promise.
>
> Attached is an updated patch with both of the aforementioned changes.
> The demangling of constants matches the demangling in rustc-demangle
> library as best as I could manage. The strategy of demangling constant
> chars via `format!("{:?}", char)` in Rust makes life hard for us in C,
> because there is no easy way to replicate Rust's debug formatting for
> chars. (Unless GCC has a Unicode library handy somewhere.)
>
> In the course of this work I noticed several inconsistencies in how
> rustc-demangle and libiberty were demangling some legacy Rust symbols. I
> fixed those and added test cases, though the fixes required removing
> some of the fast checks for whether a given symbol is a valid Rust symbol.
>
> For ease of review, eddyb, I also attached the diff from your last diff
> to mine. Since I don't have your collection of symbols I generated my
> own by running nm on the materialized binary from
> https://github.com/MaterializeInc/materialize.
>
> Let me know how I can help. I'm happy to address review feedback myself,
> or I'm happy to hand things back over to you, eddyb.
>
> Nikhil
>
> Attachments:
> * rust-demangle-diff.patch
> * rust-demangle.patch
Re: [PATCH] Support the new ("v0") mangling scheme in rust-demangle.
Hi Ian,
Thanks for replying! (Also I'm really sorry for the missing hard wrapping
in my earlier email, I shouldn't have sent that from my phone)
Regarding this being a blocker: we've already made the changes to Rust, and
the new ("v0") mangling format continues to remain opt-in and nightly-only,
as long as no distributions ship gdb & other tools, capable of demangling it.
I agree that landing the extra changes later on top should be fine anyway,
though when I make the sprintf -> snprintf change, I could provide the extra
changes as well (either as a combined patch, or as a second tiny patch).
Sadly I don't think I can get to either until next week, hope that's okay.
For correctness, I've been running this test after every change I make, both
with the standalone copy (in the same gist), and a local build of libiberty:
https://gist.github.com/eddyb/c41a69378750a433767cf53fe2316768#file-test-rs-L85-L116
The input is almost 1 million symbol names (collected from a full build of
the Rust compiler and Cargo), and the test ensures that the C implementation
produces the same result as the original "v0" demangler (written in Rust).
All that data is around 1GB but maybe I should try to upload it somewhere
public so that anyone can repeat this procedure, or I could even redo the
collection process (in order to gather even more / newer symbol names).
While this may not stress every aspect of the demangler, it is comprehensive
enough that it found several kinds of bugs in a 3rd party implementation
(which the IllumOS project wrote from scratch, instead of porting ours).
Thanks,
- Eddy B.
On Thu, Oct 29, 2020, at 00:45, Ian Lance Taylor wrote:
> On Wed, Oct 28, 2020 at 3:22 PM Nikhil Benesch via Gcc-patches
> wrote:
> >
> > I think it is mostly a matter of snagging some of Ian's limited time.
> > I suspect it is still worthwhile to try to get the original patch
> > reviewed and merged, because then any follow-up changes for const
> > generics support will be smaller and easier to review.
>
> Yeah, it's not a good idea for me to be a blocker for changes to Rust code.
>
> I took a quick look at the original patch. The calls to sprintf
> should use snprintf instead.
>
> Other than that it seems fine though I have no idea whether it's correct.
>
> Ian
>
>
>
> > On Wed, Oct 28, 2020 at 5:48 PM Eduard-Mihai Burtescu
> > wrote:
> > >
> > > FWIW, the patch has become slightly outdated compared to the Rust
> > > upstream, so if someone wants to review it I should prepare a new version.
> > >
> > > The changes would be for the MVP version of "const generics" (Rust's
> > > equivalent to C++ templates with value parameters, enabling e.g. types
> > > like `CustomArray`), which supports a few different kinds of
> > > primitive values, whereas the original patch only handled unsigned
> > > integers.
> > >
> > > Thanks,
> > > - Eddy B.
> > >
> > > On Wed, Oct 28, 2020, at 23:25, Nikhil Benesch wrote:
> > > > On 10/28/20 5:22 PM, Nikhil Benesch wrote:
> > > > > Ian, are you able to review this? I saw that you reviewed many of the
> > > > > prior changes to the Rust demangler.
> > > > >
> > > > > If not, can you suggest someone who can?
> > > > >
> > > > > Thanks very much.
> > > > > Nikhil
> > > >
> > > > I seem to have failed to convince my email client to set the appropriate
> > > > reply to headers. This is the patch to which I was referring:
> > > >
> > > > https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542488.html
> > > >
>
Re: [PATCH] Support the new ("v0") mangling scheme in rust-demangle.
FWIW, the patch has become slightly outdated compared to the Rust upstream, so if someone wants to review it I should prepare a new version. The changes would be for the MVP version of "const generics" (Rust's equivalent to C++ templates with value parameters, enabling e.g. types like `CustomArray`), which supports a few different kinds of primitive values, whereas the original patch only handled unsigned integers. Thanks, - Eddy B. On Wed, Oct 28, 2020, at 23:25, Nikhil Benesch wrote: > On 10/28/20 5:22 PM, Nikhil Benesch wrote: > > Ian, are you able to review this? I saw that you reviewed many of the > > prior changes to the Rust demangler. > > > > If not, can you suggest someone who can? > > > > Thanks very much. > > Nikhil > > I seem to have failed to convince my email client to set the appropriate > reply to headers. This is the patch to which I was referring: > > https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542488.html >
Re: [PATCH] Support the new ("v0") mangling scheme in rust-demangle.
Ping 4: https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542012.html Thanks, - Eddy B. On Mon, Apr 13, 2020, at 05:52, Eduard-Mihai Burtescu wrote: > Ping 3: https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542012.html > > Thanks, > - Eddy B. > > On Tue, Apr 7, 2020, at 00:52, Eduard-Mihai Burtescu wrote: > > Ping 2: https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542012.html > > > > Thanks, > > - Eddy B. > > > > On Fri, Mar 13, 2020, at 10:28 PM, Eduard-Mihai Burtescu wrote: > > > This is the libiberty (mainly for binutils/gdb) counterpart of > > > https://github.com/alexcrichton/rustc-demangle/pull/23. > > > > > > Relevant links for the new Rust mangling scheme (aka "v0"): > > > * Rust RFC: https://github.com/rust-lang/rfcs/pull/2603 > > > * tracking issue: https://github.com/rust-lang/rust/issues/60705 > > > * implementation: https://github.com/rust-lang/rust/pull/57967 > > > > > > This implementation includes full support for UTF-8 identifiers > > > via punycode, so I've included a testcase for that as well. > > > (Let me know if it causes any issues and I'll take it out) > > > > > > Last year I've submitted several small patches to rust-demangle > > > in preparation for upstreaming the entire new demangler, and > > > feedback from that has proven useful. > > > For example, I started with error-handling macros, but instead > > > the code now has "rdm->errored = 1;" before several returns/gotos. > > > > > > The patch is attached instead of inline, as it's over 1000 lines long. > > > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > > > > > Also, I have no commit access, so I'd be thankful if > > > someone would commit this for me if/once approved. > > > Attachments: > > > * 0001-Support-the-new-v0-mangling-scheme-in-rust-demangle.patch
Re: [PATCH] Support the new ("v0") mangling scheme in rust-demangle.
Ping 3: https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542012.html Thanks, - Eddy B. On Tue, Apr 7, 2020, at 00:52, Eduard-Mihai Burtescu wrote: > Ping 2: https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542012.html > > Thanks, > - Eddy B. > > On Fri, Mar 13, 2020, at 10:28 PM, Eduard-Mihai Burtescu wrote: > > This is the libiberty (mainly for binutils/gdb) counterpart of > > https://github.com/alexcrichton/rustc-demangle/pull/23. > > > > Relevant links for the new Rust mangling scheme (aka "v0"): > > * Rust RFC: https://github.com/rust-lang/rfcs/pull/2603 > > * tracking issue: https://github.com/rust-lang/rust/issues/60705 > > * implementation: https://github.com/rust-lang/rust/pull/57967 > > > > This implementation includes full support for UTF-8 identifiers > > via punycode, so I've included a testcase for that as well. > > (Let me know if it causes any issues and I'll take it out) > > > > Last year I've submitted several small patches to rust-demangle > > in preparation for upstreaming the entire new demangler, and > > feedback from that has proven useful. > > For example, I started with error-handling macros, but instead > > the code now has "rdm->errored = 1;" before several returns/gotos. > > > > The patch is attached instead of inline, as it's over 1000 lines long. > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > > > Also, I have no commit access, so I'd be thankful if > > someone would commit this for me if/once approved. > > Attachments: > > * 0001-Support-the-new-v0-mangling-scheme-in-rust-demangle.patch
Re: [PATCH] Support the new ("v0") mangling scheme in rust-demangle.
Ping 2: https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542012.html Thanks, - Eddy B. On Fri, Mar 13, 2020, at 10:28 PM, Eduard-Mihai Burtescu wrote: > This is the libiberty (mainly for binutils/gdb) counterpart of > https://github.com/alexcrichton/rustc-demangle/pull/23. > > Relevant links for the new Rust mangling scheme (aka "v0"): > * Rust RFC: https://github.com/rust-lang/rfcs/pull/2603 > * tracking issue: https://github.com/rust-lang/rust/issues/60705 > * implementation: https://github.com/rust-lang/rust/pull/57967 > > This implementation includes full support for UTF-8 identifiers > via punycode, so I've included a testcase for that as well. > (Let me know if it causes any issues and I'll take it out) > > Last year I've submitted several small patches to rust-demangle > in preparation for upstreaming the entire new demangler, and > feedback from that has proven useful. > For example, I started with error-handling macros, but instead > the code now has "rdm->errored = 1;" before several returns/gotos. > > The patch is attached instead of inline, as it's over 1000 lines long. > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > Also, I have no commit access, so I'd be thankful if > someone would commit this for me if/once approved. > Attachments: > * 0001-Support-the-new-v0-mangling-scheme-in-rust-demangle.patch
Re: [PATCH] Support the new ("v0") mangling scheme in rust-demangle.
Ping: https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542012.html Thanks, - Eddy B. On Fri, Mar 13, 2020, at 10:28 PM, Eduard-Mihai Burtescu wrote: > This is the libiberty (mainly for binutils/gdb) counterpart of > https://github.com/alexcrichton/rustc-demangle/pull/23. > > Relevant links for the new Rust mangling scheme (aka "v0"): > * Rust RFC: https://github.com/rust-lang/rfcs/pull/2603 > * tracking issue: https://github.com/rust-lang/rust/issues/60705 > * implementation: https://github.com/rust-lang/rust/pull/57967 > > This implementation includes full support for UTF-8 identifiers > via punycode, so I've included a testcase for that as well. > (Let me know if it causes any issues and I'll take it out) > > Last year I've submitted several small patches to rust-demangle > in preparation for upstreaming the entire new demangler, and > feedback from that has proven useful. > For example, I started with error-handling macros, but instead > the code now has "rdm->errored = 1;" before several returns/gotos. > > The patch is attached instead of inline, as it's over 1000 lines long. > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > Also, I have no commit access, so I'd be thankful if > someone would commit this for me if/once approved. > Attachments: > * 0001-Support-the-new-v0-mangling-scheme-in-rust-demangle.patch
[PATCH] Support the new ("v0") mangling scheme in rust-demangle.
This is the libiberty (mainly for binutils/gdb) counterpart of
https://github.com/alexcrichton/rustc-demangle/pull/23.
Relevant links for the new Rust mangling scheme (aka "v0"):
* Rust RFC: https://github.com/rust-lang/rfcs/pull/2603
* tracking issue: https://github.com/rust-lang/rust/issues/60705
* implementation: https://github.com/rust-lang/rust/pull/57967
This implementation includes full support for UTF-8 identifiers
via punycode, so I've included a testcase for that as well.
(Let me know if it causes any issues and I'll take it out)
Last year I've submitted several small patches to rust-demangle
in preparation for upstreaming the entire new demangler, and
feedback from that has proven useful.
For example, I started with error-handling macros, but instead
the code now has "rdm->errored = 1;" before several returns/gotos.
The patch is attached instead of inline, as it's over 1000 lines long.
Bootstrapped and tested on x86_64-unknown-linux-gnu.
Also, I have no commit access, so I'd be thankful if
someone would commit this for me if/once approved.From 689442d88b87453a697d2121e45a55071cb9056f Mon Sep 17 00:00:00 2001
From: Eduard-Mihai Burtescu
Date: Fri, 13 Mar 2020 21:07:28 +0200
Subject: [PATCH] Support the new ("v0") mangling scheme in rust-demangle.
This is the libiberty (mainly for binutils/gdb) counterpart of
https://github.com/alexcrichton/rustc-demangle/pull/23.
Relevant links for the new Rust mangling scheme (aka "v0"):
* Rust RFC: https://github.com/rust-lang/rfcs/pull/2603
* tracking issue: https://github.com/rust-lang/rust/issues/60705
* implementation: https://github.com/rust-lang/rust/pull/57967
This implementation includes full support for UTF-8 identifiers
via punycode, so I've included a testcase for that as well.
2020-03-13 Eduard-Mihai Burtescu
libiberty/ChangeLog:
* rust-demangle.c (struct rust_demangler): Add
skipping_printing and bound_lifetime_depth fields.
(eat): Add (v0-only).
(parse_integer_62): Add (v0-only).
(parse_opt_integer_62): Add (v0-only).
(parse_disambiguator): Add (v0-only).
(struct rust_mangled_ident): Add punycode{,_len} fields.
(parse_ident): Support v0 identifiers.
(print_str): Respect skipping_printing.
(print_uint64): Add (v0-only).
(print_uint64_hex): Add (v0-only).
(print_ident): Respect skipping_printing,
Support v0 identifiers.
(print_lifetime_from_index): Add (v0-only).
(demangle_binder): Add (v0-only).
(demangle_path): Add (v0-only).
(demangle_generic_arg): Add (v0-only).
(demangle_type): Add (v0-only).
(demangle_path_maybe_open_generics): Add (v0-only).
(demangle_dyn_trait): Add (v0-only).
(demangle_const): Add (v0-only).
(demangle_const_uint): Add (v0-only).
(basic_type): Add (v0-only).
(rust_demangle_callback): Support v0 symbols.
* testsuite/rust-demangle-expected: Add v0 testcases.
diff --git a/libiberty/rust-demangle.c b/libiberty/rust-demangle.c
index b87365c85fe9..9a31392458e0 100644
--- a/libiberty/rust-demangle.c
+++ b/libiberty/rust-demangle.c
@@ -1,6 +1,7 @@
/* Demangler for the Rust programming language
Copyright (C) 2016-2020 Free Software Foundation, Inc.
Written by David Tolnay (dtol...@gmail.com).
+ Rewritten by Eduard-Mihai Burtescu (ed...@lyken.rs) for v0 support.
This file is part of the libiberty library.
Libiberty is free software; you can redistribute it and/or
@@ -64,11 +65,16 @@ struct rust_demangler
/* Non-zero if any error occurred. */
int errored;
+ /* Non-zero if nothing should be printed. */
+ int skipping_printing;
+
/* Non-zero if printing should be verbose (e.g. include hashes). */
int verbose;
/* Rust mangling version, with legacy mangling being -1. */
int version;
+
+ uint64_t bound_lifetime_depth;
};
/* Parsing functions. */
@@ -81,6 +87,18 @@ peek (const struct rust_demangler *rdm)
return 0;
}
+static int
+eat (struct rust_demangler *rdm, char c)
+{
+ if (peek (rdm) == c)
+{
+ rdm->next++;
+ return 1;
+}
+ else
+return 0;
+}
+
static char
next (struct rust_demangler *rdm)
{
@@ -92,11 +110,58 @@ next (struct rust_demangler *rdm)
return c;
}
+static uint64_t
+parse_integer_62 (struct rust_demangler *rdm)
+{
+ char c;
+ uint64_t x;
+
+ if (eat (rdm, '_'))
+return 0;
+
+ x = 0;
+ while (!eat (rdm, '_'))
+{
+ c = next (rdm);
+ x *= 62;
+ if (ISDIGIT (c))
+x += c - '0';
+ else if (ISLOWER (c))
+x += 10 + (c - 'a');
+ else if (ISUPPER (c))
+x += 10 + 26 + (c - 'A');
+ else
+{
+ rdm->errored = 1;
+ return 0;
+}
+}
+ return x + 1;
+}
+
+static uint64_t
+parse_opt_integer_62 (struct rust_demangler *rdm, char tag)
+{
+ if (!eat (rdm, tag))
+return 0;
+ return 1 + parse_integer_62 (rdm);
+}
+
+static uint64_t
+parse_disambiguator (struct rust_demangler *rdm)
+{
+ return parse_opt_integer_62 (rdm, 's');
+}
+
struct rust_mangled_ident
{
/* ASCI
Re: [PATCH] Refactor rust-demangle to be independent of C++ demangling.
> This is OK. > > Thanks. > > Ian > Ping for someone to commit this (as mentioned before, I have no commit access). I've tried pinging some people on IRC, but saw no response. Approved version of the patch: https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00647.html Original email, containing the description: https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01591.html (ignore the original patch, two changes had to be made before it was approved) Thanks, - Eddy B.
Re: [PATCH] Refactor rust-demangle to be independent of C++ demangling.
On Fri, Nov 8, 2019, at 7:43 PM, Ian Lance Taylor wrote:
> On Fri, Nov 8, 2019 at 9:02 AM Eduard-Mihai Burtescu wrote:
> >
> > Ping #2 for https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01830.html
> > Original patch (without the early exit optimization):
> > https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01591.html
>
> Sorry for letting this slide.
>
> Do we need the CHECK_OR and ERROR_AND macros? Is there anything like
> those elsewhere in the libiberty or GCC sources? I would rather than
> have ordinary code than obscure macros.
Good point, I was wondering about the macros but forgot to ask explicitly,
they're the least usual (for C) part of this code, they arose from porting the
demangler for the new format (most of which isn't even in the current patch)
from Rust, where we have facilities for conveniently propagating errors, and I
wanted to avoid using goto too much for this purpose.
Looking at
https://gist.github.com/eddyb/c41a69378750a433767cf53fe2316768#file-rust-demangle-c
I can see:
* 5 uses of ERROR_AND
* 20 uses of CHECK_OR
* 7 of those are CHECK_OR (!rdm->errored, return ); which can just be if
(rdm->errored) return;
So in the final code there'd be ~18 places that would need to set rdm->errored
= 1; (and then return or goto cleanup).
That's not that bad, I guess, and I'd welcome any suggestions for how to clean
up that.
For this current patch, however, there's only 3 uses total, so the macros are
definitely overkill.
Assuming you'd want them removed, I took the liberty of doing that and here's
the fixed patch:
2019-10-22 Eduard-Mihai Burtescu
include/ChangeLog:
* demangle.h (rust_demangle_callback): Add.
libiberty/ChangeLog:
* cplus-dem.c (cplus_demangle): Use rust_demangle directly.
(rust_demangle): Remove.
* rust-demangle.c (is_prefixed_hash): Rename to is_legacy_prefixed_hash.
(parse_lower_hex_nibble): Rename to decode_lower_hex_nibble.
(parse_legacy_escape): Rename to decode_legacy_escape.
(rust_is_mangled): Remove.
(struct rust_demangler): Add.
(peek): Add.
(next): Add.
(struct rust_mangled_ident): Add.
(parse_ident): Add.
(rust_demangle_sym): Remove.
(print_str): Add.
(PRINT): Add.
(print_ident): Add.
(rust_demangle_callback): Add.
(struct str_buf): Add.
(str_buf_reserve): Add.
(str_buf_append): Add.
(str_buf_demangle_callback): Add.
(rust_demangle): Add.
* rust-demangle.h: Remove.
diff --git a/include/demangle.h b/include/demangle.h
index 06c32571d5c..ce7235d13f3 100644
--- a/include/demangle.h
+++ b/include/demangle.h
@@ -159,6 +159,11 @@ ada_demangle (const char *mangled, int options);
extern char *
dlang_demangle (const char *mangled, int options);
+extern int
+rust_demangle_callback (const char *mangled, int options,
+demangle_callbackref callback, void *opaque);
+
+
extern char *
rust_demangle (const char *mangled, int options);
diff --git a/libiberty/cplus-dem.c b/libiberty/cplus-dem.c
index a39e2bf2ed4..735a61d7a82 100644
--- a/libiberty/cplus-dem.c
+++ b/libiberty/cplus-dem.c
@@ -52,7 +52,6 @@ void * realloc ();
#define CURRENT_DEMANGLING_STYLE options
#include "libiberty.h"
-#include "rust-demangle.h"
enum demangling_styles current_demangling_style = auto_demangling;
@@ -160,27 +159,20 @@ cplus_demangle (const char *mangled, int options)
if ((options & DMGL_STYLE_MASK) == 0)
options |= (int) current_demangling_style & DMGL_STYLE_MASK;
+ /* The Rust demangling is implemented elsewhere.
+ Legacy Rust symbols overlap with GNU_V3, so try Rust first. */
+ if (RUST_DEMANGLING || AUTO_DEMANGLING)
+{
+ ret = rust_demangle (mangled, options);
+ if (ret || RUST_DEMANGLING)
+return ret;
+}
+
/* The V3 ABI demangling is implemented elsewhere. */
- if (GNU_V3_DEMANGLING || RUST_DEMANGLING || AUTO_DEMANGLING)
+ if (GNU_V3_DEMANGLING || AUTO_DEMANGLING)
{
ret = cplus_demangle_v3 (mangled, options);
- if (GNU_V3_DEMANGLING)
- return ret;
-
- if (ret)
- {
- /* Rust symbols are GNU_V3 mangled plus some extra subtitutions.
-The subtitutions are always smaller, so do in place changes. */
- if (rust_is_mangled (ret))
- rust_demangle_sym (ret);
- else if (RUST_DEMANGLING)
- {
- free (ret);
- ret = NULL;
- }
- }
-
- if (ret || RUST_DEMANGLING)
+ if (ret || GNU_V3_DEMANGLING)
return ret;
}
@@ -204,27 +196,6 @@ cplus_demangle (const char *mangled, int options)
return (ret);
}
-char *
-rust_demangle (const char *mangled, int options)
-{
- /* Rust symbols are GNU_V3 mangled plus some extra subtitutions. */
- char *ret = cplus_demangle_v3 (mangled, options);
-
- /
Re: [PATCH] Refactor rust-demangle to be independent of C++ demangling.
Ping #2 for https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01830.html Original patch (without the early exit optimization): https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01591.html Thanks, - Eddy B. On Wed, Oct 30, 2019, at 6:46 PM, Eduard-Mihai Burtescu wrote: > Ping: https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01830.html > Original patch (without the early exit optimization): > https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01591.html > > Thanks, > - Eddy B. > > On Fri, Oct 25, 2019, at 3:44 PM, Eduard-Mihai Burtescu wrote: > > > This can be further optimized by using memcmp in place of strncmp, since > > > from > > > the length check you know that you won't see the null terminator among > > > the three > > > chars you're checking. > > > > Fair enough, here's the combined changelog/diff, with memcmp:
Re: [PATCH] Refactor rust-demangle to be independent of C++ demangling.
Ping: https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01830.html Original patch (without the early exit optimization): https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01591.html Thanks, - Eddy B. On Fri, Oct 25, 2019, at 3:44 PM, Eduard-Mihai Burtescu wrote: > > This can be further optimized by using memcmp in place of strncmp, since > > from > > the length check you know that you won't see the null terminator among the > > three > > chars you're checking. > > Fair enough, here's the combined changelog/diff, with memcmp:
Re: [PATCH] Refactor rust-demangle to be independent of C++ demangling.
> This can be further optimized by using memcmp in place of strncmp, since from
> the length check you know that you won't see the null terminator among the
> three
> chars you're checking.
Fair enough, here's the combined changelog/diff, with memcmp:
2019-10-22 Eduard-Mihai Burtescu
include/ChangeLog:
* demangle.h (rust_demangle_callback): Add.
libiberty/ChangeLog:
* cplus-dem.c (cplus_demangle): Use rust_demangle directly.
(rust_demangle): Remove.
* rust-demangle.c (is_prefixed_hash): Rename to is_legacy_prefixed_hash.
(parse_lower_hex_nibble): Rename to decode_lower_hex_nibble.
(parse_legacy_escape): Rename to decode_legacy_escape.
(rust_is_mangled): Remove.
(struct rust_demangler): Add.
(ERROR_AND): Add.
(CHECK_OR): Add.
(peek): Add.
(next): Add.
(struct rust_mangled_ident): Add.
(parse_ident): Add.
(rust_demangle_sym): Remove.
(print_str): Add.
(PRINT): Add.
(print_ident): Add.
(rust_demangle_callback): Add.
(struct str_buf): Add.
(str_buf_reserve): Add.
(str_buf_append): Add.
(str_buf_demangle_callback): Add.
(rust_demangle): Add.
* rust-demangle.h: Remove.
diff --git a/include/demangle.h b/include/demangle.h
index 06c32571d5c..ce7235d13f3 100644
--- a/include/demangle.h
+++ b/include/demangle.h
@@ -159,6 +159,11 @@ ada_demangle (const char *mangled, int options);
extern char *
dlang_demangle (const char *mangled, int options);
+extern int
+rust_demangle_callback (const char *mangled, int options,
+demangle_callbackref callback, void *opaque);
+
+
extern char *
rust_demangle (const char *mangled, int options);
diff --git a/libiberty/cplus-dem.c b/libiberty/cplus-dem.c
index a39e2bf2ed4..735a61d7a82 100644
--- a/libiberty/cplus-dem.c
+++ b/libiberty/cplus-dem.c
@@ -52,7 +52,6 @@ void * realloc ();
#define CURRENT_DEMANGLING_STYLE options
#include "libiberty.h"
-#include "rust-demangle.h"
enum demangling_styles current_demangling_style = auto_demangling;
@@ -160,27 +159,20 @@ cplus_demangle (const char *mangled, int options)
if ((options & DMGL_STYLE_MASK) == 0)
options |= (int) current_demangling_style & DMGL_STYLE_MASK;
+ /* The Rust demangling is implemented elsewhere.
+ Legacy Rust symbols overlap with GNU_V3, so try Rust first. */
+ if (RUST_DEMANGLING || AUTO_DEMANGLING)
+{
+ ret = rust_demangle (mangled, options);
+ if (ret || RUST_DEMANGLING)
+return ret;
+}
+
/* The V3 ABI demangling is implemented elsewhere. */
- if (GNU_V3_DEMANGLING || RUST_DEMANGLING || AUTO_DEMANGLING)
+ if (GNU_V3_DEMANGLING || AUTO_DEMANGLING)
{
ret = cplus_demangle_v3 (mangled, options);
- if (GNU_V3_DEMANGLING)
- return ret;
-
- if (ret)
- {
- /* Rust symbols are GNU_V3 mangled plus some extra subtitutions.
-The subtitutions are always smaller, so do in place changes. */
- if (rust_is_mangled (ret))
- rust_demangle_sym (ret);
- else if (RUST_DEMANGLING)
- {
- free (ret);
- ret = NULL;
- }
- }
-
- if (ret || RUST_DEMANGLING)
+ if (ret || GNU_V3_DEMANGLING)
return ret;
}
@@ -204,27 +196,6 @@ cplus_demangle (const char *mangled, int options)
return (ret);
}
-char *
-rust_demangle (const char *mangled, int options)
-{
- /* Rust symbols are GNU_V3 mangled plus some extra subtitutions. */
- char *ret = cplus_demangle_v3 (mangled, options);
-
- /* The Rust subtitutions are always smaller, so do in place changes. */
- if (ret != NULL)
-{
- if (rust_is_mangled (ret))
- rust_demangle_sym (ret);
- else
- {
- free (ret);
- ret = NULL;
- }
-}
-
- return ret;
-}
-
/* Demangle ada names. The encoding is documented in gcc/ada/exp_dbug.ads. */
char *
diff --git a/libiberty/rust-demangle.c b/libiberty/rust-demangle.c
index 6b62e6dbd80..95255d0b601 100644
--- a/libiberty/rust-demangle.c
+++ b/libiberty/rust-demangle.c
@@ -33,9 +33,11 @@ If not, see <http://www.gnu.org/licenses/>. */
#include "safe-ctype.h"
+#include
#include
#include
#include
+#include
#ifdef HAVE_STRING_H
#include
@@ -47,207 +49,115 @@ extern void *memset(void *s, int c, size_t n);
#include
#include "libiberty.h"
-#include "rust-demangle.h"
-
-/* Mangled (legacy) Rust symbols look like this:
-
_$LT$std..sys..fd..FileDesc$u20$as$u20$core..ops..Drop$GT$::drop::hc68340e1baa4987a
-
- The original symbol is:
- ::drop
-
- The last component of the path is a 64-bit hash in lowercase hex,
- prefixed with "h". Rust does not have a global namespace between
- crates, an illusion which Rust maintains by using the hash to
- distinguish t
Re: [PATCH] Refactor rust-demangle to be independent of C++ demangling.
On Tue, Oct 22, 2019, at 9:39 PM, Ian Lance Taylor wrote:
> I have to assume that C++ demangling is still quite a bit more common
> than Rust demangling, so it's troubling that it looks like we're going
> to do extra work for each symbol that starts with _ZN, which is not a
> particularly uncommon prefix for a C++ mangled name. Is there some
> way we can quickly separate out Rust symbols? Or should we try C++
> demangling first?
>
> Ian
>
I definitely agree, I don't want to make demangling plain C++ symbols
significantly slower. The old code was also doing extra work, at least
in the AUTO_DEMANGLING mode, but less than the parse_ident
loop in this patch.
I've come up with an extra quick check that regular C++ symbols
won't pass most of the time and placed it before the parse_ident
loop, that should make it comparable with the old implementation,
and tests pass just fine with the extra check.
The diff is below, but if you want me to send a combined patch,
or anything else for that matter, please let me know.
diff --git a/libiberty/rust-demangle.c b/libiberty/rust-demangle.c
index da707dbab9b..4cb189c4019 100644
--- a/libiberty/rust-demangle.c
+++ b/libiberty/rust-demangle.c
@@ -384,6 +384,14 @@ rust_demangle_callback (const char *mangled, int options,
return 0;
rdm.sym_len--;
+ /* Legacy Rust symbols also always end with a path segment
+ that encodes a 16 hex digit hash, i.e. '17h[a-f0-9]{16}'.
+ This early check, before any parse_ident calls, should
+ quickly filter out most C++ symbols unrelated to Rust. */
+ if (!(rdm.sym_len > 19
+&& !strncmp ([rdm.sym_len - 19], "17h", 3)))
+return 0;
+
do
{
ident = parse_ident ();
[PATCH] Refactor rust-demangle to be independent of C++ demangling.
This way, rust-demangle is closer to the official "rustc-demangle"
Rust demangler, which also doesn't require full C++ demangling for
the legacy format (implemented here), and only recognizes simple
"_ZN"..."E" paths with identifier components that the format uses.
The new code also has fewer dependencies, so it would be easier
to integrate into a codebase which doesn't use the entirety of
libiberty, and doesn't have a C++ demangler, or it does but
with a different API which would require adjustments to use.
Lastly but not least, the new design allows for a single unified
demangler that internally handles both the legacy format (the only
one implemented right now) and the v0 format (which will come in
a later patch), without exposing any "postprocess C++ symbol" API.
You can see the unified demangler that I plan to upstream soon, at:
https://gist.github.com/eddyb/c41a69378750a433767cf53fe2316768
I've been trying to keep each patch a self-contained step to limit
review work and potential fallout, but if you think I should just
submit the entire final unified rust-demangle instead, I'll do it.
Bootstrapped and tested on x86_64-unknown-linux-gnu.
Also, I have no commit access, so I'd be thankful if
someone would commit this for me if/once approved.
2019-10-22 Eduard-Mihai Burtescu
include/ChangeLog:
* demangle.h (rust_demangle_callback): Add.
libiberty/ChangeLog:
* cplus-dem.c (cplus_demangle): Use rust_demangle directly.
(rust_demangle): Remove.
* rust-demangle.c (is_prefixed_hash): Rename to is_legacy_prefixed_hash.
(parse_lower_hex_nibble): Rename to decode_lower_hex_nibble.
(parse_legacy_escape): Rename to decode_legacy_escape.
(rust_is_mangled): Remove.
(struct rust_demangler): Add.
(ERROR_AND): Add.
(CHECK_OR): Add.
(peek): Add.
(next): Add.
(struct rust_mangled_ident): Add.
(parse_ident): Add.
(rust_demangle_sym): Remove.
(print_str): Add.
(PRINT): Add.
(print_ident): Add.
(rust_demangle_callback): Add.
(struct str_buf): Add.
(str_buf_reserve): Add.
(str_buf_append): Add.
(str_buf_demangle_callback): Add.
(rust_demangle): Add.
* rust-demangle.h: Remove.
diff --git a/include/demangle.h b/include/demangle.h
index 06c32571d5c..ce7235d13f3 100644
--- a/include/demangle.h
+++ b/include/demangle.h
@@ -159,6 +159,11 @@ ada_demangle (const char *mangled, int options);
extern char *
dlang_demangle (const char *mangled, int options);
+extern int
+rust_demangle_callback (const char *mangled, int options,
+demangle_callbackref callback, void *opaque);
+
+
extern char *
rust_demangle (const char *mangled, int options);
diff --git a/libiberty/cplus-dem.c b/libiberty/cplus-dem.c
index a39e2bf2ed4..735a61d7a82 100644
--- a/libiberty/cplus-dem.c
+++ b/libiberty/cplus-dem.c
@@ -52,7 +52,6 @@ void * realloc ();
#define CURRENT_DEMANGLING_STYLE options
#include "libiberty.h"
-#include "rust-demangle.h"
enum demangling_styles current_demangling_style = auto_demangling;
@@ -160,27 +159,20 @@ cplus_demangle (const char *mangled, int options)
if ((options & DMGL_STYLE_MASK) == 0)
options |= (int) current_demangling_style & DMGL_STYLE_MASK;
+ /* The Rust demangling is implemented elsewhere.
+ Legacy Rust symbols overlap with GNU_V3, so try Rust first. */
+ if (RUST_DEMANGLING || AUTO_DEMANGLING)
+{
+ ret = rust_demangle (mangled, options);
+ if (ret || RUST_DEMANGLING)
+return ret;
+}
+
/* The V3 ABI demangling is implemented elsewhere. */
- if (GNU_V3_DEMANGLING || RUST_DEMANGLING || AUTO_DEMANGLING)
+ if (GNU_V3_DEMANGLING || AUTO_DEMANGLING)
{
ret = cplus_demangle_v3 (mangled, options);
- if (GNU_V3_DEMANGLING)
- return ret;
-
- if (ret)
- {
- /* Rust symbols are GNU_V3 mangled plus some extra subtitutions.
-The subtitutions are always smaller, so do in place changes. */
- if (rust_is_mangled (ret))
- rust_demangle_sym (ret);
- else if (RUST_DEMANGLING)
- {
- free (ret);
- ret = NULL;
- }
- }
-
- if (ret || RUST_DEMANGLING)
+ if (ret || GNU_V3_DEMANGLING)
return ret;
}
@@ -204,27 +196,6 @@ cplus_demangle (const char *mangled, int options)
return (ret);
}
-char *
-rust_demangle (const char *mangled, int options)
-{
- /* Rust symbols are GNU_V3 mangled plus some extra subtitutions. */
- char *ret = cplus_demangle_v3 (mangled, options);
-
- /* The Rust subtitutions are always smaller, so do in place changes. */
- if (ret != NULL)
-{
- if (rust_is_mangled (ret))
- rust_demangle_sym (ret);
- else
- {
- free (ret);
- ret = NULL;
- }
-}
-
- return ret;
Re: [PATCH] Remove some restrictions from rust-demangle.
Ping: https://gcc.gnu.org/ml/gcc-patches/2019-09/msg01469.html On Wed, Sep 25, 2019, at 8:25 PM, Eduard-Mihai Burtescu wrote: > The main change here is in the treatment of $...$ escapes. > I've relaxed the treatment of unknown escapes, during > unescaping, to continue processing the input string, > leaving the remainder of current path segment as-is. > Relatedly, rust_is_mangled function doesn't check escapes > at all anymore (as unknown escapes aren't errors now). > > E.g. "a$LT$b$X$c$GT$::d$C$e" would now be demangled to > "a instead of being treated as an invalid Rust symbol. > > This behavior matches the official Rust demangler, the > intention being that this more gracefully handles new > escapes being added to the legacy mangling format. > > The other change is allowing the hash at the end of the > Rust symbol to have all 16 of the possible hex nibbles. > Previously the maximum was 15, as a permutation of all > 16 hex nibbles was considered highly unlikely, but I > stumbled across this example in my large (1M) data set: > "_ZN4core3ptr18real_drop_in_place17h8abe3105492df6c7E" > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > Also, I have no commit access, so I'd be thankful if > someone would commit this for me if/once approved. > > 2019-09-25 Eduard-Mihai Burtescu > libiberty/ChangeLog: > * rust-demangle.c (looks_like_rust): Remove. > (rust_is_mangled): Don't check escapes. > (is_prefixed_hash): Allow 0-9a-f permutations. > (rust_demangle_sym): Don't bail on unknown escapes. > * testsuite/rust-demangle-expected: Update 'main::$99$' test.
[PATCH] Remove some restrictions from rust-demangle.
The main change here is in the treatment of $...$ escapes.
I've relaxed the treatment of unknown escapes, during
unescaping, to continue processing the input string,
leaving the remainder of current path segment as-is.
Relatedly, rust_is_mangled function doesn't check escapes
at all anymore (as unknown escapes aren't errors now).
E.g. "a$LT$b$X$c$GT$::d$C$e" would now be demangled to
"a
libiberty/ChangeLog:
* rust-demangle.c (looks_like_rust): Remove.
(rust_is_mangled): Don't check escapes.
(is_prefixed_hash): Allow 0-9a-f permutations.
(rust_demangle_sym): Don't bail on unknown escapes.
* testsuite/rust-demangle-expected: Update 'main::$99$' test.
diff --git a/libiberty/rust-demangle.c b/libiberty/rust-demangle.c
index da591902db1..6b62e6dbd80 100644
--- a/libiberty/rust-demangle.c
+++ b/libiberty/rust-demangle.c
@@ -85,7 +85,6 @@ static const size_t hash_prefix_len = 3;
static const size_t hash_len = 16;
static int is_prefixed_hash (const char *start);
-static int looks_like_rust (const char *sym, size_t len);
static int parse_lower_hex_nibble (char nibble);
static char parse_legacy_escape (const char **in);
@@ -105,16 +104,13 @@ static char parse_legacy_escape (const char **in);
negative (the rare Rust symbol is not demangled) so this sets
the balance in favor of false negatives.
- 3. There must be no characters other than a-zA-Z0-9 and _.:$
-
- 4. There must be no unrecognized $-sign sequences.
-
- 5. There must be no sequence of three or more dots in a row ("..."). */
+ 3. There must be no characters other than a-zA-Z0-9 and _.:$ */
int
rust_is_mangled (const char *sym)
{
size_t len, len_without_hash;
+ const char *end;
if (!sym)
return 0;
@@ -128,12 +124,22 @@ rust_is_mangled (const char *sym)
if (!is_prefixed_hash (sym + len_without_hash))
return 0;
- return looks_like_rust (sym, len_without_hash);
+ end = sym + len_without_hash;
+
+ while (sym < end)
+{
+ if (*sym == '$' || *sym == '.' || *sym == '_' || *sym == ':'
+ || ISALNUM (*sym))
+sym++;
+ else
+return 0;
+}
+
+ return 1;
}
/* A hash is the prefix "::h" followed by 16 lowercase hex digits. The
- hex digits must comprise between 5 and 15 (inclusive) distinct
- digits. */
+ hex digits must contain at least 5 distinct digits. */
static int
is_prefixed_hash (const char *str)
@@ -162,28 +168,7 @@ is_prefixed_hash (const char *str)
if (seen[i])
count++;
- return count >= 5 && count <= 15;
-}
-
-static int
-looks_like_rust (const char *str, size_t len)
-{
- const char *end = str + len;
-
- while (str < end)
-{
- if (*str == '$')
-{
- if (!parse_legacy_escape ())
-return 0;
-}
- else if (*str == '.' || *str == '_' || *str == ':' || ISALNUM (*str))
-str++;
- else
-return 0;
-}
-
- return 1;
+ return count >= 5;
}
/*
@@ -215,8 +200,9 @@ rust_demangle_sym (char *sym)
if (unescaped)
*out++ = unescaped;
else
-/* unexpected escape sequence, not looks_like_rust. */
-goto fail;
+/* unexpected escape sequence, skip the rest of this segment. */
+while (in < end && *in != ':')
+ *out++ = *in++;
}
else if (*in == '_')
{
@@ -248,14 +234,14 @@ rust_demangle_sym (char *sym)
else if (*in == ':' || ISALNUM (*in))
*out++ = *in++;
else
-/* unexpected character in symbol, not looks_like_rust. */
-goto fail;
+{
+ /* unexpected character in symbol, not rust_is_mangled. */
+ *out++ = '?'; /* This is pretty lame, but it's hard to do better. */
+ *out = '\0';
+ return;
+}
}
- goto done;
-fail:
- *out++ = '?'; /* This is pretty lame, but it's hard to do better. */
-done:
*out = '\0';
}
diff --git a/libiberty/testsuite/rust-demangle-expected
b/libiberty/testsuite/rust-demangle-expected
index c3b03f9f02d..74774794736 100644
--- a/libiberty/testsuite/rust-demangle-expected
+++ b/libiberty/testsuite/rust-demangle-expected
@@ -41,7 +41,7 @@ main::main::he714a2e23ed7db2g
# $XX$ substitutions should not contain just numbers.
--format=auto
_ZN4main4$99$17he714a2e23ed7db23E
-main::$99$::he714a2e23ed7db23
+main::$99$
# _ at start of path should be removed.
# ".." translates to "::" "$GT$" to ">" and "$LT$" to "<".
--format=rust
Re: [PATCH] Simplify and generalize rust-demangle's unescaping logic.
Could you, or someone else, commit this for me (as I have no commit access)? Thanks. On Mon, Aug 26, 2019, at 11:04 PM, Ian Lance Taylor wrote: > On Wed, Aug 14, 2019 at 10:24 AM Eduard-Mihai Burtescu wrote: > > > > Previously, rust-demangle.c was special-casing a fixed number > > of '$uXY$' escapes, but 'XY' can technically be any hex value, > > representing some Unicode codepoint. > > > > This patch adds more general support for '$u...$' escapes, > > similar to https://github.com/alexcrichton/rustc-demangle/pull/29, > > but only for the the ASCII subset. More complete Unicode support > > may come at a later time, but right now I want to keep it simple. > > > > Escapes that decode to ASCII control codes are considered invalid, > > as the Rust compiler should never emit them, and to avoid any > > undesirable effects from accidentally outputting a control code. > > > > Additionally, the switch statements, which had one case for each > > alphanumeric character, were replaced with if-else chains. > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > > > 2019-08-14 Eduard-Mihai Burtescu > > libiberty/ChangeLog: > > * rust-demangle.c (unescape): Remove. > > (parse_lower_hex_nibble): New function. > > (parse_legacy_escape): New function. > > (is_prefixed_hash): Use parse_lower_hex_nibble. > > (looks_like_rust): Use parse_legacy_escape. > > (rust_demangle_sym): Use parse_legacy_escape. > > * testsuite/rust-demangle-expected: Add 'llv$u6d$' test. > > This is OK. > > Thanks. > > Ian >
[PING][PATCH] Simplify and generalize rust-demangle's unescaping logic.
Ping: https://gcc.gnu.org/ml/gcc-patches/2019-08/msg01007.html On Wed, Aug 14, 2019, at 8:22 PM, Eduard-Mihai Burtescu wrote: > Previously, rust-demangle.c was special-casing a fixed number > of '$uXY$' escapes, but 'XY' can technically be any hex value, > representing some Unicode codepoint. > > This patch adds more general support for '$u...$' escapes, > similar to https://github.com/alexcrichton/rustc-demangle/pull/29, > but only for the the ASCII subset. More complete Unicode support > may come at a later time, but right now I want to keep it simple. > > Escapes that decode to ASCII control codes are considered invalid, > as the Rust compiler should never emit them, and to avoid any > undesirable effects from accidentally outputting a control code. > > Additionally, the switch statements, which had one case for each > alphanumeric character, were replaced with if-else chains. > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > 2019-08-14 Eduard-Mihai Burtescu > libiberty/ChangeLog: > * rust-demangle.c (unescape): Remove. > (parse_lower_hex_nibble): New function. > (parse_legacy_escape): New function. > (is_prefixed_hash): Use parse_lower_hex_nibble. > (looks_like_rust): Use parse_legacy_escape. > (rust_demangle_sym): Use parse_legacy_escape. > * testsuite/rust-demangle-expected: Add 'llv$u6d$' test.
[PATCH] Simplify and generalize rust-demangle's unescaping logic.
Previously, rust-demangle.c was special-casing a fixed number
of '$uXY$' escapes, but 'XY' can technically be any hex value,
representing some Unicode codepoint.
This patch adds more general support for '$u...$' escapes,
similar to https://github.com/alexcrichton/rustc-demangle/pull/29,
but only for the the ASCII subset. More complete Unicode support
may come at a later time, but right now I want to keep it simple.
Escapes that decode to ASCII control codes are considered invalid,
as the Rust compiler should never emit them, and to avoid any
undesirable effects from accidentally outputting a control code.
Additionally, the switch statements, which had one case for each
alphanumeric character, were replaced with if-else chains.
Bootstrapped and tested on x86_64-unknown-linux-gnu.
2019-08-14 Eduard-Mihai Burtescu
libiberty/ChangeLog:
* rust-demangle.c (unescape): Remove.
(parse_lower_hex_nibble): New function.
(parse_legacy_escape): New function.
(is_prefixed_hash): Use parse_lower_hex_nibble.
(looks_like_rust): Use parse_legacy_escape.
(rust_demangle_sym): Use parse_legacy_escape.
* testsuite/rust-demangle-expected: Add 'llv$u6d$' test.
diff --git a/libiberty/rust-demangle.c b/libiberty/rust-demangle.c
index 2302db45b6f..da591902db1 100644
--- a/libiberty/rust-demangle.c
+++ b/libiberty/rust-demangle.c
@@ -50,7 +50,7 @@ extern void *memset(void *s, int c, size_t n);
#include "rust-demangle.h"
-/* Mangled Rust symbols look like this:
+/* Mangled (legacy) Rust symbols look like this:
_$LT$std..sys..fd..FileDesc$u20$as$u20$core..ops..Drop$GT$::drop::hc68340e1baa4987a
The original symbol is:
@@ -74,16 +74,7 @@ extern void *memset(void *s, int c, size_t n);
">" => $GT$
"(" => $LP$
")" => $RP$
- " " => $u20$
- "\"" => $u22$
- "'" => $u27$
- "+" => $u2b$
- ";" => $u3b$
- "[" => $u5b$
- "]" => $u5d$
- "{" => $u7b$
- "}" => $u7d$
- "~" => $u7e$
+ "\u{XY}" => $uXY$
A double ".." means "::" and a single "." means "-".
@@ -95,7 +86,8 @@ static const size_t hash_len = 16;
static int is_prefixed_hash (const char *start);
static int looks_like_rust (const char *sym, size_t len);
-static int unescape (const char **in, char **out, const char *seq, char value);
+static int parse_lower_hex_nibble (char nibble);
+static char parse_legacy_escape (const char **in);
/* INPUT: sym: symbol that has been through C++ (gnu v3) demangling
@@ -149,7 +141,7 @@ is_prefixed_hash (const char *str)
const char *end;
char seen[16];
size_t i;
- int count;
+ int count, nibble;
if (strncmp (str, hash_prefix, hash_prefix_len))
return 0;
@@ -157,12 +149,12 @@ is_prefixed_hash (const char *str)
memset (seen, 0, sizeof(seen));
for (end = str + hash_len; str < end; str++)
-if (*str >= '0' && *str <= '9')
- seen[*str - '0'] = 1;
-else if (*str >= 'a' && *str <= 'f')
- seen[*str - 'a' + 10] = 1;
-else
- return 0;
+{
+ nibble = parse_lower_hex_nibble (*str);
+ if (nibble < 0)
+return 0;
+ seen[nibble] = 1;
+}
/* Count how many distinct digits seen */
count = 0;
@@ -179,57 +171,17 @@ looks_like_rust (const char *str, size_t len)
const char *end = str + len;
while (str < end)
-switch (*str)
- {
- case '$':
- if (!strncmp (str, "$C$", 3))
- str += 3;
- else if (!strncmp (str, "$SP$", 4)
-|| !strncmp (str, "$BP$", 4)
-|| !strncmp (str, "$RF$", 4)
-|| !strncmp (str, "$LT$", 4)
-|| !strncmp (str, "$GT$", 4)
-|| !strncmp (str, "$LP$", 4)
-|| !strncmp (str, "$RP$", 4))
- str += 4;
- else if (!strncmp (str, "$u20$", 5)
-|| !strncmp (str, "$u22$", 5)
-|| !strncmp (str, "$u27$", 5)
-|| !strncmp (str, "$u2b$", 5)
-|| !strncmp (str, "$u3b$", 5)
-|| !strncmp (str, "$u5b$", 5)
-|| !strncmp (str, "$u5d$", 5)
-|| !strncmp (str, "$u7b$", 5)
-|| !strncmp (str, "$u7d$", 5)
-|| !strncmp (str, "$u7e$", 5))
- str += 5;
- else
- return 0;
- break;
- case '.':
- /* Do not allow three or more consecutive dots */
- if (!strncmp (str, "...", 3))
- return 0;
- /* Fall through */
- case 'a': case 'b': case 'c': case '
Re: [PATCH] Move rust_{is_mangled,demangle_sym} to a private libiberty header.
Pinging this again - while it's a tiny change, I want it to land before I submit anything else in this area. Also, I forgot to mention I have no commit access. Original submission can be found at https://gcc.gnu.org/ml/gcc-patches/2019-06/msg6.html. Thanks, - Eddy B. On Wed, Jun 26, 2019, at 11:54 AM, Eduard-Mihai Burtescu wrote: > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > (Apologies for the delay, while I was able to run libiberty tests back when I > submitted the patch, I wanted to make sure I can run the whole GCC testsuite, > especially for more significant future contributions, so I had to wait until > I had the time to troubleshoot the NixOS support for GCC's make check) > > Thanks, > - Eddy B. > > > On Mon, Jun 3, 2019, at 7:23 AM, Ian Lance Taylor wrote: > > On Sat, Jun 1, 2019 at 7:15 AM Eduard-Mihai Burtescu wrote: > > > > > > 2019-06-01 Eduard-Mihai Burtescu > > > include/ChangeLog: > > > * demangle.h (rust_is_mangled): Move to libiberty/rust-demangle.h. > > > (rust_demangle_sym): Move to libiberty/rust-demangle.h. > > > libiberty/ChangeLog: > > > * cplus-dem.c: Include rust-demangle.h. > > > * rust-demangle.c: Include rust-demangle.h. > > > * rust-demangle.h: New file. > > > > This is OK if it bootstraps and tests pass. > > > > Thanks. > > > > Ian > >
Re: [PATCH] Move rust_{is_mangled,demangle_sym} to a private libiberty header.
Bootstrapped and tested on x86_64-unknown-linux-gnu. (Apologies for the delay, while I was able to run libiberty tests back when I submitted the patch, I wanted to make sure I can run the whole GCC testsuite, especially for more significant future contributions, so I had to wait until I had the time to troubleshoot the NixOS support for GCC's make check) Thanks, - Eddy B. On Mon, Jun 3, 2019, at 7:23 AM, Ian Lance Taylor wrote: > On Sat, Jun 1, 2019 at 7:15 AM Eduard-Mihai Burtescu wrote: > > > > 2019-06-01 Eduard-Mihai Burtescu > > include/ChangeLog: > > * demangle.h (rust_is_mangled): Move to libiberty/rust-demangle.h. > > (rust_demangle_sym): Move to libiberty/rust-demangle.h. > > libiberty/ChangeLog: > > * cplus-dem.c: Include rust-demangle.h. > > * rust-demangle.c: Include rust-demangle.h. > > * rust-demangle.h: New file. > > This is OK if it bootstraps and tests pass. > > Thanks. > > Ian >
Re: [PATCH] Move rust_{is_mangled,demangle_sym} to a private libiberty header.
Hi Mark, Valgrind is definitely on my upstreaming list, alongside GDB, LLDB and Linux perf. You can see the preliminary version here: https://gist.github.com/eddyb/c41a69378750a433767cf53fe2316768 (do not use it yet, I still want to tweak it a bit more before upstreaming it, soon, and I want it to go through the GCC/GDB review process). You'll be able to either modify it, to replace the malloc/realloc/free calls, or use rust_demangle_with_callback and use your own buffer (or directly print the demangling, if that's all you need). Feel free to contact me outside of this list, at this email or on IRC (eddyb on Freenode and OFTC), if you want to further discuss the details of upstreaming the new demangler to Valgrind. - Eddy B. On Tue, Jun 4, 2019, at 11:29 AM, Mark Wielaard wrote: > On Sat, 2019-06-01 at 17:14 +0300, Eduard-Mihai Burtescu wrote: > > When libiberty/rust-demangle.c was initially added, its two exports, > > rust_is_mangled and rust_demangle_sym, made it to include/demangle.h. > > However, these two functions are merely implementation details of > > cplus_demangle and rust_demangle, only the latter should be public. > > > > This is becoming a problem, because the new Rust mangling scheme > > does not fit this "postprocess after C++ demangling" API at all, > > so rust_demangle_sym would forever be stuck supporting only the > > legacy mangling, whereas rust_demangle can easily handle both > > (the new version of which I plan to upstream soon). > > > > I'm hoping that libiberty doesn't have strict backwards-compat > > requirements, so that we can hide these two functions. > > Also, as far as I'm aware, nobody is using them in the wild. > > valgrind uses an embedded copy of the libiberty demangler (slightly > changed to use valgrind's internal memory allocation scheme) which does > use these functions directly: > > https://sourceware.org/git/?p=valgrind.git;a=blob;f=coregrind/m_demangle/demangle.c;hb=HEAD#l153 > But we could of course just include the "private" header instead, when > we next sync up with libiberty. > > We use these functions directly precisely because the rust demangling > scheme is (currently) based on top of the traditional _Z C++ demangling > scheme and we know that it will be done "in place". If there is a new > Rust demangling scheme that doesn't have that property we'll have to > adopt to a different demangling scheme in the future. Any help with > that appreciated. valgrind has been useful for combined c/c++/rust > programs. > > Cheers, > > Mark >
[PATCH] Move rust_{is_mangled,demangle_sym} to a private libiberty header.
When libiberty/rust-demangle.c was initially added, its two exports, rust_is_mangled and rust_demangle_sym, made it to include/demangle.h. However, these two functions are merely implementation details of cplus_demangle and rust_demangle, only the latter should be public. This is becoming a problem, because the new Rust mangling scheme does not fit this "postprocess after C++ demangling" API at all, so rust_demangle_sym would forever be stuck supporting only the legacy mangling, whereas rust_demangle can easily handle both (the new version of which I plan to upstream soon). I'm hoping that libiberty doesn't have strict backwards-compat requirements, so that we can hide these two functions. Also, as far as I'm aware, nobody is using them in the wild. 2019-06-01 Eduard-Mihai Burtescu include/ChangeLog: * demangle.h (rust_is_mangled): Move to libiberty/rust-demangle.h. (rust_demangle_sym): Move to libiberty/rust-demangle.h. libiberty/ChangeLog: * cplus-dem.c: Include rust-demangle.h. * rust-demangle.c: Include rust-demangle.h. * rust-demangle.h: New file. diff --git a/include/demangle.h b/include/demangle.h index f5d9b9e8b..06c32571d 100644 --- a/include/demangle.h +++ b/include/demangle.h @@ -159,24 +159,6 @@ ada_demangle (const char *mangled, int options); extern char * dlang_demangle (const char *mangled, int options); -/* Returns non-zero iff MANGLED is a rust mangled symbol. MANGLED must - already have been demangled through cplus_demangle_v3. If this function - returns non-zero then MANGLED can be demangled (in-place) using - RUST_DEMANGLE_SYM. */ -extern int -rust_is_mangled (const char *mangled); - -/* Demangles SYM (in-place) if RUST_IS_MANGLED returned non-zero for SYM. - If RUST_IS_MANGLED returned zero for SYM then RUST_DEMANGLE_SYM might - replace characters that cannot be demangled with '?' and might truncate - SYM. After calling RUST_DEMANGLE_SYM SYM might be shorter, but never - larger. */ -extern void -rust_demangle_sym (char *sym); - -/* Demangles MANGLED if it was GNU_V3 and then RUST mangled, otherwise - returns NULL. Uses CPLUS_DEMANGLE_V3, RUST_IS_MANGLED and - RUST_DEMANGLE_SYM. Returns a new string that is owned by the caller. */ extern char * rust_demangle (const char *mangled, int options); diff --git a/libiberty/cplus-dem.c b/libiberty/cplus-dem.c index afceed2a1..a39e2bf2e 100644 --- a/libiberty/cplus-dem.c +++ b/libiberty/cplus-dem.c @@ -52,6 +52,7 @@ void * realloc (); #define CURRENT_DEMANGLING_STYLE options #include "libiberty.h" +#include "rust-demangle.h" enum demangling_styles current_demangling_style = auto_demangling; diff --git a/libiberty/rust-demangle.c b/libiberty/rust-demangle.c index 9b2d2dbe6..2302db45b 100644 --- a/libiberty/rust-demangle.c +++ b/libiberty/rust-demangle.c @@ -47,6 +47,7 @@ extern void *memset(void *s, int c, size_t n); #include #include "libiberty.h" +#include "rust-demangle.h" /* Mangled Rust symbols look like this: diff --git a/libiberty/rust-demangle.h b/libiberty/rust-demangle.h new file mode 100644 index 0..abf4c6cde --- /dev/null +++ b/libiberty/rust-demangle.h @@ -0,0 +1,45 @@ +/* Internal demangler interface for the Rust programming language. + Copyright (C) 2016-2019 Free Software Foundation, Inc. + Written by David Tolnay (dtol...@gmail.com). + +This file is part of the libiberty library. +Libiberty is free software; you can redistribute it and/or +modify it under the terms of the GNU Library General Public +License as published by the Free Software Foundation; either +version 2 of the License, or (at your option) any later version. + +In addition to the permissions in the GNU Library General Public +License, the Free Software Foundation gives you unlimited permission +to link the compiled version of this file into combinations with other +programs, and to distribute those combinations without any restriction +coming from the use of this file. (The Library Public License +restrictions do apply in other respects; for example, they cover +modification of the file, and distribution when not linked into a +combined executable.) + +Libiberty is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +Library General Public License for more details. + +You should have received a copy of the GNU Library General Public +License along with libiberty; see the file COPYING.LIB. +If not, see <http://www.gnu.org/licenses/>. */ + +/* This file provides some definitions shared by cplus-dem.c and + rust-demangle.c. It should not be included by any other files. */ + +/* Returns non-zero iff MANGLED is a rust mangled symbol. MANGLED must + already have been demangled through cplus_demangle_v3. If this function + returns non-zero then MANGLED can be demangled
