Re: C PATCH for comptypes handling of TYPE_REF_CAN_ALIAS_ALL

[Joseph Myers]

On Mon, 6 Jun 2016, Marek Polacek wrote:

> > I don't see how this test is supposed to verify properties of the 
> > composite type.  I'd expect you to need to verify that something does not 
> > get optimized away, that would get optimized away in the absence of 
> > may_alias.
> 
> Well, were it not for the may_alias attribute, we'd warn about type punning
> (hence the -O2), so I thought that this test would be enough.

In that case, the patch is OK.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: C PATCH for comptypes handling of TYPE_REF_CAN_ALIAS_ALL

[Marek Polacek]

On Mon, Jun 06, 2016 at 03:06:18PM +, Joseph Myers wrote:
> On Tue, 31 May 2016, Marek Polacek wrote:
> 
> > > diff --git gcc/testsuite/c-c++-common/attr-may-alias-1.c 
> > > gcc/testsuite/c-c++-common/attr-may-alias-1.c
> > > index e69de29..978b9a5 100644
> > > --- gcc/testsuite/c-c++-common/attr-may-alias-1.c
> > > +++ gcc/testsuite/c-c++-common/attr-may-alias-1.c
> > > @@ -0,0 +1,26 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-O2 -Wall" } */
> > > +
> > > +typedef int T __attribute__((may_alias));
> > > +
> > > +extern T *p;
> > > +extern int *p;
> > > +
> > > +extern int *p2;
> > > +extern T *p2;
> > > +
> > > +void fn1 (T);
> > > +void fn1 (int);
> > > +
> > > +void fn2 (int);
> > > +void fn2 (T);
> > > +
> > > +/* Ensure that the composite types have may_alias.  */
> > > +void
> > > +f (long *i)
> > > +{
> > > +  *i = *(__typeof (*p) *) 
> > > +  asm ("" : : "r" (*p));
> > > +  *i = *(__typeof (*p2) *) 
> > > +  asm ("" : : "r" (*p2));
> 
> I don't see how this test is supposed to verify properties of the 
> composite type.  I'd expect you to need to verify that something does not 
> get optimized away, that would get optimized away in the absence of 
> may_alias.

Well, were it not for the may_alias attribute, we'd warn about type punning
(hence the -O2), so I thought that this test would be enough.

Marek

Re: C PATCH for comptypes handling of TYPE_REF_CAN_ALIAS_ALL

[Joseph Myers]

On Tue, 31 May 2016, Marek Polacek wrote:

> > diff --git gcc/testsuite/c-c++-common/attr-may-alias-1.c 
> > gcc/testsuite/c-c++-common/attr-may-alias-1.c
> > index e69de29..978b9a5 100644
> > --- gcc/testsuite/c-c++-common/attr-may-alias-1.c
> > +++ gcc/testsuite/c-c++-common/attr-may-alias-1.c
> > @@ -0,0 +1,26 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -Wall" } */
> > +
> > +typedef int T __attribute__((may_alias));
> > +
> > +extern T *p;
> > +extern int *p;
> > +
> > +extern int *p2;
> > +extern T *p2;
> > +
> > +void fn1 (T);
> > +void fn1 (int);
> > +
> > +void fn2 (int);
> > +void fn2 (T);
> > +
> > +/* Ensure that the composite types have may_alias.  */
> > +void
> > +f (long *i)
> > +{
> > +  *i = *(__typeof (*p) *) 
> > +  asm ("" : : "r" (*p));
> > +  *i = *(__typeof (*p2) *) 
> > +  asm ("" : : "r" (*p2));

I don't see how this test is supposed to verify properties of the 
composite type.  I'd expect you to need to verify that something does not 
get optimized away, that would get optimized away in the absence of 
may_alias.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: C PATCH for comptypes handling of TYPE_REF_CAN_ALIAS_ALL

[Marek Polacek]

Sorry, gcc-patches fell out of CC.

On Tue, May 31, 2016 at 03:14:43PM +0200, Marek Polacek wrote:
> On Thu, May 26, 2016 at 05:16:39PM +, Joseph Myers wrote:
> > On Thu, 26 May 2016, Marek Polacek wrote:
> > 
> > > The C++ FE has been changed, as a part of c++/50800, in such a way that 
> > > it no
> > > longer considers types differentiating only in TYPE_REF_CAN_ALIAS_ALL
> > > incompatible.  But the C FE still rejects the following testcase, so this 
> > > patch
> > > makes the C FE follow suit.  After all, the may_alias attribute is not
> > > considered as "affects_type_identity".  This TYPE_REF_CAN_ALIAS_ALL check 
> > > was
> > > introduced back in 2004 (r90078), but since then we've gotten rid of 
> > > them, only
> > > comptypes_internal retained it.  I suspect the TYPE_MODE check might go 
> > > too,
> > > but I don't feel like changing that right now.
> > > 
> > > This arised when discussing struct sockaddr vs. may_alias issue in glibc.
> > > 
> > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > 
> > I'd expect you to need to do something about composite types, to ensure 
> > that the composite type has TYPE_REF_CAN_ALIAS_ALL set if either of the 
> > two types does - along with tests in such a case, where the two types are 
> > in either order, that the composite type produced really is treated as 
> > may_alias.  (The sort of cases I'm thinking of are
> > 
> > typedef int T __attribute__((may_alias));
> > extern T *p;
> > extern int *p;
> > 
> > with the declarations in either order, and then making sure that 
> > type-based aliasing handles references through this pointer properly.)
> 
> I investigated this and it turned out the problem is not in composite_types,
> but still in comptypes.  For the example above, 'T' and 'int' have different
> TYPE_MAIN_VARIANT so they were deemed incompatible.  This eventually led me
> back to  and to
> .  The conclusion
> there was that we should rely on canonical types to compare INTEGER_TYPE,
> FIXED_POINT_TYPE, and REAL_TYPE nodes -- canonical types of 'T' and 'int' are
> the same.  Since the C++ FE does exactly this, I just used the same code and
> commentary.  Now, this regressed gcc.dg/pr39464.c, but cc1plus doesn't warn
> on that code either, so that might not be a big deal.  With this patch cc1
> behaves more like cc1plus so I put the new tests into c-c++-common/.  I also
> verified that the composite types do have the may_alias property (using 
> typeof).
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
> 
> 2016-05-31  Marek Polacek  
> 
>   * c-typeck.c (comptypes_internal): Handle comparisons of
>   INTEGER_TYPE, FIXED_POINT_TYPE, and REAL_TYPE nodes.  Don't check
>   TYPE_REF_CAN_ALIAS_ALL.
> 
>   * c-c++-common/attr-may-alias-1.c: New test.
>   * c-c++-common/attr-may-alias-2.c: New test.
>   * gcc.dg/pr39464.c: Turn dg-warning into dg-bogus.
> 
> diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
> index 1520c20..a7f28cc 100644
> --- gcc/c/c-typeck.c
> +++ gcc/c/c-typeck.c
> @@ -1105,10 +1105,28 @@ comptypes_internal (const_tree type1, const_tree 
> type2, bool *enum_and_int_p,
>  
>switch (TREE_CODE (t1))
>  {
> +case INTEGER_TYPE:
> +case FIXED_POINT_TYPE:
> +case REAL_TYPE:
> +  /* With these nodes, we can't determine type equivalence by
> +  looking at what is stored in the nodes themselves, because
> +  two nodes might have different TYPE_MAIN_VARIANTs but still
> +  represent the same type.  For example, wchar_t and int could
> +  have the same properties (TYPE_PRECISION, TYPE_MIN_VALUE,
> +  TYPE_MAX_VALUE, etc.), but have different TYPE_MAIN_VARIANTs
> +  and are distinct types.  On the other hand, int and the
> +  following typedef
> +
> +typedef int INT __attribute((may_alias));
> +
> +  have identical properties, different TYPE_MAIN_VARIANTs, but
> +  represent the same type.  The canonical type system keeps
> +  track of equivalence in this case, so we fall back on it.  */
> +  return TYPE_CANONICAL (t1) == TYPE_CANONICAL (t2);
> +
>  case POINTER_TYPE:
> -  /* Do not remove mode or aliasing information.  */
> -  if (TYPE_MODE (t1) != TYPE_MODE (t2)
> -   || TYPE_REF_CAN_ALIAS_ALL (t1) != TYPE_REF_CAN_ALIAS_ALL (t2))
> +  /* Do not remove mode information.  */
> +  if (TYPE_MODE (t1) != TYPE_MODE (t2))
>   break;
>val = (TREE_TYPE (t1) == TREE_TYPE (t2)
>? 1 : comptypes_internal (TREE_TYPE (t1), TREE_TYPE (t2),
> diff --git gcc/testsuite/c-c++-common/attr-may-alias-1.c 
> gcc/testsuite/c-c++-common/attr-may-alias-1.c
> index e69de29..978b9a5 100644
> --- gcc/testsuite/c-c++-common/attr-may-alias-1.c
> +++ gcc/testsuite/c-c++-common/attr-may-alias-1.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wall" } */
> +
> 

Re: C PATCH for comptypes handling of TYPE_REF_CAN_ALIAS_ALL

[Joseph Myers]

On Thu, 26 May 2016, Marek Polacek wrote:

> The C++ FE has been changed, as a part of c++/50800, in such a way that it no
> longer considers types differentiating only in TYPE_REF_CAN_ALIAS_ALL
> incompatible.  But the C FE still rejects the following testcase, so this 
> patch
> makes the C FE follow suit.  After all, the may_alias attribute is not
> considered as "affects_type_identity".  This TYPE_REF_CAN_ALIAS_ALL check was
> introduced back in 2004 (r90078), but since then we've gotten rid of them, 
> only
> comptypes_internal retained it.  I suspect the TYPE_MODE check might go too,
> but I don't feel like changing that right now.
> 
> This arised when discussing struct sockaddr vs. may_alias issue in glibc.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?

I'd expect you to need to do something about composite types, to ensure 
that the composite type has TYPE_REF_CAN_ALIAS_ALL set if either of the 
two types does - along with tests in such a case, where the two types are 
in either order, that the composite type produced really is treated as 
may_alias.  (The sort of cases I'm thinking of are

typedef int T __attribute__((may_alias));
extern T *p;
extern int *p;

with the declarations in either order, and then making sure that 
type-based aliasing handles references through this pointer properly.)

-- 
Joseph S. Myers
jos...@codesourcery.com

C PATCH for comptypes handling of TYPE_REF_CAN_ALIAS_ALL

[Marek Polacek]

The C++ FE has been changed, as a part of c++/50800, in such a way that it no
longer considers types differentiating only in TYPE_REF_CAN_ALIAS_ALL
incompatible.  But the C FE still rejects the following testcase, so this patch
makes the C FE follow suit.  After all, the may_alias attribute is not
considered as "affects_type_identity".  This TYPE_REF_CAN_ALIAS_ALL check was
introduced back in 2004 (r90078), but since then we've gotten rid of them, only
comptypes_internal retained it.  I suspect the TYPE_MODE check might go too,
but I don't feel like changing that right now.

This arised when discussing struct sockaddr vs. may_alias issue in glibc.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2016-05-26  Marek Polacek  

* c-typeck.c (comptypes_internal): Don't check TYPE_REF_CAN_ALIAS_ALL.

* gcc.dg/attr-may-alias-2.c: New test.

diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 1520c20..02f2cf8 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -1106,9 +1106,8 @@ comptypes_internal (const_tree type1, const_tree type2, 
bool *enum_and_int_p,
   switch (TREE_CODE (t1))
 {
 case POINTER_TYPE:
-  /* Do not remove mode or aliasing information.  */
-  if (TYPE_MODE (t1) != TYPE_MODE (t2)
- || TYPE_REF_CAN_ALIAS_ALL (t1) != TYPE_REF_CAN_ALIAS_ALL (t2))
+  /* Do not remove mode information.  */
+  if (TYPE_MODE (t1) != TYPE_MODE (t2))
break;
   val = (TREE_TYPE (t1) == TREE_TYPE (t2)
 ? 1 : comptypes_internal (TREE_TYPE (t1), TREE_TYPE (t2),
diff --git gcc/testsuite/gcc.dg/attr-may-alias-2.c 
gcc/testsuite/gcc.dg/attr-may-alias-2.c
index e69de29..892748e 100644
--- gcc/testsuite/gcc.dg/attr-may-alias-2.c
+++ gcc/testsuite/gcc.dg/attr-may-alias-2.c
@@ -0,0 +1,13 @@
+/* We used to reject this because types differentiating only in
+   TYPE_REF_CAN_ALIAS_ALL were deemed incompatible.  */
+/* { dg-do compile } */
+
+struct sockaddr;
+struct sockaddr *f (void);
+
+struct __attribute__((may_alias)) sockaddr { int j; };
+struct sockaddr *
+f (void)
+{
+  return (void *) 0;
+}

Marek