[c++] Fix DECL_BY_REFERENCE of clone parms

Message ID 20180730154338.xdqau3ltcgjyo4vr@delia
State New
Headers show
Series
  • [c++] Fix DECL_BY_REFERENCE of clone parms
Related show

Commit Message

Tom de Vries July 30, 2018, 3:43 p.m.
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(+)

Comments

Richard Biener July 31, 2018, 9:22 a.m. | #1
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)
Jason Merrill July 31, 2018, 1:19 p.m. | #2
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)
Tom de Vries Oct. 23, 2018, 4:28 p.m. | #3
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
Jakub Jelinek Oct. 23, 2018, 4:30 p.m. | #4
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

Patch

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);
+}