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

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Sourceware - gdb-patches mailing list
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

Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-2
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?
Reply | Threaded
Open this post in threaded view
|

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

Sourceware - gdb-patches mailing list
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

Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-2
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
Reply | Threaded
Open this post in threaded view
|

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

Sourceware - gdb-patches mailing list
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

Reply | Threaded
Open this post in threaded view
|

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

Sourceware - gdb-patches mailing list
In reply to this post by Pedro Alves-2
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

Reply | Threaded
Open this post in threaded view
|

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

Sourceware - gdb-patches mailing list
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?
Reply | Threaded
Open this post in threaded view
|

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

Sourceware - gdb-patches mailing list
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

Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-2
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  <[hidden email]>
 +
 +
 +       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
Reply | Threaded
Open this post in threaded view
|

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

Sourceware - gdb-patches mailing list
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  <[hidden email]>
>   +
>   +
>   +       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  <[hidden email]>

     * gdb.cp/inherit.exp: Rewrite.  Use gdb_test_multiple and gdb for
all tests.  Remove old hp-ux and cygnus xfail cases.
Reply | Threaded
Open this post in threaded view
|

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

Sourceware - gdb-patches mailing list
In reply to this post by Pedro Alves-2
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

Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-2
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.
Reply | Threaded
Open this post in threaded view
|

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

Sourceware - gdb-patches mailing list
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

Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-2
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
Reply | Threaded
Open this post in threaded view
|

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