| Message ID | 5b9595ca-c515-4379-be39-65b6fb5a7405@www.fastmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Move rust_{is_mangled,demangle_sym} to a private libiberty header. | expand |
On Sat, Jun 1, 2019 at 7:15 AM Eduard-Mihai Burtescu <eddyb@lyken.rs> wrote: > > 2019-06-01 Eduard-Mihai Burtescu <eddyb@lyken.rs> > 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
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
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 >
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 <eddyb@lyken.rs> wrote: > > > > 2019-06-01 Eduard-Mihai Burtescu <eddyb@lyken.rs> > > 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 >
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/msg00006.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 <eddyb@lyken.rs> wrote: > > > > > > 2019-06-01 Eduard-Mihai Burtescu <eddyb@lyken.rs> > > > 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 > >
"Eduard-Mihai Burtescu" <eddyb@lyken.rs> writes: > 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/msg00006.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 <eddyb@lyken.rs> wrote: >> > > >> > > 2019-06-01 Eduard-Mihai Burtescu <eddyb@lyken.rs> >> > > 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 >> > Can someone volunteer to commit this patch? Thanks. Ian
On Thu, Jul 18, 2019 at 07:04:05AM -0700, Ian Lance Taylor wrote: > >> 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 <eddyb@lyken.rs> wrote: > >> > > > >> > > 2019-06-01 Eduard-Mihai Burtescu <eddyb@lyken.rs> > >> > > 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 > >> > > > > Can someone volunteer to commit this patch? Thanks. Done. Jakub
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 <demangle.h> #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 000000000..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 (dtolnay@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 (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);