public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix dynamic_cast
       [not found] <20231208210016.5260-1-ssbssa.ref@yahoo.de>
@ 2023-12-08 21:00 ` Hannes Domani
  2023-12-10 22:08   ` Tom Tromey
  0 siblings, 1 reply; 3+ messages in thread
From: Hannes Domani @ 2023-12-08 21:00 UTC (permalink / raw)
  To: gdb-patches

PR29011 notes that dynamic_cast does not work correctly if
classes with virtual methods are involved, some of the results
wrongly point into the vtable of the derived class:
```
(gdb) p vlr
$1 = (VirtualLeftRight *) 0x162240
(gdb) p vl
$2 = (VirtualLeft *) 0x162240
(gdb) p vr
$3 = (VirtualRight *) 0x162250
(gdb) p dynamic_cast<VirtualLeftRight*>(vlr)
$4 = (VirtualLeftRight *) 0x13fab89b0 <vtable for VirtualLeftRight+16>
(gdb) p dynamic_cast<VirtualLeftRight*>(vl)
$5 = (VirtualLeftRight *) 0x13fab89b0 <vtable for VirtualLeftRight+16>
(gdb) p dynamic_cast<VirtualLeftRight*>(vr)
$6 = (VirtualLeftRight *) 0x13fab89b0 <vtable for VirtualLeftRight+16>
(gdb) p dynamic_cast<VirtualLeft*>(vlr)
$7 = (VirtualLeft *) 0x162240
(gdb) p dynamic_cast<VirtualLeft*>(vl)
$8 = (VirtualLeft *) 0x13fab89b0 <vtable for VirtualLeftRight+16>
(gdb) p dynamic_cast<VirtualLeft*>(vr)
$9 = (VirtualLeft *) 0x162240
(gdb) p dynamic_cast<VirtualRight*>(vlr)
$10 = (VirtualRight *) 0x162250
(gdb) p dynamic_cast<VirtualRight*>(vl)
$11 = (VirtualRight *) 0x162250
(gdb) p dynamic_cast<VirtualRight*>(vr)
$12 = (VirtualRight *) 0x13fab89b0 <vtable for VirtualLeftRight+16>
```

For the cases where the dynamic_cast type is the same as the
original type, it used the ARG value for the result, which in
case of pointer types was already the dereferenced value.

And the TEM value at the value address was created with the
pointer/reference type, not the actual class type.

With these fixed, the dynamic_cast results make more sense:
```
(gdb) p vlr
$1 = (VirtualLeftRight *) 0x692240
(gdb) p vl
$2 = (VirtualLeft *) 0x692240
(gdb) p vr
$3 = (VirtualRight *) 0x692250
(gdb) p dynamic_cast<VirtualLeftRight*>(vlr)
$4 = (VirtualLeftRight *) 0x692240
(gdb) p dynamic_cast<VirtualLeftRight*>(vl)
$5 = (VirtualLeftRight *) 0x692240
(gdb) p dynamic_cast<VirtualLeftRight*>(vr)
$6 = (VirtualLeftRight *) 0x692240
(gdb) p dynamic_cast<VirtualLeft*>(vlr)
$7 = (VirtualLeft *) 0x692240
(gdb) p dynamic_cast<VirtualLeft*>(vl)
$8 = (VirtualLeft *) 0x692240
(gdb) p dynamic_cast<VirtualLeft*>(vr)
$9 = (VirtualLeft *) 0x692240
(gdb) p dynamic_cast<VirtualRight*>(vlr)
$10 = (VirtualRight *) 0x692250
(gdb) p dynamic_cast<VirtualRight*>(vl)
$11 = (VirtualRight *) 0x692250
(gdb) p dynamic_cast<VirtualRight*>(vr)
$12 = (VirtualRight *) 0x692250
```

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29011
---
 gdb/testsuite/gdb.cp/casts.cc  | 22 ++++++++++++++++++++++
 gdb/testsuite/gdb.cp/casts.exp | 20 ++++++++++++++++++++
 gdb/valops.c                   | 12 ++++++++----
 3 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.cp/casts.cc b/gdb/testsuite/gdb.cp/casts.cc
index f64d13c26df..5c7f9dc8a1c 100644
--- a/gdb/testsuite/gdb.cp/casts.cc
+++ b/gdb/testsuite/gdb.cp/casts.cc
@@ -50,6 +50,24 @@ struct LeftRight : public Left, public Right
 {
 };
 
+struct VirtualLeft
+{
+  virtual ~VirtualLeft () {}
+
+  int left;
+};
+
+struct VirtualRight
+{
+  virtual ~VirtualRight () {}
+
+  int right;
+};
+
+struct VirtualLeftRight : public VirtualLeft, public VirtualRight
+{
+};
+
 int
 main (int argc, char **argv)
 {
@@ -70,5 +88,9 @@ main (int argc, char **argv)
   unsigned long long gd_value = (unsigned long long) (std::uintptr_t)&gd;
   unsigned long long r_value = (unsigned long long) (Right *) &gd;
 
+  VirtualLeftRight *vlr = new VirtualLeftRight ();
+  VirtualLeft *vl = vlr;
+  VirtualRight *vr = vlr;
+
   return 0;  /* breakpoint spot: casts.exp: 1 */
 }
diff --git a/gdb/testsuite/gdb.cp/casts.exp b/gdb/testsuite/gdb.cp/casts.exp
index 41bb1259270..a0c73826fa8 100644
--- a/gdb/testsuite/gdb.cp/casts.exp
+++ b/gdb/testsuite/gdb.cp/casts.exp
@@ -180,6 +180,26 @@ gdb_test "print (unsigned long long) (LeftRight *) (Right *) &gd == gd_value" \
 gdb_test "print (unsigned long long) (LeftRight *) (Right *) r_value == gd_value" \
     " = true"
 
+gdb_test "print dynamic_cast<VirtualLeftRight *> (vlr) == vlr" " = true"
+gdb_test "print dynamic_cast<VirtualLeftRight *> (vl) == vlr" " = true"
+gdb_test "print dynamic_cast<VirtualLeftRight *> (vr) == vlr" " = true"
+gdb_test "print dynamic_cast<VirtualLeft *> (vlr) == vl" " = true"
+gdb_test "print dynamic_cast<VirtualLeft *> (vl) == vl" " = true"
+gdb_test "print dynamic_cast<VirtualLeft *> (vr) == vl" " = true"
+gdb_test "print dynamic_cast<VirtualRight *> (vlr) == vr" " = true"
+gdb_test "print dynamic_cast<VirtualRight *> (vl) == vr" " = true"
+gdb_test "print dynamic_cast<VirtualRight *> (vr) == vr" " = true"
+
+gdb_test "print &dynamic_cast<VirtualLeftRight &> (*vlr) == vlr" " = true"
+gdb_test "print &dynamic_cast<VirtualLeftRight &> (*vl) == vlr" " = true"
+gdb_test "print &dynamic_cast<VirtualLeftRight &> (*vr) == vlr" " = true"
+gdb_test "print &dynamic_cast<VirtualLeft &> (*vlr) == vl" " = true"
+gdb_test "print &dynamic_cast<VirtualLeft &> (*vl) == vl" " = true"
+gdb_test "print &dynamic_cast<VirtualLeft &> (*vr) == vl" " = true"
+gdb_test "print &dynamic_cast<VirtualRight &> (*vlr) == vr" " = true"
+gdb_test "print &dynamic_cast<VirtualRight &> (*vl) == vr" " = true"
+gdb_test "print &dynamic_cast<VirtualRight &> (*vr) == vr" " = true"
+
 if {[prepare_for_testing "failed to prepare" ${testfile}03 $srcfile2 \
 			 {debug c++ additional_flags=-std=c++03}]} {
     return -1
diff --git a/gdb/valops.c b/gdb/valops.c
index b0e95c330a0..49ea1fd7676 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -856,7 +856,7 @@ value_dynamic_cast (struct type *type, struct value *arg)
 
   /* If the classes are the same, just return the argument.  */
   if (class_types_same_p (class_type, arg_type))
-    return value_cast (type, arg);
+    return value_cast (type, original_arg);
 
   /* If the target type is a unique base class of the argument's
      declared type, just cast it.  */
@@ -888,14 +888,18 @@ value_dynamic_cast (struct type *type, struct value *arg)
       && resolved_type->target_type ()->code () == TYPE_CODE_VOID)
     return value_at_lazy (type, addr);
 
-  tem = value_at (type, addr);
-  type = tem->type ();
+  tem = value_at (resolved_type->target_type (), addr);
+  type = (is_ref
+	  ? lookup_reference_type (tem->type (), resolved_type->code ())
+	  : lookup_pointer_type (tem->type ()));
 
   /* The first dynamic check specified in 5.2.7.  */
   if (is_public_ancestor (arg_type, resolved_type->target_type ()))
     {
       if (class_types_same_p (rtti_type, resolved_type->target_type ()))
-	return tem;
+	return (is_ref
+		? value_ref (tem, resolved_type->code ())
+		: value_addr (tem));
       result = NULL;
       if (dynamic_cast_check_1 (resolved_type->target_type (),
 				tem->contents_for_printing ().data (),
-- 
2.35.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fix dynamic_cast
  2023-12-08 21:00 ` [PATCH] Fix dynamic_cast Hannes Domani
@ 2023-12-10 22:08   ` Tom Tromey
  2023-12-11 14:54     ` Hannes Domani
  0 siblings, 1 reply; 3+ messages in thread
From: Tom Tromey @ 2023-12-10 22:08 UTC (permalink / raw)
  To: Hannes Domani; +Cc: gdb-patches

>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

Hannes> PR29011 notes that dynamic_cast does not work correctly if
Hannes> classes with virtual methods are involved, some of the results
Hannes> wrongly point into the vtable of the derived class:

Thank you.  This is ok.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Fix dynamic_cast
  2023-12-10 22:08   ` Tom Tromey
@ 2023-12-11 14:54     ` Hannes Domani
  0 siblings, 0 replies; 3+ messages in thread
From: Hannes Domani @ 2023-12-11 14:54 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

 Am Sonntag, 10. Dezember 2023, 23:08:40 MEZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
>
> Hannes> PR29011 notes that dynamic_cast does not work correctly if
> Hannes> classes with virtual methods are involved, some of the results
> Hannes> wrongly point into the vtable of the derived class:
>
> Thank you.  This is ok.
>
> Approved-By: Tom Tromey <tom@tromey.com>

Pushed, thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-12-11 14:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20231208210016.5260-1-ssbssa.ref@yahoo.de>
2023-12-08 21:00 ` [PATCH] Fix dynamic_cast Hannes Domani
2023-12-10 22:08   ` Tom Tromey
2023-12-11 14:54     ` Hannes Domani

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).