diff mbox series

Fix getting pointer-to-member for classes with virtual base classes

Message ID 20251222121244.38935-1-ssbssa@yahoo.de
State New
Headers
Series Fix getting pointer-to-member for classes with virtual base classes |

Commit Message

Hannes Domani Dec. 22, 2025, 12:12 p.m. UTC 
  Currently this assertion fails:

print &derived::i
$125 = C:/src/repos/binutils-gdb.git/gdb/gdbtypes.h:621: internal-error: loc_bitpos: Assertion `m_loc_kind == FIELD_LOC_KIND_BITPOS' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging sessionFAIL: gdb.cp/virtbase2.exp: foo::func_f: print &derived::i (GDB internal error)

The comment above cp_find_class_member even mentions that only non-virtual
base classes should be checked, so this adds that check.

Now we get:

print &derived::i
$125 = 4
(gdb) PASS: gdb.cp/virtbase2.exp: foo::func_f: print &derived::i
---
 gdb/cp-valprint.c                  | 2 ++
 gdb/testsuite/gdb.cp/virtbase2.exp | 1 +
 2 files changed, 3 insertions(+)

Comments

Tom Tromey Jan. 5, 2026, 5:03 p.m. UTC | #1 
>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

Hannes> The comment above cp_find_class_member even mentions that only non-virtual
Hannes> base classes should be checked, so this adds that check.

I don't have a problem with this patch, but I wonder if this conforms to
what C++ compilers do.

Like, does a pointer-to-member work with virtual base classes?  If so
then it seems more correct to try to lift this constraint than to reject
virtual bases here.

Tom
Hannes Domani Jan. 6, 2026, 7:41 p.m. UTC | #2 
Am Montag, 5. Januar 2026 um 18:04:03 MEZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
> 
> Hannes> The comment above cp_find_class_member even mentions that only non-virtual
> Hannes> base classes should be checked, so this adds that check.
> 
> I don't have a problem with this patch, but I wonder if this conforms to
> what C++ compilers do.
> 
> Like, does a pointer-to-member work with virtual base classes?  If so
> then it seems more correct to try to lift this constraint than to reject
> virtual bases here.

You're right, pointer-to-member with virtual base classes don't work at all.
The problem starts when trying to get the member-address, in
value_struct_elt_for_reference:

      if (BASETYPE_VIA_VIRTUAL (t, i))
    base_offset = 0;
      else
    base_offset = TYPE_BASECLASS_BITPOS (t, i) / 8;

Since base_offset is 0 for all virtual base classes, the resulting bitpos
sum isn't really correct, so lookup_memberptr_type doesn't have enough
information in this situation.
But I'm not sure how we could make it work (maybe add the type of the
class whose member is asked for?).


Hannes
Hannes Domani Jan. 7, 2026, 2:21 p.m. UTC | #3 
Am Dienstag, 6. Januar 2026 um 20:41:49 MEZ hat Hannes Domani <ssbssa@yahoo.de> Folgendes geschrieben:

> Am Montag, 5. Januar 2026 um 18:04:03 MEZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:
> 
> > >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
> > 
> > Hannes> The comment above cp_find_class_member even mentions that only non-virtual
> > Hannes> base classes should be checked, so this adds that check.
> > 
> > I don't have a problem with this patch, but I wonder if this conforms to
> > what C++ compilers do.

I forgot yesterday to actually test this with a compiler, but it looks like
taking pointer-to-members with virtual base classes isn't allowed in c++:

../../../gdb/testsuite/gdb.cp/virtbase2.cc:52:26: error: pointer to member conversion via virtual base 'super'
   52 |   int derived::* w_ptr = &derived::w;
      |                          ^~~~~~~~~~~
../../../gdb/testsuite/gdb.cp/virtbase2.cc:53:26: error: pointer to member conversion via virtual base 'base'
   53 |   int derived::* x_ptr = &derived::x;
      |                          ^~~~~~~~~~~
../../../gdb/testsuite/gdb.cp/virtbase2.cc:54:26: error: pointer to member conversion via virtual base 'base'
   54 |   int derived::* i_ptr = &derived::i;
      |                          ^~~~~~~~~~~
../../../gdb/testsuite/gdb.cp/virtbase2.cc:55:29: error: pointer to member conversion via virtual base 'base'
   55 |   double derived::* d_ptr = &derived::d;
      |                             ^~~~~~~~~~~

So maybe we should print a similar error in this case.


Hannes
Tom Tromey Jan. 22, 2026, 8:03 p.m. UTC | #4 
Hannes> I forgot yesterday to actually test this with a compiler, but it looks like
Hannes> taking pointer-to-members with virtual base classes isn't allowed in c++:

Hannes> ../../../gdb/testsuite/gdb.cp/virtbase2.cc:52:26: error: pointer to member conversion via virtual base 'super'
Hannes>    52 |   int derived::* w_ptr = &derived::w;
Hannes>       |                          ^~~~~~~~~~~
Hannes> ../../../gdb/testsuite/gdb.cp/virtbase2.cc:53:26: error: pointer to member conversion via virtual base 'base'
Hannes>    53 |   int derived::* x_ptr = &derived::x;
Hannes>       |                          ^~~~~~~~~~~
Hannes> ../../../gdb/testsuite/gdb.cp/virtbase2.cc:54:26: error: pointer to member conversion via virtual base 'base'
Hannes>    54 |   int derived::* i_ptr = &derived::i;
Hannes>       |                          ^~~~~~~~~~~
Hannes> ../../../gdb/testsuite/gdb.cp/virtbase2.cc:55:29: error: pointer to member conversion via virtual base 'base'
Hannes>    55 |   double derived::* d_ptr = &derived::d;
Hannes>       |                             ^~~~~~~~~~~

Hannes> So maybe we should print a similar error in this case.

Yeah, that would make sense to me.  In most cases gdb tries to follow
the language.  Sometimes extensions are available, when that makes
sense; but in a case like this I think it doesn't, and plus which this
is an obscure thing to do anyway.

Tom
diff mbox series

Patch

diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index 252072b66dd..fa373acce1d 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -667,6 +667,8 @@  cp_find_class_member (struct type **self_p, int *fieldno,
 
   for (i = 0; i < TYPE_N_BASECLASSES (self); i++)
     {
+      if (BASETYPE_VIA_VIRTUAL (self, i))
+	continue;
       LONGEST bitpos = self->field (i).loc_bitpos ();
       LONGEST bitsize = 8 * self->field (i).type ()->length ();
 
diff --git a/gdb/testsuite/gdb.cp/virtbase2.exp b/gdb/testsuite/gdb.cp/virtbase2.exp
index d18312be002..1a73e31c179 100644
--- a/gdb/testsuite/gdb.cp/virtbase2.exp
+++ b/gdb/testsuite/gdb.cp/virtbase2.exp
@@ -113,4 +113,5 @@  with_test_prefix "foo::func_f" {
     test_variables_in_super {super}
     test_variables_in_super {foo super}
     test_variables_in_super {foo derived super}
+    gdb_test "print &derived::i" " = $decimal"
 }