[Bug gdb/28480] New: pretty printing getting confused

[Bug gdb/28480] New: pretty printing getting confused

[jwakely.gcc at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

            Bug ID: 28480
           Summary: pretty printing getting confused
           Product: gdb
           Version: HEAD
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: jwakely.gcc at gmail dot com
  Target Milestone: ---

On fedora, with libstdc++-debuginfo installed, and either rawhide's gdb-11 or a
self-built gdb-12 from git.

#include <memory>
#include <string>

struct datum
{
  std::string s;
  int i;
};

int
main()
{
  std::unique_ptr<datum> uptr (new datum);
  uptr->s = "hi bob";
  uptr->i = 23;
  std::unique_ptr<datum> &ruptr = uptr;
}

$ g++ -g cxx11.cc
$ gdb ./a.out
GNU gdb (GDB) Fedora 11.1-2.fc36
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./a.out...
(gdb) start
Temporary breakpoint 1 at 0x40118f: file cxx11.cc, line 13.
Starting program: /tmp/a.out 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Temporary breakpoint 1, main () at cxx11.cc:13
13        std::unique_ptr<datum> uptr (new datum);
(gdb) n
14        uptr->s = "hi bob";
(gdb) 
15        uptr->i = 23;
(gdb) 
16        std::unique_ptr<datum> &ruptr = uptr;
(gdb) p uptr
$1 = std::unique_ptr<datum> = {get() = {<No data fields>}}


This "<No data fields>" is wrong, it's a datum* pointer, and should be printed
as such.

This works with GDB 10, but not GDB 11 in rawhide or a self-built GDB 12 from
git.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[jwakely.gcc at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

Jonathan Wakely <jwakely.gcc at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pedro at palves dot net

--- Comment #1 from Jonathan Wakely <jwakely.gcc at gmail dot com> ---
87a37e5e078f506fa9905b74e9238593c537fcd5 is the first bad commit
commit 87a37e5e078f506fa9905b74e9238593c537fcd5
Author: Pedro Alves 
Date:   Fri Aug 28 21:10:59 2020 +0100

    Reject ambiguous C++ field accesses (PR exp/26602)

    The gdb.cp/ambiguous.exp testcase had been disabled for many years,
    but recently it was re-enabled.  However, it is failing everywhere.
    That is because it is testing an old feature that is gone from GDB.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[jwakely.gcc at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #2 from Jonathan Wakely <jwakely.gcc at gmail dot com> ---
$ valgrind /tmp/gdb/gdb/gdb -q -ex 'br 16' -ex r -ex 'p uptr' -ex qq /tmp/a.out
 | grep 'get() ='
==1770130== Memcheck, a memory error detector
==1770130== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1770130== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==1770130== Command: /tmp/gdb/gdb/gdb -q -ex br\ 16 -ex r -ex p\ uptr -ex qq
/tmp/a.out
==1770130== 
warning: linux_ptrace_test_ret_to_nx: WSTOPSIG 6 is neither SIGTRAP nor
SIGSEGV!
==1770130== Invalid read of size 1
==1770130==    at 0x484A2F0: memmove (vg_replace_strmem.c:1382)
==1770130==    by 0x76FE49: memcpy (string_fortified.h:29)
==1770130==    by 0x76FE49: value_contents_copy_raw(value*, long, value*, long,
long) (value.c:1330)
==1770130==    by 0x7735C0: value_primitive_field(value*, long, int, type*)
(value.c:3036)
==1770130==    by 0x764152: struct_field_searcher::search(value*, long, type*)
(valops.c:1901)
==1770130==    by 0x763E40: struct_field_searcher::search(value*, long, type*)
(valops.c:2002)
==1770130==    by 0x763E40: struct_field_searcher::search(value*, long, type*)
(valops.c:2002)
==1770130==    by 0x763E40: struct_field_searcher::search(value*, long, type*)
(valops.c:2002)
==1770130==    by 0x76423D: search_struct_field(char const*, value*, type*,
int) (valops.c:2023)
==1770130==    by 0x766CF0: value_struct_elt(value**, value**, char const*,
int*, char const*) (valops.c:2251)
==1770130==    by 0x67DDB6: valpy_getitem(_object*, _object*) (py-value.c:990)
==1770130==    by 0x497D836: _PyEval_EvalFrameDefault (ceval.c:2101)
==1770130==    by 0x497BE93: UnknownInlinedFun (pycore_ceval.h:46)
==1770130==    by 0x497BE93: _PyEval_Vector (ceval.c:5073)
==1770130==  Address 0x536a99f is 1 bytes before a block of size 8 alloc'd
==1770130==    at 0x4845464: calloc (vg_replace_malloc.c:1328)
==1770130==    by 0x456D60: xcalloc (alloc.c:100)
==1770130==    by 0x770986: allocate_value_contents (value.c:1022)
==1770130==    by 0x770986: allocate_value_contents (value.c:1016)
==1770130==    by 0x770986: allocate_value(type*) (value.c:1033)
==1770130==    by 0x77359B: value_primitive_field(value*, long, int, type*)
(value.c:3035)
==1770130==    by 0x764152: struct_field_searcher::search(value*, long, type*)
(valops.c:1901)
==1770130==    by 0x763E40: struct_field_searcher::search(value*, long, type*)
(valops.c:2002)
==1770130==    by 0x76423D: search_struct_field(char const*, value*, type*,
int) (valops.c:2023)
==1770130==    by 0x766CF0: value_struct_elt(value**, value**, char const*,
int*, char const*) (valops.c:2251)
==1770130==    by 0x67DDB6: valpy_getitem(_object*, _object*) (py-value.c:990)
==1770130==    by 0x497D836: _PyEval_EvalFrameDefault (ceval.c:2101)
==1770130==    by 0x497BE93: UnknownInlinedFun (pycore_ceval.h:46)
==1770130==    by 0x497BE93: _PyEval_Vector (ceval.c:5073)
==1770130==    by 0x4985890: UnknownInlinedFun (call.c:342)
==1770130==    by 0x4985890: _PyObject_FastCallDictTstate (call.c:142)
==1770130== 
$1 = std::unique_ptr<datum> = {get() = {<No data fields>}}
==1770130== 
==1770130== HEAP SUMMARY:
==1770130==     in use at exit: 18,182,520 bytes in 34,241 blocks
==1770130==   total heap usage: 259,945 allocs, 225,704 frees, 171,556,058
bytes allocated
==1770130== 
==1770130== LEAK SUMMARY:
==1770130==    definitely lost: 160 bytes in 3 blocks
==1770130==    indirectly lost: 0 bytes in 0 blocks
==1770130==      possibly lost: 446,388 bytes in 3,992 blocks
==1770130==    still reachable: 17,735,972 bytes in 30,246 blocks
==1770130==         suppressed: 0 bytes in 0 blocks
==1770130== Rerun with --leak-check=full to see details of leaked memory
==1770130== 
==1770130== For lists of detected and suppressed errors, rerun with: -s
==1770130== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[jwakely.gcc at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #3 from Jonathan Wakely <jwakely.gcc at gmail dot com> ---
The valgrind error is already present before the commit that introduced the
bug, so is probably unrelated.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

Simon Marchi <simark at simark dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simark at simark dot ca

--- Comment #4 from Simon Marchi <simark at simark dot ca> ---
Ah, I do get this (the memory error, but from ASan) when debugging GDB, very
annoying.  I think I dug a little bit at some point to try to understand, but I
forgot.  I'll give it another try.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[jwakely.gcc at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #5 from Jonathan Wakely <jwakely.gcc at gmail dot com> ---
Well as I said, the memory error was present at the previous commit too, so
seems unrelated to the pretty printing regression (maybe it should be a
separate bug though).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[blarsen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

B. Larsen <blarsen at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-10-26
                 CC|                            |blarsen at redhat dot com
     Ever confirmed|0                           |1

--- Comment #6 from B. Larsen <blarsen at redhat dot com> ---
I'm starting to look into this, and I can confirm that the bug is there and
that Wakely's commit is the one that introduced the problem.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[blarsen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #7 from B. Larsen <blarsen at redhat dot com> ---
After looking at this for a bit, I have an RFC. My fix changes the order in
which Pedro's searcher returns things. I'm not sure this is the best way to
handle a pretty printer bug, but it's a starting point. feedback welcome:
https://sourceware.org/pipermail/gdb-patches/2021-November/183084.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[jwakely.gcc at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #8 from Jonathan Wakely <jwakely.gcc at gmail dot com> ---
Standalone reproducer:

namespace std
{
#if __has_cpp_attribute(__no_unique_address__)
#warning using attr
  template<typename T>
    struct _Head_base
    {
      [[__no_unique_address__]] T _M_head_impl;
    };
#else
#warning not using attr
  template<typename T>
    struct _Head_base : T
    { };
#endif

  template<typename T>
    struct _Head_base<T*>
    {
      T* _M_head_impl;
    };

  template<unsigned long, typename ...> struct _Tuple_impl;

  template<typename T, typename U>
    struct _Tuple_impl<0, T, U> : _Tuple_impl<1, U>, _Head_base<T>
    { };

  template<typename U>
    struct _Tuple_impl<1, U> : _Head_base<U>
    { };

  template<typename T, typename U>
    struct tuple : _Tuple_impl<0, T, U>
    { };

  template<typename T> struct default_delete { };

  template<typename T, typename D = default_delete<T>>
    struct unique_ptr
    {
      unique_ptr(T* p) { _M_t._Head_base<T*>::_M_head_impl = p; }

      using __tuple_type = tuple<T*, D>;

      __tuple_type _M_t;
    };
}

struct datum
{
  const char* s;
  int i;
};

int
main()
{
  std::unique_ptr<datum> uptr (new datum);
  return 0;
}

compiled with GCC 9 or above this uses the [[no_unique_address]] attribute, an
GDB gets confused:

$ GCC 8 up.C -Wno-unused && ~/gcc/gdb/11/bin/gdb -q  -ex 'start' -ex n -ex  'p
uptr' a.out
up.C:11:2: warning: #warning not using attr [-Wcpp]
 #warning not using attr
  ^~~~~~~
Reading symbols from a.out...
Temporary breakpoint 1 at 0x40112a: file up.C, line 59.
Starting program: /tmp/a.out 

Temporary breakpoint 1, main () at up.C:59
59        std::unique_ptr<datum> uptr (new datum);
60        return 0;
$1 = std::unique_ptr<datum> = {get() = 0x416eb0}


$ GCC 9 up.C -Wno-unused && ~/gcc/gdb/11/bin/gdb -q  -ex 'start' -ex n -ex  'p
uptr' a.out
up.C:4:2: warning: #warning using attr [-Wcpp]
    4 | #warning using attr
      |  ^~~~~~~
Reading symbols from a.out...
Temporary breakpoint 1 at 0x40112a: file up.C, line 59.
Starting program: /tmp/a.out 

Temporary breakpoint 1, main () at up.C:59
59        std::unique_ptr<datum> uptr (new datum);
60        return 0;
$1 = std::unique_ptr<datum> = {get() = {<No data fields>}}

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[jwakely.gcc at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #9 from Jonathan Wakely <jwakely.gcc at gmail dot com> ---
It's a bug in the libstdc++ printers:

class UniquePointerPrinter:
    "Print a unique_ptr"

    def __init__ (self, typename, val):
        self.val = val
        impl_type = val.type.fields()[0].type.strip_typedefs()
        # Check for new implementations first:
        if is_specialization_of(impl_type, '__uniq_ptr_data') \
            or is_specialization_of(impl_type, '__uniq_ptr_impl'):
            tuple_member = val['_M_t']['_M_t']
        elif is_specialization_of(impl_type, 'tuple'):
            tuple_member = val['_M_t']
        else:
            raise ValueError("Unsupported implementation for unique_ptr: %s" %
str(impl_type))
        tuple_impl_type = tuple_member.type.fields()[0].type # _Tuple_impl
        tuple_head_type = tuple_impl_type.fields()[1].type   # _Head_base
        head_field = tuple_head_type.fields()[0]
        if head_field.name == '_M_head_impl':
            self.pointer = tuple_member['_M_head_impl']  ## BUG
        elif head_field.is_base_class:
            self.pointer = tuple_member.cast(head_field.type)
        else:
            raise ValueError("Unsupported implementation for tuple in
unique_ptr: %s" % str(impl_type))


On the line marked BUG we get different behaviour depending on whether the
class uses [[no_unique_address]] or not. With the [[no_unique_address]]
attribute there are two subobjects called _M_head_impl and so the printer is
wrong to use tuple_member['_M_head_impl'] and assume that the right one gets
chosen.

When the [[no_unique_address]] attribute is not used (i.e. GCC 8 and older)
there is only one mnember called _M_head_impl and it works OK.

Pedro's patch must have changed which member gets used by the ['_M_head_impl']
member access. And that's fine -- it's ambiguous, so the printer is wrong to
rely on a particular one being returned.

I will fix the printer ...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[jwakely.gcc at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #10 from Jonathan Wakely <jwakely.gcc at gmail dot com> ---
I think this is the fix:

--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -258,7 +258,7 @@ class UniquePointerPrinter:
         tuple_head_type = tuple_impl_type.fields()[1].type   # _Head_base
         head_field = tuple_head_type.fields()[0]
         if head_field.name == '_M_head_impl':
-            self.pointer = tuple_member['_M_head_impl']
+            self.pointer = tuple_member.cast(tuple_head_type)['_M_head_impl']
         elif head_field.is_base_class:
             self.pointer = tuple_member.cast(head_field.type)
         else:

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[jwakely.gcc at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

Jonathan Wakely <jwakely.gcc at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=103086

--- Comment #11 from Jonathan Wakely <jwakely.gcc at gmail dot com> ---
I've created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103086 for this, with
a more detailed explanation.

I think GDB's new behaviour is probably better, and the libstdc++ printers
should be fixed.

However, Bruno did suggest that when tuple_member['_M_head_impl'] is ambiguous
it should give an error, not silently return either the first or last matching
member.

For example, in this code:

struct X
{
  int i = 0x1;
};

struct Y
{
  char i = 'Y';
};

struct Z : X, Y { };

int main()
{
  Z z;
  return 0;
}

GDB 10 will happily print z.i

$ gdb -ex start -ex n -ex "py print(gdb.parse_and_eval('z')['i'])"  a.out
GNU gdb (GDB) Fedora 10.2-3.fc34
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from a.out...
Temporary breakpoint 1 at 0x40110a: file ambig.cc, line 15.
Starting program: /tmp/a.out 

Temporary breakpoint 1, main () at ambig.cc:15
15        Z z;
16        return 0;
89 'Y'

But GDB 12 gives an error:

$ ~/gcc/gdb/11/bin/gdb -ex start -ex n -ex "py
print(gdb.parse_and_eval('z')['i'])"  a.out
GNU gdb (GDB) 12.0.50.20211020-git
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from a.out...
Temporary breakpoint 1 at 0x40110a: file ambig.cc, line 15.
Starting program: /tmp/a.out 

Temporary breakpoint 1, main () at ambig.cc:15
15        Z z;
16        return 0;
Traceback (most recent call last):
  File "<string>", line 1, in <module>
gdb.error: Request for member 'i' is ambiguous in type 'Z'. Candidates are:
  'int X::i' (Z -> X)
  'char Y::i' (Z -> Y)
Error while executing Python code.

I wonder why there wasn't an error for tuple_member['_M_head_impl'] in the
libstdc++ printer.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[jwakely.gcc at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #12 from Jonathan Wakely <jwakely.gcc at gmail dot com> ---
Aha, the std::tuple case is more like this:

struct X
{
  int i = 0x1;
};

struct Y
{
  struct S { };
  [[no_unique_address]] S i;
};

struct XX : X { };

struct Z : Y, XX { };

int main()
{
  Z z;
  return 0;
}

Here z.i is ambiguous, but GDB doesn't diagnose it.

And GDB 10 returns z.X::i but GDB 11 returns z.Y::i

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[jwakely.gcc at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #13 from Jonathan Wakely <jwakely.gcc at gmail dot com> ---
According to the C++ standard, z.i finds i in two base classes, so it's
ambiguous.

GDB seems to consider it non-ambiguous, maybe because they're at different
depths?  i.e. we have z.Y::i and z.XX::X::i where Y::i is in a direct base
class, and X::i is in a base class of a base class. But C++ says that is still
ambiguous, because X::i is also accessible as z.XX::i and that's ambiguous with
z.Y::i

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #14 from Simon Marchi <simark at simark dot ca> ---
(In reply to Jonathan Wakely from comment #12)
> Aha, the std::tuple case is more like this:
> 
> struct X
> {
>   int i = 0x1;
> };
> 
> struct Y
> {
>   struct S { };
>   [[no_unique_address]] S i;
> };
> 
> struct XX : X { };
> 
> struct Z : Y, XX { };
> 
> int main()
> {
>   Z z;
>   return 0;
> }

I think you can get rid of XX (make Z directly inherit from X) and it still
reproduces.

> 
> Here z.i is ambiguous, but GDB doesn't diagnose it.
> 
> And GDB 10 returns z.X::i but GDB 11 returns z.Y::i

If I try to use z.i in the program, it doesn't compile:

test.cpp: In function ‘int main()’:
test.cpp:17:12: error: request for member ‘i’ is ambiguous
   17 |   return z.i;
      |            ^
test.cpp:3:7: note: candidates are: ‘int X::i’
    3 |   int i = 0x1;
      |       ^
test.cpp:9:27: note:                 ‘Y::S Y::i’
    9 |   [[no_unique_address]] S i;
      |                           ^

So I agree, it makes sense for GDB to not accept the expression.  It looks like
the intent is to reject and (and print the "request is ambiguous" message), but
that the no_unique_address tag confuses GDB.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #15 from Simon Marchi <simark at simark dot ca> ---
(In reply to Jonathan Wakely from comment #13)
> According to the C++ standard, z.i finds i in two base classes, so it's
> ambiguous.
> 
> GDB seems to consider it non-ambiguous, maybe because they're at different
> depths?  i.e. we have z.Y::i and z.XX::X::i where Y::i is in a direct base
> class, and X::i is in a base class of a base class. But C++ says that is
> still ambiguous, because X::i is also accessible as z.XX::i and that's
> ambiguous with z.Y::i

I don't think it's the depth that confuses GDB, it seems to be the
no_unique_address.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[blarsen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #16 from B. Larsen <blarsen at redhat dot com> ---
Ah! I think I got it!

GDB is using boffset to check if there is virtual inheritance. However, this
test case has an empty struct, so the offset for both members is the same! If
any members are added to struct S, GDB correctly identifies the ambiguity.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[pedro at palves dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #17 from Pedro Alves <pedro at palves dot net> ---
> I don't think it's the depth that confuses GDB, it seems to be the 
> no_unique_address.

Maybe unrelated, but FYI this last reproducer triggers this GCC bug:

 Bug 101378 - Negative DW_AT_data_member_location
 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101378

Still reproduces with today's gcc.  :-/

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[blarsen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #18 from B. Larsen <blarsen at redhat dot com> ---
> Maybe unrelated, but FYI this last reproducer triggers this GCC bug:

This is unrelated, the actual offset being correct can still point ambiguous
things to the same spot

A quick patch to struct_field_searcher::update_value, to check if the values in
the end of the inheritance path are the same results in the expected behavior.
I'll work on the ambiguous test case and submit a new patch to the mailing list
soon

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/28480] pretty printing getting confused

[blarsen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #19 from B. Larsen <blarsen at redhat dot com> ---
I've managed to spin up a patch to fix this, it's on the mailing list already:
https://sourceware.org/pipermail/gdb-patches/2021-November/183145.html

I'd suggest that the title of the bug should be updated to something more
representative of the actual bug, like "Incorrect ambiguity detection due to
[[no_unique_address]]" or something like that

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug c++/28480] Incorrect field ambiguity detection due to [[no_unique_address]]

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

Simon Marchi <simark at simark dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|pretty printing getting     |Incorrect field ambiguity
                   |confused                    |detection due to
                   |                            |[[no_unique_address]]
          Component|gdb                         |c++

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug c++/28480] Incorrect field ambiguity detection due to [[no_unique_address]]

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #20 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Bruno Larsen <blarsen@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a41ad3474ceacba39e11c7478154c0e553784a01

commit a41ad3474ceacba39e11c7478154c0e553784a01
Author: Bruno Larsen <blarsen@redhat.com>
Date:   Fri Oct 29 17:56:28 2021 -0300

    PR gdb/28480: Improve ambiguous member detection

    Basic ambiguity detection assumes that when 2 fields with the same name
    have the same byte offset, it must be an unambiguous request. This is not
    always correct. Consider the following code:

    class empty { };

    class A {
    public:
      [[no_unique_address]] empty e;
    };

    class B {
    public:
      int e;
    };

    class C: public A, public B { };

    if we tried to use c.e in code, the compiler would warn of an ambiguity,
    however, since A::e does not demand an unique address, it gets the same
    address (and thus byte offset) of the members, making A::e and B::e have
the
    same address. however, "print c.e" would fail to report the ambiguity,
    and would instead print it as an empty class (first path found).

    The new code solves this by checking for other found_fields that have
    different m_struct_path.back() (final class that the member was found
    in), despite having the same byte offset.

    The testcase gdb.cp/ambiguous.exp was also changed to test for this
    behavior.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug c++/28480] Incorrect field ambiguity detection due to [[no_unique_address]]

[blarsen at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

B. Larsen <blarsen at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #21 from B. Larsen <blarsen at redhat dot com> ---
The commit mentioned fixes this bug

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug c++/28480] Incorrect field ambiguity detection due to [[no_unique_address]]

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28480

--- Comment #22 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The gdb-11-branch branch has been updated by Joel Brobecker
<brobecke@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b6fc91c70a43d96d02c065ec0a9be96f272dace6

commit b6fc91c70a43d96d02c065ec0a9be96f272dace6
Author: Bruno Larsen <blarsen@redhat.com>
Date:   Sat Dec 11 11:47:53 2021 +0400

    PR gdb/28480: Improve ambiguous member detection

    Basic ambiguity detection assumes that when 2 fields with the same name
    have the same byte offset, it must be an unambiguous request. This is not
    always correct. Consider the following code:

    class empty { };

    class A {
    public:
      [[no_unique_address]] empty e;
    };

    class B {
    public:
      int e;
    };

    class C: public A, public B { };

    if we tried to use c.e in code, the compiler would warn of an ambiguity,
    however, since A::e does not demand an unique address, it gets the same
    address (and thus byte offset) of the members, making A::e and B::e have
the
    same address. however, "print c.e" would fail to report the ambiguity,
    and would instead print it as an empty class (first path found).

    The new code solves this by checking for other found_fields that have
    different m_struct_path.back() (final class that the member was found
    in), despite having the same byte offset.

    The testcase gdb.cp/ambiguous.exp was also changed to test for this
    behavior.

    gdb/ChangeLog:

            PR gdb/28480
            * valops.c (struct_field_searcher::update_result): Improve
            ambiguous member detection.

    gdb/testsuite/ChangeLog:

            PR gdb/28480

            Pushed by Joel Brobecker  <brobecker@adacore.com>
            * gdb.cp/ambiguous.cc: Add code to permit ambiguous member testing.
            * gdb.cp/ambiguous.exp: Add ambiguous member test.

    (cherry picked from commit a41ad3474ceacba39e11c7478154c0e553784a01)

-- 
You are receiving this mail because:
You are on the CC list for the bug.