diff mbox series

[1/1] PR100281 Fix SImode pointer handling

Message ID 20210428054547.9550-1-krebbel@linux.ibm.com
State New
Headers show
Series [1/1] PR100281 Fix SImode pointer handling | expand

Commit Message

Andreas Krebbel April 28, 2021, 5:45 a.m. UTC
The problem appears to be triggered by two locations in the front-end
where non-POINTER_SIZE pointers aren't handled right now.

1. An assertion in strip_typedefs is triggered because the alignment
of the types don't match. This in turn is caused by creating the new
type with build_pointer_type instead of taking the type of the
original pointer into account.

2. An assertion in cp_convert_to_pointer is triggered which expects
the target type to always have POINTER_SIZE.

Ok for mainline?

gcc/cp/ChangeLog:

	PR c++/100281
	* cvt.c (cp_convert_to_pointer): Use the size of the target
	pointer type.
	* tree.c (strip_typedefs): Use build_pointer_type_for_mode for
	non-POINTER_SIZE pointers.

gcc/testsuite/ChangeLog:

	PR c++/100281
	* g++.target/s390/pr100281.C: New test.
---
 gcc/cp/cvt.c                             |  2 +-
 gcc/cp/tree.c                            |  5 ++++-
 gcc/testsuite/g++.target/s390/pr100281.C | 10 ++++++++++
 3 files changed, 15 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.target/s390/pr100281.C

Comments

Richard Biener April 28, 2021, 8:12 a.m. UTC | #1
On Wed, Apr 28, 2021 at 8:54 AM Andreas Krebbel via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> The problem appears to be triggered by two locations in the front-end
> where non-POINTER_SIZE pointers aren't handled right now.
>
> 1. An assertion in strip_typedefs is triggered because the alignment
> of the types don't match. This in turn is caused by creating the new
> type with build_pointer_type instead of taking the type of the
> original pointer into account.
>
> 2. An assertion in cp_convert_to_pointer is triggered which expects
> the target type to always have POINTER_SIZE.
>
> Ok for mainline?
>
> gcc/cp/ChangeLog:
>
>         PR c++/100281
>         * cvt.c (cp_convert_to_pointer): Use the size of the target
>         pointer type.
>         * tree.c (strip_typedefs): Use build_pointer_type_for_mode for
>         non-POINTER_SIZE pointers.
>
> gcc/testsuite/ChangeLog:
>
>         PR c++/100281
>         * g++.target/s390/pr100281.C: New test.
> ---
>  gcc/cp/cvt.c                             |  2 +-
>  gcc/cp/tree.c                            |  5 ++++-
>  gcc/testsuite/g++.target/s390/pr100281.C | 10 ++++++++++
>  3 files changed, 15 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/g++.target/s390/pr100281.C
>
> diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
> index f1687e804d1..7fa6e8df52b 100644
> --- a/gcc/cp/cvt.c
> +++ b/gcc/cp/cvt.c
> @@ -232,7 +232,7 @@ cp_convert_to_pointer (tree type, tree expr, bool dofold,
>      {
>        if (TYPE_PRECISION (intype) == POINTER_SIZE)
>         return build1 (CONVERT_EXPR, type, expr);
> -      expr = cp_convert (c_common_type_for_size (POINTER_SIZE, 0), expr,
> +      expr = cp_convert (c_common_type_for_size (TYPE_PRECISION (type), 0), expr,
>                          complain);
>        /* Modes may be different but sizes should be the same.  There
>          is supposed to be some integral type that is the same width
> diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
> index a8bfd5fc053..6f6b732c9c9 100644
> --- a/gcc/cp/tree.c
> +++ b/gcc/cp/tree.c
> @@ -1556,7 +1556,10 @@ strip_typedefs (tree t, bool *remove_attributes, unsigned int flags)
>      {
>      case POINTER_TYPE:
>        type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
> -      result = build_pointer_type (type);
> +      if (TYPE_PRECISION (t) == POINTER_SIZE)
> +       result = build_pointer_type (type);
> +      else
> +       result = build_pointer_type_for_mode (type, TYPE_MODE (t), false);

I wonder under which circumstances re-using the original mode will fail?  In
particular I do not like the TYPE_PRECISION check.  Supposedly you
were thinking of playing safe?

>        break;
>      case REFERENCE_TYPE:
>        type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);

There's code below with exactly the same issue for reference types which
would need adjustments to cp_build_reference_type.

> diff --git a/gcc/testsuite/g++.target/s390/pr100281.C b/gcc/testsuite/g++.target/s390/pr100281.C
> new file mode 100644
> index 00000000000..f45798c3879
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/s390/pr100281.C
> @@ -0,0 +1,10 @@
> +// PR C++/100281
> +// { dg-do compile }
> +
> +typedef void * __attribute__((mode (SI))) __ptr32_t;
> +
> +void foo(){
> +  unsigned int b = 100;
> +  __ptr32_t a;
> +  a = b; /* { dg-error "invalid conversion from 'unsigned int' to '__ptr32_t'.*" } */
> +}
> --
> 2.30.2
>
Andreas Krebbel April 28, 2021, 8:22 a.m. UTC | #2
On 4/28/21 10:12 AM, Richard Biener wrote:
> On Wed, Apr 28, 2021 at 8:54 AM Andreas Krebbel via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> The problem appears to be triggered by two locations in the front-end
>> where non-POINTER_SIZE pointers aren't handled right now.
>>
>> 1. An assertion in strip_typedefs is triggered because the alignment
>> of the types don't match. This in turn is caused by creating the new
>> type with build_pointer_type instead of taking the type of the
>> original pointer into account.
>>
>> 2. An assertion in cp_convert_to_pointer is triggered which expects
>> the target type to always have POINTER_SIZE.
>>
>> Ok for mainline?
>>
>> gcc/cp/ChangeLog:
>>
>>         PR c++/100281
>>         * cvt.c (cp_convert_to_pointer): Use the size of the target
>>         pointer type.
>>         * tree.c (strip_typedefs): Use build_pointer_type_for_mode for
>>         non-POINTER_SIZE pointers.
>>
>> gcc/testsuite/ChangeLog:
>>
>>         PR c++/100281
>>         * g++.target/s390/pr100281.C: New test.
>> ---
>>  gcc/cp/cvt.c                             |  2 +-
>>  gcc/cp/tree.c                            |  5 ++++-
>>  gcc/testsuite/g++.target/s390/pr100281.C | 10 ++++++++++
>>  3 files changed, 15 insertions(+), 2 deletions(-)
>>  create mode 100644 gcc/testsuite/g++.target/s390/pr100281.C
>>
>> diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
>> index f1687e804d1..7fa6e8df52b 100644
>> --- a/gcc/cp/cvt.c
>> +++ b/gcc/cp/cvt.c
>> @@ -232,7 +232,7 @@ cp_convert_to_pointer (tree type, tree expr, bool dofold,
>>      {
>>        if (TYPE_PRECISION (intype) == POINTER_SIZE)
>>         return build1 (CONVERT_EXPR, type, expr);
>> -      expr = cp_convert (c_common_type_for_size (POINTER_SIZE, 0), expr,
>> +      expr = cp_convert (c_common_type_for_size (TYPE_PRECISION (type), 0), expr,
>>                          complain);
>>        /* Modes may be different but sizes should be the same.  There
>>          is supposed to be some integral type that is the same width
>> diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
>> index a8bfd5fc053..6f6b732c9c9 100644
>> --- a/gcc/cp/tree.c
>> +++ b/gcc/cp/tree.c
>> @@ -1556,7 +1556,10 @@ strip_typedefs (tree t, bool *remove_attributes, unsigned int flags)
>>      {
>>      case POINTER_TYPE:
>>        type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
>> -      result = build_pointer_type (type);
>> +      if (TYPE_PRECISION (t) == POINTER_SIZE)
>> +       result = build_pointer_type (type);
>> +      else
>> +       result = build_pointer_type_for_mode (type, TYPE_MODE (t), false);
> 
> I wonder under which circumstances re-using the original mode will fail?  In
> particular I do not like the TYPE_PRECISION check.  Supposedly you
> were thinking of playing safe?

Yes. build_pointer_type_for_mode carries some additional logic compared to just build_pointer_type
and I wanted to avoid impacting other targets that way.

> 
>>        break;
>>      case REFERENCE_TYPE:
>>        type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
> 
> There's code below with exactly the same issue for reference types which
> would need adjustments to cp_build_reference_type.

Ok. I'll have a look.

Andreas

> 
>> diff --git a/gcc/testsuite/g++.target/s390/pr100281.C b/gcc/testsuite/g++.target/s390/pr100281.C
>> new file mode 100644
>> index 00000000000..f45798c3879
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.target/s390/pr100281.C
>> @@ -0,0 +1,10 @@
>> +// PR C++/100281
>> +// { dg-do compile }
>> +
>> +typedef void * __attribute__((mode (SI))) __ptr32_t;
>> +
>> +void foo(){
>> +  unsigned int b = 100;
>> +  __ptr32_t a;
>> +  a = b; /* { dg-error "invalid conversion from 'unsigned int' to '__ptr32_t'.*" } */
>> +}
>> --
>> 2.30.2
>>
Andreas Krebbel April 28, 2021, 8:29 a.m. UTC | #3
On 4/28/21 10:22 AM, Andreas Krebbel via Gcc-patches wrote:
> On 4/28/21 10:12 AM, Richard Biener wrote:
>> On Wed, Apr 28, 2021 at 8:54 AM Andreas Krebbel via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>>
>>> The problem appears to be triggered by two locations in the front-end
>>> where non-POINTER_SIZE pointers aren't handled right now.
>>>
>>> 1. An assertion in strip_typedefs is triggered because the alignment
>>> of the types don't match. This in turn is caused by creating the new
>>> type with build_pointer_type instead of taking the type of the
>>> original pointer into account.
>>>
>>> 2. An assertion in cp_convert_to_pointer is triggered which expects
>>> the target type to always have POINTER_SIZE.
>>>
>>> Ok for mainline?
>>>
>>> gcc/cp/ChangeLog:
>>>
>>>         PR c++/100281
>>>         * cvt.c (cp_convert_to_pointer): Use the size of the target
>>>         pointer type.
>>>         * tree.c (strip_typedefs): Use build_pointer_type_for_mode for
>>>         non-POINTER_SIZE pointers.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>         PR c++/100281
>>>         * g++.target/s390/pr100281.C: New test.
>>> ---
>>>  gcc/cp/cvt.c                             |  2 +-
>>>  gcc/cp/tree.c                            |  5 ++++-
>>>  gcc/testsuite/g++.target/s390/pr100281.C | 10 ++++++++++
>>>  3 files changed, 15 insertions(+), 2 deletions(-)
>>>  create mode 100644 gcc/testsuite/g++.target/s390/pr100281.C
>>>
>>> diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
>>> index f1687e804d1..7fa6e8df52b 100644
>>> --- a/gcc/cp/cvt.c
>>> +++ b/gcc/cp/cvt.c
>>> @@ -232,7 +232,7 @@ cp_convert_to_pointer (tree type, tree expr, bool dofold,
>>>      {
>>>        if (TYPE_PRECISION (intype) == POINTER_SIZE)
>>>         return build1 (CONVERT_EXPR, type, expr);
>>> -      expr = cp_convert (c_common_type_for_size (POINTER_SIZE, 0), expr,
>>> +      expr = cp_convert (c_common_type_for_size (TYPE_PRECISION (type), 0), expr,
>>>                          complain);
>>>        /* Modes may be different but sizes should be the same.  There
>>>          is supposed to be some integral type that is the same width
>>> diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
>>> index a8bfd5fc053..6f6b732c9c9 100644
>>> --- a/gcc/cp/tree.c
>>> +++ b/gcc/cp/tree.c
>>> @@ -1556,7 +1556,10 @@ strip_typedefs (tree t, bool *remove_attributes, unsigned int flags)
>>>      {
>>>      case POINTER_TYPE:
>>>        type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
>>> -      result = build_pointer_type (type);
>>> +      if (TYPE_PRECISION (t) == POINTER_SIZE)
>>> +       result = build_pointer_type (type);
>>> +      else
>>> +       result = build_pointer_type_for_mode (type, TYPE_MODE (t), false);
>>
>> I wonder under which circumstances re-using the original mode will fail?  In
>> particular I do not like the TYPE_PRECISION check.  Supposedly you
>> were thinking of playing safe?
> 
> Yes. build_pointer_type_for_mode carries some additional logic compared to just build_pointer_type
> and I wanted to avoid impacting other targets that way.

build_pointer_type just calls build_pointer_type_for_mode. I'll drop the check then and re-test.

> 
>>
>>>        break;
>>>      case REFERENCE_TYPE:
>>>        type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
>>
>> There's code below with exactly the same issue for reference types which
>> would need adjustments to cp_build_reference_type.
> 
> Ok. I'll have a look.
> 
> Andreas
> 
>>
>>> diff --git a/gcc/testsuite/g++.target/s390/pr100281.C b/gcc/testsuite/g++.target/s390/pr100281.C
>>> new file mode 100644
>>> index 00000000000..f45798c3879
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.target/s390/pr100281.C
>>> @@ -0,0 +1,10 @@
>>> +// PR C++/100281
>>> +// { dg-do compile }
>>> +
>>> +typedef void * __attribute__((mode (SI))) __ptr32_t;
>>> +
>>> +void foo(){
>>> +  unsigned int b = 100;
>>> +  __ptr32_t a;
>>> +  a = b; /* { dg-error "invalid conversion from 'unsigned int' to '__ptr32_t'.*" } */
>>> +}
>>> --
>>> 2.30.2
>>>
>
Andreas Krebbel April 28, 2021, 11:51 a.m. UTC | #4
On 4/28/21 10:12 AM, Richard Biener wrote:
> On Wed, Apr 28, 2021 at 8:54 AM Andreas Krebbel via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> The problem appears to be triggered by two locations in the front-end
>> where non-POINTER_SIZE pointers aren't handled right now.
>>
>> 1. An assertion in strip_typedefs is triggered because the alignment
>> of the types don't match. This in turn is caused by creating the new
>> type with build_pointer_type instead of taking the type of the
>> original pointer into account.
>>
>> 2. An assertion in cp_convert_to_pointer is triggered which expects
>> the target type to always have POINTER_SIZE.
>>
>> Ok for mainline?
>>
>> gcc/cp/ChangeLog:
>>
>>         PR c++/100281
>>         * cvt.c (cp_convert_to_pointer): Use the size of the target
>>         pointer type.
>>         * tree.c (strip_typedefs): Use build_pointer_type_for_mode for
>>         non-POINTER_SIZE pointers.
>>
>> gcc/testsuite/ChangeLog:
>>
>>         PR c++/100281
>>         * g++.target/s390/pr100281.C: New test.
>> ---
>>  gcc/cp/cvt.c                             |  2 +-
>>  gcc/cp/tree.c                            |  5 ++++-
>>  gcc/testsuite/g++.target/s390/pr100281.C | 10 ++++++++++
>>  3 files changed, 15 insertions(+), 2 deletions(-)
>>  create mode 100644 gcc/testsuite/g++.target/s390/pr100281.C
>>
>> diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
>> index f1687e804d1..7fa6e8df52b 100644
>> --- a/gcc/cp/cvt.c
>> +++ b/gcc/cp/cvt.c
>> @@ -232,7 +232,7 @@ cp_convert_to_pointer (tree type, tree expr, bool dofold,
>>      {
>>        if (TYPE_PRECISION (intype) == POINTER_SIZE)
>>         return build1 (CONVERT_EXPR, type, expr);
>> -      expr = cp_convert (c_common_type_for_size (POINTER_SIZE, 0), expr,
>> +      expr = cp_convert (c_common_type_for_size (TYPE_PRECISION (type), 0), expr,
>>                          complain);
>>        /* Modes may be different but sizes should be the same.  There
>>          is supposed to be some integral type that is the same width
>> diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
>> index a8bfd5fc053..6f6b732c9c9 100644
>> --- a/gcc/cp/tree.c
>> +++ b/gcc/cp/tree.c
>> @@ -1556,7 +1556,10 @@ strip_typedefs (tree t, bool *remove_attributes, unsigned int flags)
>>      {
>>      case POINTER_TYPE:
>>        type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
>> -      result = build_pointer_type (type);
>> +      if (TYPE_PRECISION (t) == POINTER_SIZE)
>> +       result = build_pointer_type (type);
>> +      else
>> +       result = build_pointer_type_for_mode (type, TYPE_MODE (t), false);
> 
> I wonder under which circumstances re-using the original mode will fail?  In
> particular I do not like the TYPE_PRECISION check.  Supposedly you
> were thinking of playing safe?
> 
>>        break;
>>      case REFERENCE_TYPE:
>>        type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
> 
> There's code below with exactly the same issue for reference types which
> would need adjustments to cp_build_reference_type.
> 
>> diff --git a/gcc/testsuite/g++.target/s390/pr100281.C b/gcc/testsuite/g++.target/s390/pr100281.C
>> new file mode 100644
>> index 00000000000..f45798c3879
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.target/s390/pr100281.C
>> @@ -0,0 +1,10 @@
>> +// PR C++/100281
>> +// { dg-do compile }
>> +
>> +typedef void * __attribute__((mode (SI))) __ptr32_t;
>> +
>> +void foo(){
>> +  unsigned int b = 100;
>> +  __ptr32_t a;
>> +  a = b; /* { dg-error "invalid conversion from 'unsigned int' to '__ptr32_t'.*" } */
>> +}
>> --
>> 2.30.2
>>

Like so?

diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index f1687e804d1..7fa6e8df52b 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
@@ -232,7 +232,7 @@ cp_convert_to_pointer (tree type, tree expr, bool dofold,
     {
       if (TYPE_PRECISION (intype) == POINTER_SIZE)
 	return build1 (CONVERT_EXPR, type, expr);
-      expr = cp_convert (c_common_type_for_size (POINTER_SIZE, 0), expr,
+      expr = cp_convert (c_common_type_for_size (TYPE_PRECISION (type), 0), expr,
 			 complain);
       /* Modes may be different but sizes should be the same.  There
 	 is supposed to be some integral type that is the same width
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index a8bfd5fc053..fe5c414c8d9 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -1201,12 +1201,14 @@ vla_type_p (tree t)
   return false;
 }

-/* Return a reference type node referring to TO_TYPE.  If RVAL is
-   true, return an rvalue reference type, otherwise return an lvalue
-   reference type.  If a type node exists, reuse it, otherwise create
-   a new one.  */
+
+/* Return a reference type node of MODE referring to TO_TYPE.  If MODE
+   is MAX_MACHINE_MODE the standard pointer mode will be picked.  If
+   RVAL is true, return an rvalue reference type, otherwise return an
+   lvalue reference type.  If a type node exists, reuse it, otherwise
+   create a new one.  */
 tree
-cp_build_reference_type (tree to_type, bool rval)
+cp_build_reference_type_for_mode (tree to_type, machine_mode mode, bool rval)
 {
   tree lvalue_ref, t;

@@ -1219,7 +1221,11 @@ cp_build_reference_type (tree to_type, bool rval)
       to_type = TREE_TYPE (to_type);
     }

-  lvalue_ref = build_reference_type (to_type);
+  if (mode == MAX_MACHINE_MODE)
+    lvalue_ref = build_reference_type (to_type);
+  else
+    lvalue_ref = build_reference_type_for_mode (to_type, mode, false);
+
   if (!rval)
     return lvalue_ref;

@@ -1245,7 +1251,7 @@ cp_build_reference_type (tree to_type, bool rval)
     SET_TYPE_STRUCTURAL_EQUALITY (t);
   else if (TYPE_CANONICAL (to_type) != to_type)
     TYPE_CANONICAL (t)
-      = cp_build_reference_type (TYPE_CANONICAL (to_type), rval);
+      = cp_build_reference_type_for_mode (TYPE_CANONICAL (to_type), mode, rval);
   else
     TYPE_CANONICAL (t) = t;

@@ -1255,6 +1261,16 @@ cp_build_reference_type (tree to_type, bool rval)

 }

+/* Return a reference type node referring to TO_TYPE.  If RVAL is
+   true, return an rvalue reference type, otherwise return an lvalue
+   reference type.  If a type node exists, reuse it, otherwise create
+   a new one.  */
+tree
+cp_build_reference_type (tree to_type, bool rval)
+{
+  return cp_build_reference_type_for_mode (to_type, MAX_MACHINE_MODE, rval);
+}
+
 /* Returns EXPR cast to rvalue reference type, like std::move.  */

 tree
@@ -1556,11 +1572,11 @@ strip_typedefs (tree t, bool *remove_attributes, unsigned int flags)
     {
     case POINTER_TYPE:
       type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
-      result = build_pointer_type (type);
+      result = build_pointer_type_for_mode (type, TYPE_MODE (t), false);
       break;
     case REFERENCE_TYPE:
       type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
-      result = cp_build_reference_type (type, TYPE_REF_IS_RVALUE (t));
+      result = cp_build_reference_type_for_mode (type, TYPE_MODE (t), TYPE_REF_IS_RVALUE (t));
       break;
     case OFFSET_TYPE:
       t0 = strip_typedefs (TYPE_OFFSET_BASETYPE (t), remove_attributes, flags);
diff --git a/gcc/testsuite/g++.target/s390/pr100281-1.C b/gcc/testsuite/g++.target/s390/pr100281-1.C
new file mode 100644
index 00000000000..b82e27b64e9
--- /dev/null
+++ b/gcc/testsuite/g++.target/s390/pr100281-1.C
@@ -0,0 +1,10 @@
+// PR C++/100281
+// { dg-do compile }
+
+typedef void * __attribute__((mode (SI))) __ptr32_t;
+
+void foo () {
+  unsigned int b = 100;
+  __ptr32_t a;
+  a = b; /* { dg-error "invalid conversion from 'unsigned int' to '__ptr32_t'.*" } */
+}
diff --git a/gcc/testsuite/g++.target/s390/pr100281-2.C b/gcc/testsuite/g++.target/s390/pr100281-2.C
new file mode 100644
index 00000000000..58552becd7c
--- /dev/null
+++ b/gcc/testsuite/g++.target/s390/pr100281-2.C
@@ -0,0 +1,9 @@
+// PR C++/100281
+// { dg-do compile }
+
+typedef int & __attribute__((mode (SI))) __ref32_t;
+
+void foo () {
+  unsigned int b = 100;
+  __ref32_t a = b; /* { dg-error "cannot bind non-const lvalue reference of type '__ref32_t'.*" } */
+}
Richard Biener April 29, 2021, 10:21 a.m. UTC | #5
On Wed, Apr 28, 2021 at 1:52 PM Andreas Krebbel <krebbel@linux.ibm.com> wrote:
>
> On 4/28/21 10:12 AM, Richard Biener wrote:
> > On Wed, Apr 28, 2021 at 8:54 AM Andreas Krebbel via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> The problem appears to be triggered by two locations in the front-end
> >> where non-POINTER_SIZE pointers aren't handled right now.
> >>
> >> 1. An assertion in strip_typedefs is triggered because the alignment
> >> of the types don't match. This in turn is caused by creating the new
> >> type with build_pointer_type instead of taking the type of the
> >> original pointer into account.
> >>
> >> 2. An assertion in cp_convert_to_pointer is triggered which expects
> >> the target type to always have POINTER_SIZE.
> >>
> >> Ok for mainline?
> >>
> >> gcc/cp/ChangeLog:
> >>
> >>         PR c++/100281
> >>         * cvt.c (cp_convert_to_pointer): Use the size of the target
> >>         pointer type.
> >>         * tree.c (strip_typedefs): Use build_pointer_type_for_mode for
> >>         non-POINTER_SIZE pointers.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >>         PR c++/100281
> >>         * g++.target/s390/pr100281.C: New test.
> >> ---
> >>  gcc/cp/cvt.c                             |  2 +-
> >>  gcc/cp/tree.c                            |  5 ++++-
> >>  gcc/testsuite/g++.target/s390/pr100281.C | 10 ++++++++++
> >>  3 files changed, 15 insertions(+), 2 deletions(-)
> >>  create mode 100644 gcc/testsuite/g++.target/s390/pr100281.C
> >>
> >> diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
> >> index f1687e804d1..7fa6e8df52b 100644
> >> --- a/gcc/cp/cvt.c
> >> +++ b/gcc/cp/cvt.c
> >> @@ -232,7 +232,7 @@ cp_convert_to_pointer (tree type, tree expr, bool dofold,
> >>      {
> >>        if (TYPE_PRECISION (intype) == POINTER_SIZE)
> >>         return build1 (CONVERT_EXPR, type, expr);
> >> -      expr = cp_convert (c_common_type_for_size (POINTER_SIZE, 0), expr,
> >> +      expr = cp_convert (c_common_type_for_size (TYPE_PRECISION (type), 0), expr,
> >>                          complain);
> >>        /* Modes may be different but sizes should be the same.  There
> >>          is supposed to be some integral type that is the same width
> >> diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
> >> index a8bfd5fc053..6f6b732c9c9 100644
> >> --- a/gcc/cp/tree.c
> >> +++ b/gcc/cp/tree.c
> >> @@ -1556,7 +1556,10 @@ strip_typedefs (tree t, bool *remove_attributes, unsigned int flags)
> >>      {
> >>      case POINTER_TYPE:
> >>        type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
> >> -      result = build_pointer_type (type);
> >> +      if (TYPE_PRECISION (t) == POINTER_SIZE)
> >> +       result = build_pointer_type (type);
> >> +      else
> >> +       result = build_pointer_type_for_mode (type, TYPE_MODE (t), false);
> >
> > I wonder under which circumstances re-using the original mode will fail?  In
> > particular I do not like the TYPE_PRECISION check.  Supposedly you
> > were thinking of playing safe?
> >
> >>        break;
> >>      case REFERENCE_TYPE:
> >>        type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
> >
> > There's code below with exactly the same issue for reference types which
> > would need adjustments to cp_build_reference_type.
> >
> >> diff --git a/gcc/testsuite/g++.target/s390/pr100281.C b/gcc/testsuite/g++.target/s390/pr100281.C
> >> new file mode 100644
> >> index 00000000000..f45798c3879
> >> --- /dev/null
> >> +++ b/gcc/testsuite/g++.target/s390/pr100281.C
> >> @@ -0,0 +1,10 @@
> >> +// PR C++/100281
> >> +// { dg-do compile }
> >> +
> >> +typedef void * __attribute__((mode (SI))) __ptr32_t;
> >> +
> >> +void foo(){
> >> +  unsigned int b = 100;
> >> +  __ptr32_t a;
> >> +  a = b; /* { dg-error "invalid conversion from 'unsigned int' to '__ptr32_t'.*" } */
> >> +}
> >> --
> >> 2.30.2
> >>
>
> Like so?

Sth like that - the MAX_MACHINE_MODE thing is a bit ugly
(maybe use VOIDmode?).  Of course should be reviewed by a C++ maintainer.

Richard.

> diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
> index f1687e804d1..7fa6e8df52b 100644
> --- a/gcc/cp/cvt.c
> +++ b/gcc/cp/cvt.c
> @@ -232,7 +232,7 @@ cp_convert_to_pointer (tree type, tree expr, bool dofold,
>      {
>        if (TYPE_PRECISION (intype) == POINTER_SIZE)
>         return build1 (CONVERT_EXPR, type, expr);
> -      expr = cp_convert (c_common_type_for_size (POINTER_SIZE, 0), expr,
> +      expr = cp_convert (c_common_type_for_size (TYPE_PRECISION (type), 0), expr,
>                          complain);
>        /* Modes may be different but sizes should be the same.  There
>          is supposed to be some integral type that is the same width
> diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
> index a8bfd5fc053..fe5c414c8d9 100644
> --- a/gcc/cp/tree.c
> +++ b/gcc/cp/tree.c
> @@ -1201,12 +1201,14 @@ vla_type_p (tree t)
>    return false;
>  }
>
> -/* Return a reference type node referring to TO_TYPE.  If RVAL is
> -   true, return an rvalue reference type, otherwise return an lvalue
> -   reference type.  If a type node exists, reuse it, otherwise create
> -   a new one.  */
> +
> +/* Return a reference type node of MODE referring to TO_TYPE.  If MODE
> +   is MAX_MACHINE_MODE the standard pointer mode will be picked.  If
> +   RVAL is true, return an rvalue reference type, otherwise return an
> +   lvalue reference type.  If a type node exists, reuse it, otherwise
> +   create a new one.  */
>  tree
> -cp_build_reference_type (tree to_type, bool rval)
> +cp_build_reference_type_for_mode (tree to_type, machine_mode mode, bool rval)
>  {
>    tree lvalue_ref, t;
>
> @@ -1219,7 +1221,11 @@ cp_build_reference_type (tree to_type, bool rval)
>        to_type = TREE_TYPE (to_type);
>      }
>
> -  lvalue_ref = build_reference_type (to_type);
> +  if (mode == MAX_MACHINE_MODE)
> +    lvalue_ref = build_reference_type (to_type);
> +  else
> +    lvalue_ref = build_reference_type_for_mode (to_type, mode, false);
> +
>    if (!rval)
>      return lvalue_ref;
>
> @@ -1245,7 +1251,7 @@ cp_build_reference_type (tree to_type, bool rval)
>      SET_TYPE_STRUCTURAL_EQUALITY (t);
>    else if (TYPE_CANONICAL (to_type) != to_type)
>      TYPE_CANONICAL (t)
> -      = cp_build_reference_type (TYPE_CANONICAL (to_type), rval);
> +      = cp_build_reference_type_for_mode (TYPE_CANONICAL (to_type), mode, rval);
>    else
>      TYPE_CANONICAL (t) = t;
>
> @@ -1255,6 +1261,16 @@ cp_build_reference_type (tree to_type, bool rval)
>
>  }
>
> +/* Return a reference type node referring to TO_TYPE.  If RVAL is
> +   true, return an rvalue reference type, otherwise return an lvalue
> +   reference type.  If a type node exists, reuse it, otherwise create
> +   a new one.  */
> +tree
> +cp_build_reference_type (tree to_type, bool rval)
> +{
> +  return cp_build_reference_type_for_mode (to_type, MAX_MACHINE_MODE, rval);
> +}
> +
>  /* Returns EXPR cast to rvalue reference type, like std::move.  */
>
>  tree
> @@ -1556,11 +1572,11 @@ strip_typedefs (tree t, bool *remove_attributes, unsigned int flags)
>      {
>      case POINTER_TYPE:
>        type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
> -      result = build_pointer_type (type);
> +      result = build_pointer_type_for_mode (type, TYPE_MODE (t), false);
>        break;
>      case REFERENCE_TYPE:
>        type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
> -      result = cp_build_reference_type (type, TYPE_REF_IS_RVALUE (t));
> +      result = cp_build_reference_type_for_mode (type, TYPE_MODE (t), TYPE_REF_IS_RVALUE (t));
>        break;
>      case OFFSET_TYPE:
>        t0 = strip_typedefs (TYPE_OFFSET_BASETYPE (t), remove_attributes, flags);
> diff --git a/gcc/testsuite/g++.target/s390/pr100281-1.C b/gcc/testsuite/g++.target/s390/pr100281-1.C
> new file mode 100644
> index 00000000000..b82e27b64e9
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/s390/pr100281-1.C
> @@ -0,0 +1,10 @@
> +// PR C++/100281
> +// { dg-do compile }
> +
> +typedef void * __attribute__((mode (SI))) __ptr32_t;
> +
> +void foo () {
> +  unsigned int b = 100;
> +  __ptr32_t a;
> +  a = b; /* { dg-error "invalid conversion from 'unsigned int' to '__ptr32_t'.*" } */
> +}
> diff --git a/gcc/testsuite/g++.target/s390/pr100281-2.C b/gcc/testsuite/g++.target/s390/pr100281-2.C
> new file mode 100644
> index 00000000000..58552becd7c
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/s390/pr100281-2.C
> @@ -0,0 +1,9 @@
> +// PR C++/100281
> +// { dg-do compile }
> +
> +typedef int & __attribute__((mode (SI))) __ref32_t;
> +
> +void foo () {
> +  unsigned int b = 100;
> +  __ref32_t a = b; /* { dg-error "cannot bind non-const lvalue reference of type '__ref32_t'.*" } */
> +}
> --
> 2.30.2
>
>
diff mbox series

Patch

diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
index f1687e804d1..7fa6e8df52b 100644
--- a/gcc/cp/cvt.c
+++ b/gcc/cp/cvt.c
@@ -232,7 +232,7 @@  cp_convert_to_pointer (tree type, tree expr, bool dofold,
     {
       if (TYPE_PRECISION (intype) == POINTER_SIZE)
 	return build1 (CONVERT_EXPR, type, expr);
-      expr = cp_convert (c_common_type_for_size (POINTER_SIZE, 0), expr,
+      expr = cp_convert (c_common_type_for_size (TYPE_PRECISION (type), 0), expr,
 			 complain);
       /* Modes may be different but sizes should be the same.  There
 	 is supposed to be some integral type that is the same width
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index a8bfd5fc053..6f6b732c9c9 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -1556,7 +1556,10 @@  strip_typedefs (tree t, bool *remove_attributes, unsigned int flags)
     {
     case POINTER_TYPE:
       type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
-      result = build_pointer_type (type);
+      if (TYPE_PRECISION (t) == POINTER_SIZE)
+	result = build_pointer_type (type);
+      else
+	result = build_pointer_type_for_mode (type, TYPE_MODE (t), false);
       break;
     case REFERENCE_TYPE:
       type = strip_typedefs (TREE_TYPE (t), remove_attributes, flags);
diff --git a/gcc/testsuite/g++.target/s390/pr100281.C b/gcc/testsuite/g++.target/s390/pr100281.C
new file mode 100644
index 00000000000..f45798c3879
--- /dev/null
+++ b/gcc/testsuite/g++.target/s390/pr100281.C
@@ -0,0 +1,10 @@ 
+// PR C++/100281
+// { dg-do compile }
+
+typedef void * __attribute__((mode (SI))) __ptr32_t;
+
+void foo(){
+  unsigned int b = 100;
+  __ptr32_t a;
+  a = b; /* { dg-error "invalid conversion from 'unsigned int' to '__ptr32_t'.*" } */
+}