Re: [C23 PATCH, v2] fix aliasing for structures/unions with incomplete types

2024-05-28 Thread Joseph Myers
On Sun, 26 May 2024, Martin Uecker wrote:

> This is the patch I sent previously, but I tried to improve the
> description and added a long comment.  This patch is needed so
> that we do not have to update TYPE_CANONICAL of structures / unions
> when a tagged type is completed that is (recursively) pointed to 
> by a member of the structure / union.
> 
> Bootstrapped and regression tested on x86_64.
> 
> 
> C23: fix aliasing for structures/unions with incomplete types
> 
> When incomplete structure/union types are completed later, compatibility
> of struct types that contain pointers to such types changes.  When forming
> equivalence classes for TYPE_CANONICAL, we therefor need to be 
> conservative
> and treat all structs with the same tag which are pointer targets as
> equivalent for purposed of determining equivalency of structure/union
> types which contain such types as member. This avoids having to update
> TYPE_CANONICAL of such structure/unions recursively. The pointer types
> themselves are updated in c_update_type_canonical.
> 
> gcc/c/
> * c-typeck.cc (comptypes_internal): Add flag to track
> whether a struct is the target of a pointer.
> (tagged_types_tu_compatible): When forming equivalence
> classes, treat nested pointed-to structs as equivalent.
> 
> gcc/testsuite/
> * gcc.dg/c23-tag-incomplete-alias-1.c: New test.

This patch is OK.

-- 
Joseph S. Myers
josmy...@redhat.com

[C23 PATCH, v2] fix aliasing for structures/unions with incomplete types

2024-05-26 Thread Martin Uecker


This is the patch I sent previously, but I tried to improve the
description and added a long comment.  This patch is needed so
that we do not have to update TYPE_CANONICAL of structures / unions
when a tagged type is completed that is (recursively) pointed to 
by a member of the structure / union.

Bootstrapped and regression tested on x86_64.


C23: fix aliasing for structures/unions with incomplete types

When incomplete structure/union types are completed later, compatibility
of struct types that contain pointers to such types changes.  When forming
equivalence classes for TYPE_CANONICAL, we therefor need to be conservative
and treat all structs with the same tag which are pointer targets as
equivalent for purposed of determining equivalency of structure/union
types which contain such types as member. This avoids having to update
TYPE_CANONICAL of such structure/unions recursively. The pointer types
themselves are updated in c_update_type_canonical.

gcc/c/
* c-typeck.cc (comptypes_internal): Add flag to track
whether a struct is the target of a pointer.
(tagged_types_tu_compatible): When forming equivalence
classes, treat nested pointed-to structs as equivalent.

gcc/testsuite/
* gcc.dg/c23-tag-incomplete-alias-1.c: New test.

diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index c07e2f2b5cf..7a14ef1868f 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -1194,6 +1194,7 @@ struct comptypes_data {
   bool different_types_p;
   bool warning_needed;
   bool anon_field;
+  bool pointedto;
   bool equiv;
 
   const struct tagged_tu_seen_cache* cache;
@@ -1265,8 +1266,36 @@ comptypes_check_different_types (tree type1, tree type2,
 }
 
 
-/* Like comptypes, but if it returns nonzero for struct and union
-   types considered equivalent for aliasing purposes.  */
+/* Like comptypes, but if it returns true for struct and union types
+   considered equivalent for aliasing purposes, i.e. for setting
+   TYPE_CANONICAL after completing a struct or union.
+
+   This function must return false only for types which are not
+   compatible according to C language semantics (cf. comptypes),
+   otherwise the middle-end would make incorrect aliasing decisions.
+   It may return true for some similar types that are not compatible
+   according to those stricter rules.
+
+   In particular, we ignore size expression in arrays so that the
+   following structs are in the same equivalence class:
+
+   struct foo { char (*buf)[]; };
+   struct foo { char (*buf)[3]; };
+   struct foo { char (*buf)[4]; };
+
+   We also treat unions / structs with members which are pointers to
+   structures or unions with the same tag as equivalent (if they are not
+   incompatible for other reasons).  Although incomplete structure
+   or union types are not compatible to any other type, they may become
+   compatible to different types when completed.  To avoid having to update
+   TYPE_CANONICAL at this point, we only consider the tag when forming
+   the equivalence classes.  For example, the following types with tag
+   'foo' are all considered equivalent:
+
+   struct bar;
+   struct foo { struct bar *x };
+   struct foo { struct bar { int a; } *x };
+   struct foo { struct bar { char b; } *x };  */
 
 bool
 comptypes_equiv_p (tree type1, tree type2)
@@ -1391,6 +1420,7 @@ comptypes_internal (const_tree type1, const_tree type2,
   /* Do not remove mode information.  */
   if (TYPE_MODE (t1) != TYPE_MODE (t2))
return false;
+  data->pointedto = true;
   return comptypes_internal (TREE_TYPE (t1), TREE_TYPE (t2), data);
 
 case FUNCTION_TYPE:
@@ -1409,7 +1439,7 @@ comptypes_internal (const_tree type1, const_tree type2,
 
if ((d1 == NULL_TREE) != (d2 == NULL_TREE))
  data->different_types_p = true;
-   /* Ignore size mismatches.  */
+   /* Ignore size mismatches when forming equivalence classes.  */
if (data->equiv)
  return true;
/* Sizes must match unless one is missing or variable.  */
@@ -1549,6 +1579,12 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree 
t2,
   if (TYPE_NAME (t1) != TYPE_NAME (t2))
 return false;
 
+  /* When forming equivalence classes for TYPE_CANONICAL in C23, we treat
+ structs with the same tag as equivalent, but only when they are targets
+ of pointers inside other structs.  */
+  if (data->equiv && data->pointedto)
+return true;
+
   if (!data->anon_field && NULL_TREE == TYPE_NAME (t1))
 return false;
 
@@ -1641,6 +1677,7 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree 
t2,
  return false;
 
data->anon_field = !DECL_NAME (s1);
+   data->pointedto = false;
 
data->cache = 
if (!comptypes_internal (TREE_TYPE (s1), TREE_TYPE (s2), data))
diff --git a/gcc/testsuite/gcc.dg/c23-tag-incomplete-alias-1.c