Patchwork relax constraint on integral<->offset conversions (PR53336)

login
register
mail settings
Submitter Paolo Bonzini
Date May 16, 2012, 7:19 a.m.
Message ID <1337152777-8102-1-git-send-email-bonzini@gnu.org>
Download mbox | patch
Permalink /patch/159540/
State New
Headers show

Comments

Paolo Bonzini - May 16, 2012, 7:19 a.m.
OFFSET_TYPE is treated as an integral type for the purpose of conversion
in fold-const.c.  However, the GIMPLE verifier disagrees, leading to
verification errors when a cast from boolean to offset type is gimplified.

Bootstrapped/regtested x86_64-pc-linux-gnu, ok for mainline?

Paolo

2012-05-16  Paolo Bonzini  <bonzini@gnu.org>

	* tree-cfg.c (verify_gimple_assign_unary): Allow conversion from
	non-integer integral types to offset type and vice versa.

2012-05-16  Paolo Bonzini  <bonzini@gnu.org>

	* g++.dg/torture/pr53336.C: New testcase.
Richard Guenther - May 16, 2012, 9:43 a.m.
On Wed, May 16, 2012 at 9:19 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
> OFFSET_TYPE is treated as an integral type for the purpose of conversion
> in fold-const.c.  However, the GIMPLE verifier disagrees, leading to
> verification errors when a cast from boolean to offset type is gimplified.
>
> Bootstrapped/regtested x86_64-pc-linux-gnu, ok for mainline?

Ok.

Thanks,
Richard.

> Paolo
>
> 2012-05-16  Paolo Bonzini  <bonzini@gnu.org>
>
>        * tree-cfg.c (verify_gimple_assign_unary): Allow conversion from
>        non-integer integral types to offset type and vice versa.
>
> 2012-05-16  Paolo Bonzini  <bonzini@gnu.org>
>
>        * g++.dg/torture/pr53336.C: New testcase.
>
> Index: tree-cfg.c
> ===================================================================
> --- tree-cfg.c  (revisione 186903)
> +++ tree-cfg.c  (copia locale)
> @@ -3374,11 +3374,11 @@ verify_gimple_assign_unary (gimple stmt)
>                    || ptrofftype_p (sizetype))))
>          return false;
>
> -       /* Allow conversion from integer to offset type and vice versa.  */
> +       /* Allow conversion from integral to offset type and vice versa.  */
>        if ((TREE_CODE (lhs_type) == OFFSET_TYPE
> -            && TREE_CODE (rhs1_type) == INTEGER_TYPE)
> +            && INTEGRAL_TYPE_P (rhs1_type))
>            || (TREE_CODE (lhs_type) == INTEGER_TYPE
> -               && TREE_CODE (rhs1_type) == OFFSET_TYPE))
> +               && INTEGRAL_TYPE_P (rhs1_type)))
>          return false;
>
>        /* Otherwise assert we are converting between types of the
> Index: testsuite/g++.dg/torture/pr53336.C
> ===================================================================
> --- testsuite/g++.dg/torture/pr53336.C  (revisione 0)
> +++ testsuite/g++.dg/torture/pr53336.C  (revisione 0)
> @@ -0,0 +1,45 @@
> +// { dg-do compile }
> +
> +bool foo();
> +
> +struct C
> +{
> +    C()
> +    {
> +        if (foo())
> +            foo();
> +    }
> +};
> +
> +struct S
> +{
> +    struct dummy
> +    {
> +        int i_;
> +    };
> +    typedef int dummy::*bool_type;
> +
> +    operator bool_type() const
> +    {
> +        return foo() ? &dummy::i_ : 0;
> +    }
> +};
> +
> +int x;
> +
> +struct adaptor
> +{
> +    C c;
> +
> +    virtual void bar()
> +    {
> +        if (S())
> +            x = 0;
> +    }
> +};
> +
> +int main()
> +{
> +    adaptor a;
> +}
> +
Paolo Carlini - May 22, 2012, 10:26 a.m.
On 05/16/2012 11:43 AM, Richard Guenther wrote:
> On Wed, May 16, 2012 at 9:19 AM, Paolo Bonzini<bonzini@gnu.org>  wrote:
>> OFFSET_TYPE is treated as an integral type for the purpose of conversion
>> in fold-const.c.  However, the GIMPLE verifier disagrees, leading to
>> verification errors when a cast from boolean to offset type is gimplified.
>>
>> Bootstrapped/regtested x86_64-pc-linux-gnu, ok for mainline?
I see this patch is now installed and I'm afraid is causing regressions 
in the c++ testsuite: today I see many unexpected fails having to do 
with pointers to members.

Thanks,
Paolo.
Richard Guenther - May 22, 2012, 10:38 a.m.
On Tue, May 22, 2012 at 12:26 PM, Paolo Carlini
<paolo.carlini@oracle.com> wrote:
> On 05/16/2012 11:43 AM, Richard Guenther wrote:
>>
>> On Wed, May 16, 2012 at 9:19 AM, Paolo Bonzini<bonzini@gnu.org>  wrote:
>>>
>>> OFFSET_TYPE is treated as an integral type for the purpose of conversion
>>> in fold-const.c.  However, the GIMPLE verifier disagrees, leading to
>>> verification errors when a cast from boolean to offset type is
>>> gimplified.
>>>
>>> Bootstrapped/regtested x86_64-pc-linux-gnu, ok for mainline?
>
> I see this patch is now installed and I'm afraid is causing regressions in
> the c++ testsuite: today I see many unexpected fails having to do with
> pointers to members.

Can't be this patch though, it only lets more things pass the verifier.
Unless

-       /* Allow conversion from integer to offset type and vice versa.  */
+       /* Allow conversion from integral to offset type and vice versa.  */
       if ((TREE_CODE (lhs_type) == OFFSET_TYPE
-            && TREE_CODE (rhs1_type) == INTEGER_TYPE)
+            && INTEGRAL_TYPE_P (rhs1_type))
           || (TREE_CODE (lhs_type) == INTEGER_TYPE
-               && TREE_CODE (rhs1_type) == OFFSET_TYPE))
+               && INTEGRAL_TYPE_P (rhs1_type)))

the latter looks like a typo - INTEGRAL_TYPE_P should be used for
lhs_type, not rhs1_type.

Richard.

> Thanks,
> Paolo.
Paolo Carlini - May 22, 2012, 11:09 a.m.
On 05/22/2012 12:38 PM, Richard Guenther wrote:
> Can't be this patch though, it only lets more things pass the verifier.
> Unless
>
> -       /* Allow conversion from integer to offset type and vice versa.  */
> +       /* Allow conversion from integral to offset type and vice versa.  */
>         if ((TREE_CODE (lhs_type) == OFFSET_TYPE
> -&&  TREE_CODE (rhs1_type) == INTEGER_TYPE)
> +&&  INTEGRAL_TYPE_P (rhs1_type))
>             || (TREE_CODE (lhs_type) == INTEGER_TYPE
> -&&  TREE_CODE (rhs1_type) == OFFSET_TYPE))
> +&&  INTEGRAL_TYPE_P (rhs1_type)))
>
> the latter looks like a typo - INTEGRAL_TYPE_P should be used for
> lhs_type, not rhs1_type.
Ok, thanks. If nobody beats me later today I will regtest and in case 
commit what you are suggesting.

Paolo.
Richard Guenther - May 22, 2012, 11:16 a.m.
On Tue, May 22, 2012 at 1:09 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> On 05/22/2012 12:38 PM, Richard Guenther wrote:
>>
>> Can't be this patch though, it only lets more things pass the verifier.
>> Unless
>>
>> -       /* Allow conversion from integer to offset type and vice versa.
>>  */
>> +       /* Allow conversion from integral to offset type and vice versa.
>>  */
>>        if ((TREE_CODE (lhs_type) == OFFSET_TYPE
>> -&&  TREE_CODE (rhs1_type) == INTEGER_TYPE)
>> +&&  INTEGRAL_TYPE_P (rhs1_type))
>>
>>            || (TREE_CODE (lhs_type) == INTEGER_TYPE
>> -&&  TREE_CODE (rhs1_type) == OFFSET_TYPE))
>> +&&  INTEGRAL_TYPE_P (rhs1_type)))
>>
>>
>> the latter looks like a typo - INTEGRAL_TYPE_P should be used for
>> lhs_type, not rhs1_type.
>
> Ok, thanks. If nobody beats me later today I will regtest and in case commit
> what you are suggesting.

I think it's quite obvious even ... thus I'll commit it without testing.  Heh.

Richard.

> Paolo.

Patch

Index: tree-cfg.c
===================================================================
--- tree-cfg.c	(revisione 186903)
+++ tree-cfg.c	(copia locale)
@@ -3374,11 +3374,11 @@  verify_gimple_assign_unary (gimple stmt)
 		    || ptrofftype_p (sizetype))))
 	  return false;
 
-	/* Allow conversion from integer to offset type and vice versa.  */
+	/* Allow conversion from integral to offset type and vice versa.  */
 	if ((TREE_CODE (lhs_type) == OFFSET_TYPE
-	     && TREE_CODE (rhs1_type) == INTEGER_TYPE)
+	     && INTEGRAL_TYPE_P (rhs1_type))
 	    || (TREE_CODE (lhs_type) == INTEGER_TYPE
-		&& TREE_CODE (rhs1_type) == OFFSET_TYPE))
+		&& INTEGRAL_TYPE_P (rhs1_type)))
 	  return false;
 
 	/* Otherwise assert we are converting between types of the
Index: testsuite/g++.dg/torture/pr53336.C
===================================================================
--- testsuite/g++.dg/torture/pr53336.C	(revisione 0)
+++ testsuite/g++.dg/torture/pr53336.C	(revisione 0)
@@ -0,0 +1,45 @@ 
+// { dg-do compile }
+
+bool foo();
+
+struct C
+{
+    C()
+    {
+        if (foo())
+            foo();
+    }
+};
+
+struct S
+{
+    struct dummy
+    {
+        int i_;
+    };
+    typedef int dummy::*bool_type;
+
+    operator bool_type() const
+    {
+        return foo() ? &dummy::i_ : 0;
+    }
+};
+
+int x;
+
+struct adaptor
+{
+    C c;
+
+    virtual void bar()
+    {
+        if (S())
+            x = 0;
+    }
+};
+
+int main()
+{
+    adaptor a;
+}
+