[PATCH] Avoid infinite loop with duplicate anonymous union fields

[Bogdan Harjoc]

#define foo(a) did it, thanks!

As Joseph suspected, the hang/no hang result depended on the values of
DECL_NAME pointers:

- with #define foo(a) plus the testcase from bugzilla id 86690 and no
-save-temps, the "return s.a" that triggers lookup_field() will find
the sorted field_array containing:

(gdb) p component
$1 = (tree) 0x7ffff6300050

(gdb) p field_array[0].decl_minimal.name
$0 = (tree) 0x7ffff61cfd70
$1 = (tree) 0x0
$2 = (tree) 0x7ffff63000f0
$3 = (tree) 0x7ffff6300140
$4 = (tree) 0x7ffff6300190
$5 = (tree) 0x7ffff63001e0
$6 = (tree) 0x7ffff6300230
$7 = (tree) 0x7ffff6300280
$8 = (tree) 0x7ffff63002d0
$9 = (tree) 0x7ffff6300320

So array[0] < component < array[2], which loops (I removed the gdb p
commands for field_array[1] and so on).

- with same test-case and with -save-temps I get:

(gdb) p component
$1 = (tree) 0x7ffff6300c30

(gdb) p field_array[0].decl_minimal.name
$0 = (tree) 0x7ffff61cfd70
$1 = (tree) 0x0
$2 = (tree) 0x7ffff6300050
$3 = (tree) 0x7ffff63000a0
$4 = (tree) 0x7ffff63000f0
$5 = (tree) 0x7ffff6300140
$6 = (tree) 0x7ffff6300190
$7 = (tree) 0x7ffff63001e0
$8 = (tree) 0x7ffff6300230
$9 = (tree) 0x7ffff6300280

So component > array[9], and in this case the binary search doesn't
end up with bottom=0 and top=2, where it would hang earlier.

Component here is the s.a field, and with -save-temps, cc1 gets bin.i
as input (which it treats as preprocessed due to the extension)
instead of bin.c. So with preprocessed/unprocessed source, the order
in which builtin/user-defined names are allocated changes, resulting
in a hang or no-hang result.

I propose this testcase for gcc/testsuite/gcc.dg/ as it reproduces
with or without -save-temps, and with no other magic 'scanf'
identifiers because it keeps a0 first in the sorted array and a1
(which will be replaced with NULL) second:

int a0;

struct S
{
    int a1;
    union {
        int a0;
        int a1;
        int a2, a3, a4, a5, a6, a7, a8, a9;
        int a10, a11, a12, a13, a14, a15;
    };
};

int f()
{
    struct S s;
    return s.a0;
}

(gdb) p component
$1 = (tree) 0x7ffff6300c30

(gdb) p field_array[0].decl_minimal.name
$1 = (tree) 0x7ffff6300c30
$1 = (tree) 0x0
$2 = (tree) 0x7ffff6300050
$3 = (tree) 0x7ffff63000a0...

Thanks for the guidance,
Bogdan

On Tue, Jul 31, 2018 at 10:43 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Tue, 31 Jul 2018, Bogdan Harjoc wrote:
>
>> With fresh git sources and contrib/gcc_update the tests pass:
>>
>> === gcc Summary ===
>>
>> # of expected passes 133500
>> # of expected failures 422
>> # of unsupported tests 2104
>>
>> gcc-build/gcc/xgcc  version 9.0.0 20180730 (experimental) (GCC)
>>
>> I wasn't able to reduce the input to avoid including <time.h> and as
>> it only reproduces without -save-temps, it's not clear how to write a
>> testcase for this one.
>
> Could you give more details of the paths through the code that are
> involved in the infinite loop, and the different paths you get without
> -save-temps?  Is this an issue of dependence on the values of pointers, or
> something like that?  Is it possible to produce a test with more instances
> of the problem in it, say, so that the probability of the problem showing
> up at least once with the test is much higher?
>
> A binary search should not result in an infinite loop simply because the
> elements of the array are not sorted; in that case it should just converge
> on an unpredictable element.  So more explanation of how the infinite loop
> occurs is needed.  (But if choice of an unpredictable element results in
> e.g. subsequent diagnostics varying depending on pointer values, that by
> itself is a problem that may justify avoiding this code in the case where
> the array may not be sorted.)
>
> --
> Joseph S. Myers
> joseph@codesourcery.com