Re: [PATCH v2] Enable gdb.cp/ambiguous.exp with GCC and clang

[Luis Machado <luis.machado@linaro.org>]

On 8/27/20 8:20 AM, Pedro Alves wrote:
> On 8/27/20 11:39 AM, Gary Benson wrote:
>> Luis Machado wrote:
>>> On 8/25/20 11:21 AM, Gary Benson via Gdb-patches wrote:
>>>> Pedro Alves wrote:
>>>>> On 8/17/20 2:24 PM, Gary Benson wrote:
>>>>>> Pedro Alves wrote:
>>>>>>> On 7/27/20 2:09 PM, Gary Benson via Gdb-patches wrote:
>>>>>>>> +    unsupported "compiler does not support -Wno-inaccessible-base"
>>>>>>>
>>>>>>> How about instead of bailing out, use "-Wno-inaccessible-base"
>>>>>>> with GCC >= 10, and use "-w" with older GCCs?
>>>>>>
>>>>>> Sure.  How about this?
>>>>>
>>>>> OK.
>>>>
>>>> Thanks, I pushed it.
>>>
>>> I get the following, under Ubuntu 18.04 (GCC 7.x) with this commit...
>>>
>>> FAIL: gdb.cp/ambiguous.exp: print x.x
>>> FAIL: gdb.cp/ambiguous.exp: print n.x
>>> FAIL: gdb.cp/ambiguous.exp: print j.x
>>> FAIL: gdb.cp/ambiguous.exp: print jva1.x
>>> FAIL: gdb.cp/ambiguous.exp: print jva2.x
>>> FAIL: gdb.cp/ambiguous.exp: print (A1)j
>>> FAIL: gdb.cp/ambiguous.exp: print (A1)jva1
>>>
>>> Is the test really supposed to run with older GCC's?
>>
>> Maybe not.  Though, I don't know what version of GCC it ought to start
>> working on, so it's hard to know what to do.  I could leave the "-w"
>> in for GCC < 10, and add an extra check to make it bail out for GCC
>> <= your version, Luis?  With a suitable comment to mention that that's
>> not set in stone?
> 
> 
> I'm seeing it fail with GCC 9 and clang 10 as well.
> 
> Actually, the testcase can't be working _anywhere_.  It's
> testing a feature that is gone from GDB.
> 
> The testcase come in with the HP merge:
> 
>   +Sun Jan 10 23:44:11 1999  David Taylor  <taylor@texas.cygnus.com>
>   +
>   +
>   +       The following files are part of the HP merge; some had longer
>   +       names at HP, but have been renamed to be no more than 14
>   +       characters in length.
> 
> Looking at the tree back then, we had:
> 
>   /* Helper function used by value_struct_elt to recurse through baseclasses.
>      Look for a field NAME in ARG1. Adjust the address of ARG1 by OFFSET bytes,
>      and search in it assuming it has (class) type TYPE.
>      If found, return value, else return NULL.
>   
>      If LOOKING_FOR_BASECLASS, then instead of looking for struct fields,
>      look for a baseclass named NAME.  */
>   
>   static value_ptr
>   search_struct_field (name, arg1, offset, type, looking_for_baseclass)
>        char *name;
>        register value_ptr arg1;
>        int offset;
>        register struct type *type;
>        int looking_for_baseclass;
>   {
>     int found = 0;
>     char found_class[1024];
>     value_ptr v;
>     struct type *vbase = NULL;
>   
>     found_class[0] = '\000';
>    
>     v = search_struct_field_aux (name, arg1, offset, type, looking_for_baseclass, &found, found_class, &vbase);
>     if (found > 1)
>       warning ("%s ambiguous; using %s::%s. Use a cast to disambiguate.",
>                name, found_class, name);
>   
>     return v;
>   }
> 
> But search_struct_field does not handle the ambiguous field
> case nowadays.  Somehow it got lost over the years.
> That seems like a regression.  I wrote up a patch that adds
> it back (though different), but it exposed other latent
> bugs...  Sigh.  I'll post it soon.

Thanks. I've reached the same conclusion. This is an artifact of the HP 
merge back in the day. I see gdb.cp/ambiguous.exp (previously 
gdb.c++/ambiguous.exp) was not updated to remove these cases like 
gdb.cp/inherit.exp (previously gdb.c++/inherit.exp) was.

See commit ebac27b4c38 for example:

2004-01-29  Michael Chastain  <mec.gnu@mindspring.com>

     * gdb.cp/inherit.exp: Rewrite.  Use gdb_test_multiple and gdb for 
all tests.  Remove old hp-ux and cygnus xfail cases.