[PATCH] Enable gdb.cp/ambiguous.exp with GCC >= 10.1 and clang

[PATCH] Enable gdb.cp/ambiguous.exp with GCC >= 10.1 and clang

[Gary Benson]

Hi all,

gdb.cp/ambiguous.exp failed to build using clang with the following
error:

 gdb compile failed, /gdbtest/src/gdb/testsuite/gdb.cp/ambiguous.cc:70:36:
   warning: direct base 'A1' is inaccessible due to ambiguity:
     class JVA1 -> class KV -> class A1
     class JVA1 -> class A1 [-Winaccessible-base]
 class JVA1 : public KV, public LV, public A1 {
                                   ^~~~~~~~~

This commit builds this testcase with -Wno-inaccessible-base when
using clang, to avoid this failure.

Furthermore, gdb.cp/ambiguous.exp has been disabled when using GCC
since 1998.  This commit enables this testcase when compiling with
GCC >= 10.1, the first version with -Wno-inaccessible-base.

Checked on Fedora 31 x86_64, GCC and clang.  Ok to commit?

Cheers,
Gary

--
gdb/testsuite/ChangeLog:

	* gdb.cp/ambiguous.exp: Enable test when compiling with GCC >=
	10.1.  Add additional_flags=-Wno-inaccessible-base when compiling
	with GCC or clang.
---
 gdb/testsuite/ChangeLog            |  6 ++++++
 gdb/testsuite/gdb.cp/ambiguous.exp | 17 +++++++++++++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.cp/ambiguous.exp b/gdb/testsuite/gdb.cp/ambiguous.exp
index fffe20a..26f96a9 100644
--- a/gdb/testsuite/gdb.cp/ambiguous.exp
+++ b/gdb/testsuite/gdb.cp/ambiguous.exp
@@ -30,12 +30,25 @@ if { [skip_cplus_tests] } { continue }
 standard_testfile .cc
 
 if [get_compiler_info "c++"] {
+    unsupported "couldn't find a valid c++ compiler"
     return -1
 }
 
-if { [test_compiler_info gcc-*] } then { continue }
+# GCCs prior to 10.1 do not support -Wno-inaccessible-base.
+if { [test_compiler_info {gcc-[0-9]-*}]
+     || [test_compiler_info {gcc-10-0-*}] } {
+    unsupported "compiler does not support -Wno-inaccessible-base"
+    return -1
+}
+
+set additional_flags ""
+if { [test_compiler_info gcc*] } {
+     || [test_compiler_info clang*] } {
+    set additional_flags "additional_flags=-Wno-inaccessible-base"
+}
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+     [list debug c++ $additional_flags]]} {
     return -1
 }
 
-- 
1.8.3.1

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

[Pedro Alves]

On 7/27/20 2:09 PM, Gary Benson via Gdb-patches wrote:
> Hi all,
> 
> gdb.cp/ambiguous.exp failed to build using clang with the following
> error:
> 
>  gdb compile failed, /gdbtest/src/gdb/testsuite/gdb.cp/ambiguous.cc:70:36:
>    warning: direct base 'A1' is inaccessible due to ambiguity:
>      class JVA1 -> class KV -> class A1
>      class JVA1 -> class A1 [-Winaccessible-base]
>  class JVA1 : public KV, public LV, public A1 {
>                                    ^~~~~~~~~
> 
> This commit builds this testcase with -Wno-inaccessible-base when
> using clang, to avoid this failure.
> 
> Furthermore, gdb.cp/ambiguous.exp has been disabled when using GCC
> since 1998.  

Curious you reference the year, do you know it because you found the
commit that disabled it?

> This commit enables this testcase when compiling with
> GCC >= 10.1, the first version with -Wno-inaccessible-base.
> 
> Checked on Fedora 31 x86_64, GCC and clang.  Ok to commit?
> 
> Cheers,
> Gary
> 
> --
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.cp/ambiguous.exp: Enable test when compiling with GCC >=
> 	10.1.  Add additional_flags=-Wno-inaccessible-base when compiling
> 	with GCC or clang.
> ---
>  gdb/testsuite/ChangeLog            |  6 ++++++
>  gdb/testsuite/gdb.cp/ambiguous.exp | 17 +++++++++++++++--
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.cp/ambiguous.exp b/gdb/testsuite/gdb.cp/ambiguous.exp
> index fffe20a..26f96a9 100644
> --- a/gdb/testsuite/gdb.cp/ambiguous.exp
> +++ b/gdb/testsuite/gdb.cp/ambiguous.exp
> @@ -30,12 +30,25 @@ if { [skip_cplus_tests] } { continue }
>  standard_testfile .cc
>  
>  if [get_compiler_info "c++"] {
> +    unsupported "couldn't find a valid c++ compiler"
>      return -1
>  }
>  
> -if { [test_compiler_info gcc-*] } then { continue }
> +# GCCs prior to 10.1 do not support -Wno-inaccessible-base.
> +if { [test_compiler_info {gcc-[0-9]-*}]
> +     || [test_compiler_info {gcc-10-0-*}] } {

Given the first release of GCC 10 is 10.1, do we really
need the second check?

> +    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?

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

[Gary Benson]

Pedro Alves wrote:
> On 7/27/20 2:09 PM, Gary Benson via Gdb-patches wrote:
> > Furthermore, gdb.cp/ambiguous.exp has been disabled when using GCC
> > since 1998.
> 
> Curious you reference the year, do you know it because you found the
> commit that disabled it?

Yes :)

> > -if { [test_compiler_info gcc-*] } then { continue }
> > +# GCCs prior to 10.1 do not support -Wno-inaccessible-base.
> > +if { [test_compiler_info {gcc-[0-9]-*}]
> > +     || [test_compiler_info {gcc-10-0-*}] } {
> 
> Given the first release of GCC 10 is 10.1...

Ah, I didn't know that.

> ...do we really need the second check?

No :)  I've removed it.

> > +    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?

--
gdb.cp/ambiguous.exp failed to build using clang with the following
error:

 gdb compile failed, /gdbtest/src/gdb/testsuite/gdb.cp/ambiguous.cc:70:36:
   warning: direct base 'A1' is inaccessible due to ambiguity:
     class JVA1 -> class KV -> class A1
     class JVA1 -> class A1 [-Winaccessible-base]
 class JVA1 : public KV, public LV, public A1 {
                                   ^~~~~~~~~

This commit builds this testcase with -Wno-inaccessible-base when
using clang, to avoid this failure.

Furthermore, gdb.cp/ambiguous.exp has been disabled when using GCC
since 1998.  This commit enables this testcase, building with
-Wno-inaccessible-base when using GCC >= 10.1, and -w otherwise.

gdb/testsuite/ChangeLog:

	* gdb.cp/ambiguous.exp: Enable test when compiling with GCC.
	Add additional_flags=-Wno-inaccessible-base when compiling
	with GCC >= 10.1 or clang.  Add additional_flags=-w when
	compiling with GCC < 10.
---
 gdb/testsuite/ChangeLog            |  7 +++++++
 gdb/testsuite/gdb.cp/ambiguous.exp | 12 ++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.cp/ambiguous.exp b/gdb/testsuite/gdb.cp/ambiguous.exp
index fffe20a..17fb29f 100644
--- a/gdb/testsuite/gdb.cp/ambiguous.exp
+++ b/gdb/testsuite/gdb.cp/ambiguous.exp
@@ -30,12 +30,20 @@ if { [skip_cplus_tests] } { continue }
 standard_testfile .cc
 
 if [get_compiler_info "c++"] {
+    unsupported "couldn't find a valid c++ compiler"
     return -1
 }
 
-if { [test_compiler_info gcc-*] } then { continue }
+set additional_flags ""
+if {[test_compiler_info {gcc-[0-9]-*}]} {
+    # GCCs prior to 10.1 do not support -Wno-inaccessible-base.
+    set additional_flags "additional_flags=-w"
+} elseif {[test_compiler_info gcc*] || [test_compiler_info clang*]} {
+    set additional_flags "additional_flags=-Wno-inaccessible-base"
+}
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+     [list debug c++ $additional_flags]]} {
     return -1
 }
 
-- 
1.8.3.1

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

[Pedro Alves]

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:
>>> Furthermore, gdb.cp/ambiguous.exp has been disabled when using GCC
>>> since 1998.
>>
>> Curious you reference the year, do you know it because you found the
>> commit that disabled it?
> 
> Yes :)

What it was, then?  Does it say anything about why the testcase
was disabled?

> 
>>> -if { [test_compiler_info gcc-*] } then { continue }
>>> +# GCCs prior to 10.1 do not support -Wno-inaccessible-base.
>>> +if { [test_compiler_info {gcc-[0-9]-*}]
>>> +     || [test_compiler_info {gcc-10-0-*}] } {
>>
>> Given the first release of GCC 10 is 10.1...
> 
> Ah, I didn't know that.
> 
>> ...do we really need the second check?
> 
> No :)  I've removed it.
> 
>>> +    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,
Pedro Alves

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

[Gary Benson]

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:
> >>> Furthermore, gdb.cp/ambiguous.exp has been disabled when using GCC
> >>> since 1998.
> >>
> >> Curious you reference the year, do you know it because you found the
> >> commit that disabled it?
> > 
> > Yes :)
> 
> What it was, then?  Does it say anything about why the testcase
> was disabled?

No, it was just "import YYYYMMDD snapshot" or some such thing.

> >>> -if { [test_compiler_info gcc-*] } then { continue }
> >>> +# GCCs prior to 10.1 do not support -Wno-inaccessible-base.
> >>> +if { [test_compiler_info {gcc-[0-9]-*}]
> >>> +     || [test_compiler_info {gcc-10-0-*}] } {
> >>
> >> Given the first release of GCC 10 is 10.1...
> > 
> > Ah, I didn't know that.
> > 
> >> ...do we really need the second check?
> > 
> > No :)  I've removed it.
> > 
> >>> +    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.

Cool, thanks.

Cheers,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat

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

[Gary Benson]

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.

Cheers,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat

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

[Luis Machado]

Hi,

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?

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

[Gary Benson]

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?

Thanks,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat

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

[Pedro Alves]

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,
Pedro Alves

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

[Luis Machado]

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.

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

[Gary Benson]

Pedro Alves wrote:
> On 8/27/20 11:39 AM, Gary Benson wrote:
> > Luis Machado wrote:
> > > 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.
[snip]
> ...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.

So the test would start passing if that patch was added?
Should we leave the test alone, or XFAIL the cases that
fail, or...?

Cheers,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat

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

[Pedro Alves]

On 8/27/20 4:07 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 8/27/20 11:39 AM, Gary Benson wrote:
>>> Luis Machado wrote:
>>>> 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.
> [snip]
>> ...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.
> 
> So the test would start passing if that patch was added?
> Should we leave the test alone, or XFAIL the cases that
> fail, or...?

I'm adjusting / fixing the testcase at the same time as I'm
patching GDB.  So for now, do nothing.

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

[Gary Benson]

Pedro Alves wrote:
> On 8/27/20 4:07 PM, Gary Benson wrote:
> > Pedro Alves wrote:
> >> On 8/27/20 11:39 AM, Gary Benson wrote:
> >>> Luis Machado wrote:
> >>>> 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.
> > [snip]
> >> ...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.
> > 
> > So the test would start passing if that patch was added?
> > Should we leave the test alone, or XFAIL the cases that
> > fail, or...?
> 
> I'm adjusting / fixing the testcase at the same time as I'm
> patching GDB.  So for now, do nothing.

Awesome, thank you.

Cheers,
Gary

-- 
Gary Benson - he / him / his
Principal Software Engineer, Red Hat

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

[Pedro Alves]

On 8/27/20 5:18 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 8/27/20 4:07 PM, Gary Benson wrote:
>>> Pedro Alves wrote:
>>>> On 8/27/20 11:39 AM, Gary Benson wrote:
>>>>> Luis Machado wrote:
>>>>>> 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.
>>> [snip]
>>>> ...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.
>>>
>>> So the test would start passing if that patch was added?
>>> Should we leave the test alone, or XFAIL the cases that
>>> fail, or...?
>>
>> I'm adjusting / fixing the testcase at the same time as I'm
>> patching GDB.  So for now, do nothing.
> 
> Awesome, thank you.

I've sent it now, here:

 [PATCH] Reject ambiguous C++ field accesses
 https://sourceware.org/pipermail/gdb-patches/2020-August/171526.html

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

[Tom de Vries]

On 8/27/20 8:04 PM, Pedro Alves wrote:
> On 8/27/20 5:18 PM, Gary Benson wrote:
>> Pedro Alves wrote:
>>> On 8/27/20 4:07 PM, Gary Benson wrote:
>>>> Pedro Alves wrote:
>>>>> On 8/27/20 11:39 AM, Gary Benson wrote:
>>>>>> Luis Machado wrote:
>>>>>>> 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.
>>>> [snip]
>>>>> ...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.
>>>>
>>>> So the test would start passing if that patch was added?
>>>> Should we leave the test alone, or XFAIL the cases that
>>>> fail, or...?
>>>
>>> I'm adjusting / fixing the testcase at the same time as I'm
>>> patching GDB.  So for now, do nothing.
>>
>> Awesome, thank you.
> 
> I've sent it now, here:
> 
>  [PATCH] Reject ambiguous C++ field accesses
>  https://sourceware.org/pipermail/gdb-patches/2020-August/171526.html
> 

With the gdb 10 branching planned to happen soon, I've marked these
FAILs as KFAIL, in order to make sure that these won't show up as
"unexpected failure" in the gdb 10 release.

Thanks,
- Tom