Re: [PATCH] Avoid infinite loop with duplicate anonymous union fields

[Bogdan Harjoc <harjoc at gmail dot com>]

On Wed, Aug 1, 2018 at 1:20 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Wed, 1 Aug 2018, Bogdan Harjoc wrote:
>
>> So array[0] < component < array[2], which loops (I removed the gdb p
>> commands for field_array[1] and so on).
>
> Is the key thing here that you end up with DECL_NAME (field) == NULL_TREE,
> but DECL_NAME (field_array[bot]) != NULL_TREE - and in this particular
> case of a bad ordering only, it's possible to loop without either top or
> bot being changed?  (But other details of the DECL_NAME ordering are
> needed to actually get to that particular point.)

Yes, once it enters the "if DECL_NAME (field) == NULL_TREE" body, only
bot can change, and since "DECL_NAME (field_array[bot]) == NULL_TREE"
is false, the inner while never runs, so it skips directly to
"continue;" below with no changes to bot or top. So the function looks
correct, as long as field_array really is qsort'ed if
TYPE_LANG_SPECIFIC is set.

> seen_error () is the idiomatic way of testing whether an error has been
> reported.

The updated patch is attached and includes a test that passes with:

  make check-gcc RUNTESTFLAGS="dg.exp=union-duplicate-field.c"

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 90ae306c9..5fc62d84d 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -2209,7 +2209,11 @@ lookup_field (tree type, tree component)
      find the element.  Otherwise, do a linear search.  TYPE_LANG_SPECIFIC
      will always be set for structures which have many elements.  */
 
-  if (TYPE_LANG_SPECIFIC (type) && TYPE_LANG_SPECIFIC (type)->s)
+  /* Duplicate field checking replaces duplicates with NULL_TREE so
+     TYPE_LANG_SPECIFIC arrays are potentially no longer sorted. In that
+     case just iterate using DECL_CHAIN. */
+
+  if (TYPE_LANG_SPECIFIC (type) && TYPE_LANG_SPECIFIC (type)->s && !seen_error())
     {
       int bot, top, half;
       tree *field_array = &TYPE_LANG_SPECIFIC (type)->s->elts[0];
diff --git a/gcc/testsuite/gcc.dg/union-duplicate-field.c b/gcc/testsuite/gcc.dg/union-duplicate-field.c
new file mode 100644
index 000000000..da9a945d9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/union-duplicate-field.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-std=c99" } */
+
+int a0;
+
+struct S
+{
+    int a1;
+    union {
+        int a0;
+        int a1; /* { dg-error "duplicate member" } */
+        int a2, a3, a4, a5, a6, a7, a8, a9;
+        int a10, a11, a12, a13, a14, a15;
+    };
+};
+
+int f()
+{
+    struct S s;
+    return s.a0;
+}