| Message ID | CAO2gOZV=qW=PeEZjp37NFVHMAMwbXaCX-58Oyj=SH0B6+3izmQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
ping... On Tue, Oct 1, 2013 at 1:28 PM, Dehao Chen <dehao@google.com> wrote: > Hi, > > This patch disables the C++ frontend to add " *INTERNAL* " suffix to > maybe_in_charge_destructor/constructor. This is needed because these > functions could be emitted in the debug info, and we would want to > demangle these names. > > Bootstrapped and passed all regression tests. > > OK for trunk? > > Thanks, > Dehao > > gcc/ChangeLog: > > 2013-10-01 Dehao Chen <dehao@google.com> > > * cp/mangle.c (write_special_name_constructor): Remove the > INTERNAL suffix. > > Index: gcc/cp/mangle.c > =================================================================== > --- gcc/cp/mangle.c (revision 202991) > +++ gcc/cp/mangle.c (working copy) > @@ -687,13 +687,6 @@ write_mangled_name (const tree decl, bool top_leve > mangled_name:; > write_string ("_Z"); > write_encoding (decl); > - if (DECL_LANG_SPECIFIC (decl) > - && (DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (decl) > - || DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (decl))) > - /* We need a distinct mangled name for these entities, but > - we should never actually output it. So, we append some > - characters the assembler won't like. */ > - write_string (" *INTERNAL* "); > } > } > > @@ -1654,8 +1647,7 @@ write_identifier (const char *identifier) > Currently, allocating constructors are never used. > > We also need to provide mangled names for the maybe-in-charge > - constructor, so we treat it here too. mangle_decl_string will > - append *INTERNAL* to that, to make sure we never emit it. */ > + constructor, so we treat it here too. */ > > static void > write_special_name_constructor (const tree ctor) > @@ -1682,8 +1674,7 @@ write_special_name_constructor (const tree ctor) > ::= D2 # base object (not-in-charge) destructor > > We also need to provide mangled names for the maybe-incharge > - destructor, so we treat it here too. mangle_decl_string will > - append *INTERNAL* to that, to make sure we never emit it. */ > + destructor, so we treat it here too. */ > > static void > write_special_name_destructor (const tree dtor)
ping^2 Thanks, Dehao On Tue, Oct 1, 2013 at 1:28 PM, Dehao Chen <dehao@google.com> wrote: > Hi, > > This patch disables the C++ frontend to add " *INTERNAL* " suffix to > maybe_in_charge_destructor/constructor. This is needed because these > functions could be emitted in the debug info, and we would want to > demangle these names. > > Bootstrapped and passed all regression tests. > > OK for trunk? > > Thanks, > Dehao > > gcc/ChangeLog: > > 2013-10-01 Dehao Chen <dehao@google.com> > > * cp/mangle.c (write_special_name_constructor): Remove the > INTERNAL suffix. > > Index: gcc/cp/mangle.c > =================================================================== > --- gcc/cp/mangle.c (revision 202991) > +++ gcc/cp/mangle.c (working copy) > @@ -687,13 +687,6 @@ write_mangled_name (const tree decl, bool top_leve > mangled_name:; > write_string ("_Z"); > write_encoding (decl); > - if (DECL_LANG_SPECIFIC (decl) > - && (DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (decl) > - || DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (decl))) > - /* We need a distinct mangled name for these entities, but > - we should never actually output it. So, we append some > - characters the assembler won't like. */ > - write_string (" *INTERNAL* "); > } > } > > @@ -1654,8 +1647,7 @@ write_identifier (const char *identifier) > Currently, allocating constructors are never used. > > We also need to provide mangled names for the maybe-in-charge > - constructor, so we treat it here too. mangle_decl_string will > - append *INTERNAL* to that, to make sure we never emit it. */ > + constructor, so we treat it here too. */ > > static void > write_special_name_constructor (const tree ctor) > @@ -1682,8 +1674,7 @@ write_special_name_constructor (const tree ctor) > ::= D2 # base object (not-in-charge) destructor > > We also need to provide mangled names for the maybe-incharge > - destructor, so we treat it here too. mangle_decl_string will > - append *INTERNAL* to that, to make sure we never emit it. */ > + destructor, so we treat it here too. */ > > static void > write_special_name_destructor (const tree dtor)
This needs a testcase (compile with -dA and use scan-assembler; see other tests in g++.dg/debug/dwarf2). Jason
It's hard to get a testcase without http://gcc.gnu.org/viewcvs/gcc?view=revision&revision=201856 because none of these *INTERNAL* symbols will be emitted in debug info. Thanks, Dehao On Fri, Oct 11, 2013 at 10:55 AM, Jason Merrill <jason@redhat.com> wrote: > This needs a testcase (compile with -dA and use scan-assembler; see other > tests in g++.dg/debug/dwarf2). > > Jason
> It's hard to get a testcase without > http://gcc.gnu.org/viewcvs/gcc?view=revision&revision=201856 because > none of these *INTERNAL* symbols will be emitted in debug info. The original code was in there as a form of assembly-time assertion that the symbol would never get output. Now that you want to emit it in the debug info, the original intent is still valid, but without that extra kruft, the "assertion" is now gone. I think Jason is suggesting that you replace that with a test case, which would explicitly materialize one or more of the inlined functions that would get these mangled names, and then check the assembler output to verify that the (now valid) mangled name doesn't appear anywhere as an assembler label. -cary
On 10/11/2013 01:59 PM, Dehao Chen wrote: > It's hard to get a testcase without > http://gcc.gnu.org/viewcvs/gcc?view=revision&revision=201856 because > none of these *INTERNAL* symbols will be emitted in debug info. Why does that change cause one of these symbols to be emitted? As Cary says, that was done as an assertion. If you're invalidating the assertion, I need more explanation as to why. Jason
On Fri, Oct 18, 2013 at 10:39 AM, Jason Merrill <jason@redhat.com> wrote: > On 10/11/2013 01:59 PM, Dehao Chen wrote: >> >> It's hard to get a testcase without >> http://gcc.gnu.org/viewcvs/gcc?view=revision&revision=201856 because >> none of these *INTERNAL* symbols will be emitted in debug info. > > > Why does that change cause one of these symbols to be emitted? As Cary > says, that was done as an assertion. If you're invalidating the assertion, > I need more explanation as to why. Sorry, pointed to the incorrect patch, it should be http://gcc.gnu.org/viewcvs/gcc?view=revision&revision=201858 These symbols will not be emitted in the symbol table in assembly, but they will be emitted in the debug_info after http://gcc.gnu.org/viewcvs/gcc?view=revision&revision=201858, which emits function's linkage name in debug_info even when it's abstract. The *INTERNAL* assertion I removed is an over-assertion that not only assert for symbol table, but also for debug info. Thanks, Dehao > > Jason >
ping... Thanks, Dehao On Fri, Oct 18, 2013 at 11:06 AM, Dehao Chen <dehao@google.com> wrote: > On Fri, Oct 18, 2013 at 10:39 AM, Jason Merrill <jason@redhat.com> wrote: >> On 10/11/2013 01:59 PM, Dehao Chen wrote: >>> >>> It's hard to get a testcase without >>> http://gcc.gnu.org/viewcvs/gcc?view=revision&revision=201856 because >>> none of these *INTERNAL* symbols will be emitted in debug info. >> >> >> Why does that change cause one of these symbols to be emitted? As Cary >> says, that was done as an assertion. If you're invalidating the assertion, >> I need more explanation as to why. > > Sorry, pointed to the incorrect patch, it should be > http://gcc.gnu.org/viewcvs/gcc?view=revision&revision=201858 > > These symbols will not be emitted in the symbol table in assembly, but > they will be emitted in the debug_info after > http://gcc.gnu.org/viewcvs/gcc?view=revision&revision=201858, which > emits function's linkage name in debug_info even when it's abstract. > The *INTERNAL* assertion I removed is an over-assertion that not only > assert for symbol table, but also for debug info. > > Thanks, > Dehao > >> >> Jason >>
On 10/28/2013 06:12 PM, Dehao Chen wrote:
> ping...
Sorry for the slow response.
If we're actually emitting the name now, we need to give it a name
different from the complete constructor. I suppose it makes sense to go
with C4/D4 as in the decloning patch,
http://gcc.gnu.org/ml/gcc-patches/2007-11/txt00046.txt
(which I've been meaning to revisit before the end of stage 1).
Jason
On Tue, Oct 29, 2013 at 7:58 AM, Jason Merrill <jason@redhat.com> wrote: > On 10/28/2013 06:12 PM, Dehao Chen wrote: >> >> ping... > > > Sorry for the slow response. > > If we're actually emitting the name now, we need to give it a name different > from the complete constructor. I suppose it makes sense to go with C4/D4 as > in the decloning patch, Shall we do it in a separate patch? And I suppose binutils also need to be updated for C4/D4? Thanks, Dehao > > http://gcc.gnu.org/ml/gcc-patches/2007-11/txt00046.txt > > (which I've been meaning to revisit before the end of stage 1). > > Jason >
On 10/29/2013 01:37 PM, Dehao Chen wrote: >> If we're actually emitting the name now, we need to give it a name different >> from the complete constructor. I suppose it makes sense to go with C4/D4 as >> in the decloning patch, > > Shall we do it in a separate patch? And I suppose binutils also need > to be updated for C4/D4? In the same patch, please. And yes, the demangler will need to be updated. Jason
Index: gcc/cp/mangle.c =================================================================== --- gcc/cp/mangle.c (revision 202991) +++ gcc/cp/mangle.c (working copy) @@ -687,13 +687,6 @@ write_mangled_name (const tree decl, bool top_leve mangled_name:; write_string ("_Z"); write_encoding (decl); - if (DECL_LANG_SPECIFIC (decl) - && (DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (decl) - || DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (decl))) - /* We need a distinct mangled name for these entities, but - we should never actually output it. So, we append some - characters the assembler won't like. */ - write_string (" *INTERNAL* "); } } @@ -1654,8 +1647,7 @@ write_identifier (const char *identifier) Currently, allocating constructors are never used. We also need to provide mangled names for the maybe-in-charge - constructor, so we treat it here too. mangle_decl_string will - append *INTERNAL* to that, to make sure we never emit it. */ + constructor, so we treat it here too. */ static void write_special_name_constructor (const tree ctor) @@ -1682,8 +1674,7 @@ write_special_name_constructor (const tree ctor) ::= D2 # base object (not-in-charge) destructor We also need to provide mangled names for the maybe-incharge - destructor, so we treat it here too. mangle_decl_string will - append *INTERNAL* to that, to make sure we never emit it. */ + destructor, so we treat it here too. */ static void write_special_name_destructor (const tree dtor)