| Message ID | 20180730154338.xdqau3ltcgjyo4vr@delia |
|---|---|
| State | New |
| Headers | show |
| Series | [c++] Fix DECL_BY_REFERENCE of clone parms | expand |
On Mon, 30 Jul 2018, Tom de Vries wrote: > Hi, > > Consider test.C compiled at -O0 -g: > ... > class string { > public: > string (const char *p) { this->p = p ; } > string (const string &s) { this->p = s.p; } > > private: > const char *p; > }; > > class foo { > public: > foo (string dir_hint) {} > }; > > int > main (void) > { > std::string s = "This is just a string"; > foo bar(s); > return 0; > } > ... > > When parsing foo::foo, the dir_hint parameter gets a DECL_ARG_TYPE of > 'struct string & restrict'. Then during finish_struct, we call > clone_constructors_and_destructors and create clones for foo::foo, and > set the DECL_ARG_TYPE in the same way. > > Later on, during finish_function, cp_genericize is called for the original > foo::foo, which sets the type of parm dir_hint to DECL_ARG_TYPE, and sets > DECL_BY_REFERENCE of dir_hint to 1. > > After that, during maybe_clone_body update_cloned_parm is called with: > ... > (gdb) call debug_generic_expr (parm.typed.type) > struct string & restrict > (gdb) call debug_generic_expr (cloned_parm.typed.type) > struct string > ... > The type of the cloned_parm is then set to the type of parm, but > DECL_BY_REFERENCE is not set. > > When doing cp_genericize for the clone later on, > TREE_ADDRESSABLE (TREE_TYPE ()) is no longer true for the updated type of > the parm, so DECL_BY_REFERENCE is not set there either. > > This patch fixes the problem by copying DECL_BY_REFERENCE in update_cloned_parm. > > Build and reg-tested on x86_64. > > OK for trunk? Thanks for tracking this down. It looks OK to me but please leave Jason and Nathan a day to comment. Otherwise OK for trunk and also for branches after a while. Thanks, Richard. > Thanks, > - Tom > > [c++] Fix DECL_BY_REFERENCE of clone parms > > 2018-07-30 Tom de Vries <tdevries@suse.de> > > PR debug/86687 > * optimize.c (update_cloned_parm): Copy DECL_BY_REFERENCE. > > * g++.dg/guality/pr86687.C: New test. > > --- > gcc/cp/optimize.c | 2 ++ > gcc/testsuite/g++.dg/guality/pr86687.C | 28 ++++++++++++++++++++++++++++ > 2 files changed, 30 insertions(+) > > diff --git a/gcc/cp/optimize.c b/gcc/cp/optimize.c > index 0e9b84ed8a4..3923a5fc6c4 100644 > --- a/gcc/cp/optimize.c > +++ b/gcc/cp/optimize.c > @@ -46,6 +46,8 @@ update_cloned_parm (tree parm, tree cloned_parm, bool first) > /* We may have taken its address. */ > TREE_ADDRESSABLE (cloned_parm) = TREE_ADDRESSABLE (parm); > > + DECL_BY_REFERENCE (cloned_parm) = DECL_BY_REFERENCE (parm); > + > /* The definition might have different constness. */ > TREE_READONLY (cloned_parm) = TREE_READONLY (parm); > > diff --git a/gcc/testsuite/g++.dg/guality/pr86687.C b/gcc/testsuite/g++.dg/guality/pr86687.C > new file mode 100644 > index 00000000000..140a6fce596 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/guality/pr86687.C > @@ -0,0 +1,28 @@ > +// PR debug/86687 > +// { dg-do run } > +// { dg-options "-g" } > + > +class string { > +public: > + string (int p) { this->p = p ; } > + string (const string &s) { this->p = s.p; } > + > + int p; > +}; > + > +class foo { > +public: > + foo (string dir_hint) { > + p = dir_hint.p; // { dg-final { gdb-test . "dir_hint.p" 3 } } > + } > + > + int p; > +}; > + > +int > +main (void) > +{ > + string s = 3; > + foo bar(s); > + return !(bar.p == 3); > +} > >
OK. On Tue, Jul 31, 2018 at 7:22 PM, Richard Biener <rguenther@suse.de> wrote: > On Mon, 30 Jul 2018, Tom de Vries wrote: > >> Hi, >> >> Consider test.C compiled at -O0 -g: >> ... >> class string { >> public: >> string (const char *p) { this->p = p ; } >> string (const string &s) { this->p = s.p; } >> >> private: >> const char *p; >> }; >> >> class foo { >> public: >> foo (string dir_hint) {} >> }; >> >> int >> main (void) >> { >> std::string s = "This is just a string"; >> foo bar(s); >> return 0; >> } >> ... >> >> When parsing foo::foo, the dir_hint parameter gets a DECL_ARG_TYPE of >> 'struct string & restrict'. Then during finish_struct, we call >> clone_constructors_and_destructors and create clones for foo::foo, and >> set the DECL_ARG_TYPE in the same way. >> >> Later on, during finish_function, cp_genericize is called for the original >> foo::foo, which sets the type of parm dir_hint to DECL_ARG_TYPE, and sets >> DECL_BY_REFERENCE of dir_hint to 1. >> >> After that, during maybe_clone_body update_cloned_parm is called with: >> ... >> (gdb) call debug_generic_expr (parm.typed.type) >> struct string & restrict >> (gdb) call debug_generic_expr (cloned_parm.typed.type) >> struct string >> ... >> The type of the cloned_parm is then set to the type of parm, but >> DECL_BY_REFERENCE is not set. >> >> When doing cp_genericize for the clone later on, >> TREE_ADDRESSABLE (TREE_TYPE ()) is no longer true for the updated type of >> the parm, so DECL_BY_REFERENCE is not set there either. >> >> This patch fixes the problem by copying DECL_BY_REFERENCE in update_cloned_parm. >> >> Build and reg-tested on x86_64. >> >> OK for trunk? > > Thanks for tracking this down. It looks OK to me but please leave > Jason and Nathan a day to comment. > > Otherwise OK for trunk and also for branches after a while. > > Thanks, > Richard. > >> Thanks, >> - Tom >> >> [c++] Fix DECL_BY_REFERENCE of clone parms >> >> 2018-07-30 Tom de Vries <tdevries@suse.de> >> >> PR debug/86687 >> * optimize.c (update_cloned_parm): Copy DECL_BY_REFERENCE. >> >> * g++.dg/guality/pr86687.C: New test. >> >> --- >> gcc/cp/optimize.c | 2 ++ >> gcc/testsuite/g++.dg/guality/pr86687.C | 28 ++++++++++++++++++++++++++++ >> 2 files changed, 30 insertions(+) >> >> diff --git a/gcc/cp/optimize.c b/gcc/cp/optimize.c >> index 0e9b84ed8a4..3923a5fc6c4 100644 >> --- a/gcc/cp/optimize.c >> +++ b/gcc/cp/optimize.c >> @@ -46,6 +46,8 @@ update_cloned_parm (tree parm, tree cloned_parm, bool first) >> /* We may have taken its address. */ >> TREE_ADDRESSABLE (cloned_parm) = TREE_ADDRESSABLE (parm); >> >> + DECL_BY_REFERENCE (cloned_parm) = DECL_BY_REFERENCE (parm); >> + >> /* The definition might have different constness. */ >> TREE_READONLY (cloned_parm) = TREE_READONLY (parm); >> >> diff --git a/gcc/testsuite/g++.dg/guality/pr86687.C b/gcc/testsuite/g++.dg/guality/pr86687.C >> new file mode 100644 >> index 00000000000..140a6fce596 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/guality/pr86687.C >> @@ -0,0 +1,28 @@ >> +// PR debug/86687 >> +// { dg-do run } >> +// { dg-options "-g" } >> + >> +class string { >> +public: >> + string (int p) { this->p = p ; } >> + string (const string &s) { this->p = s.p; } >> + >> + int p; >> +}; >> + >> +class foo { >> +public: >> + foo (string dir_hint) { >> + p = dir_hint.p; // { dg-final { gdb-test . "dir_hint.p" 3 } } >> + } >> + >> + int p; >> +}; >> + >> +int >> +main (void) >> +{ >> + string s = 3; >> + foo bar(s); >> + return !(bar.p == 3); >> +} >> >> > > -- > Richard Biener <rguenther@suse.de> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
On 7/31/18 11:22 AM, Richard Biener wrote:
> Otherwise OK for trunk and also for branches after a while.
Jakub,
I just backported this fix to gcc-8-branch and gcc-7-branch.
I noticed that the gcc-6 branch is frozen, and changes require RM
approval. Do you want this fix in gcc-6?
Thanks,
- Tom
On Tue, Oct 23, 2018 at 06:28:27PM +0200, Tom de Vries wrote: > On 7/31/18 11:22 AM, Richard Biener wrote: > > Otherwise OK for trunk and also for branches after a while. > > I just backported this fix to gcc-8-branch and gcc-7-branch. > > I noticed that the gcc-6 branch is frozen, and changes require RM > approval. Do you want this fix in gcc-6? This is ok for gcc-6 now. Thanks. Jakub
diff --git a/gcc/cp/optimize.c b/gcc/cp/optimize.c index 0e9b84ed8a4..3923a5fc6c4 100644 --- a/gcc/cp/optimize.c +++ b/gcc/cp/optimize.c @@ -46,6 +46,8 @@ update_cloned_parm (tree parm, tree cloned_parm, bool first) /* We may have taken its address. */ TREE_ADDRESSABLE (cloned_parm) = TREE_ADDRESSABLE (parm); + DECL_BY_REFERENCE (cloned_parm) = DECL_BY_REFERENCE (parm); + /* The definition might have different constness. */ TREE_READONLY (cloned_parm) = TREE_READONLY (parm); diff --git a/gcc/testsuite/g++.dg/guality/pr86687.C b/gcc/testsuite/g++.dg/guality/pr86687.C new file mode 100644 index 00000000000..140a6fce596 --- /dev/null +++ b/gcc/testsuite/g++.dg/guality/pr86687.C @@ -0,0 +1,28 @@ +// PR debug/86687 +// { dg-do run } +// { dg-options "-g" } + +class string { +public: + string (int p) { this->p = p ; } + string (const string &s) { this->p = s.p; } + + int p; +}; + +class foo { +public: + foo (string dir_hint) { + p = dir_hint.p; // { dg-final { gdb-test . "dir_hint.p" 3 } } + } + + int p; +}; + +int +main (void) +{ + string s = 3; + foo bar(s); + return !(bar.p == 3); +}
Hi, Consider test.C compiled at -O0 -g: ... class string { public: string (const char *p) { this->p = p ; } string (const string &s) { this->p = s.p; } private: const char *p; }; class foo { public: foo (string dir_hint) {} }; int main (void) { std::string s = "This is just a string"; foo bar(s); return 0; } ... When parsing foo::foo, the dir_hint parameter gets a DECL_ARG_TYPE of 'struct string & restrict'. Then during finish_struct, we call clone_constructors_and_destructors and create clones for foo::foo, and set the DECL_ARG_TYPE in the same way. Later on, during finish_function, cp_genericize is called for the original foo::foo, which sets the type of parm dir_hint to DECL_ARG_TYPE, and sets DECL_BY_REFERENCE of dir_hint to 1. After that, during maybe_clone_body update_cloned_parm is called with: ... (gdb) call debug_generic_expr (parm.typed.type) struct string & restrict (gdb) call debug_generic_expr (cloned_parm.typed.type) struct string ... The type of the cloned_parm is then set to the type of parm, but DECL_BY_REFERENCE is not set. When doing cp_genericize for the clone later on, TREE_ADDRESSABLE (TREE_TYPE ()) is no longer true for the updated type of the parm, so DECL_BY_REFERENCE is not set there either. This patch fixes the problem by copying DECL_BY_REFERENCE in update_cloned_parm. Build and reg-tested on x86_64. OK for trunk? Thanks, - Tom [c++] Fix DECL_BY_REFERENCE of clone parms 2018-07-30 Tom de Vries <tdevries@suse.de> PR debug/86687 * optimize.c (update_cloned_parm): Copy DECL_BY_REFERENCE. * g++.dg/guality/pr86687.C: New test. --- gcc/cp/optimize.c | 2 ++ gcc/testsuite/g++.dg/guality/pr86687.C | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+)