[PATCH] Fix C++ cast of derived class to base class

[PATCH] Fix C++ cast of derived class to base class

[Tom Tromey]

PR c++/28907 points out that casting from a derived class to a base
class fails in some situations.  The problem turned out to be a
missing use of value_embedded_offset.  One peculiarity here is that,
if you managed to construct a pointer-to-derived with an embedded
offset of 0, the cast would work -- for example, one of the two new
tests here passes without the patch.

This embedded offset stuff is an endless source of bugs.  I wonder if
it's possible to get rid of it somehow.

Regression tested on x86-64 Fedora 34.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28907
---
 gdb/testsuite/gdb.cp/casts.cc  | 20 ++++++++++++++++++++
 gdb/testsuite/gdb.cp/casts.exp |  6 ++++++
 gdb/valops.c                   |  2 +-
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.cp/casts.cc b/gdb/testsuite/gdb.cp/casts.cc
index 543db896d3d..ea4dc961793 100644
--- a/gdb/testsuite/gdb.cp/casts.cc
+++ b/gdb/testsuite/gdb.cp/casts.cc
@@ -34,6 +34,20 @@ struct DoublyDerived : public VirtuallyDerived,
 {
 };
 
+struct Left
+{
+  int left;
+};
+
+struct Right
+{
+  int right;
+};
+
+struct LeftRight : public Left, public Right
+{
+};
+
 int
 main (int argc, char **argv)
 {
@@ -48,5 +62,11 @@ main (int argc, char **argv)
   Alpha *ad = &derived;
   Alpha *add = &doublyderived;
 
+  LeftRight gd;
+  gd.left = 23;
+  gd.right = 27;
+  unsigned long long gd_value = (unsigned long long) &gd;
+  unsigned long long r_value = (unsigned long long) (Right *) &gd;
+
   return 0;  /* breakpoint spot: casts.exp: 1 */
 }
diff --git a/gdb/testsuite/gdb.cp/casts.exp b/gdb/testsuite/gdb.cp/casts.exp
index cda870f77a4..5d0a52401a8 100644
--- a/gdb/testsuite/gdb.cp/casts.exp
+++ b/gdb/testsuite/gdb.cp/casts.exp
@@ -174,6 +174,12 @@ gdb_test "print dynamic_cast<Gamma *> (add)" \
     " = \\(Gamma \\*\\) $nonzero_hex" \
     "dynamic_cast to sibling"
 
+gdb_test "print (unsigned long long) &gd == gd_value" " = true"
+gdb_test "print (unsigned long long) (LeftRight *) (Right *) &gd == gd_value" \
+    " = true"
+gdb_test "print (unsigned long long) (LeftRight *) (Right *) r_value == gd_value" \
+    " = 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 75d52b495be..9a3a9b9567c 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -274,7 +274,7 @@ value_cast_structs (struct type *type, struct value *v2)
       if (v)
 	{
 	  /* Downcasting is possible (t1 is superclass of v2).  */
-	  CORE_ADDR addr2 = value_address (v2);
+	  CORE_ADDR addr2 = value_address (v2) + value_embedded_offset (v2);
 
 	  addr2 -= value_address (v) + value_embedded_offset (v);
 	  return value_at (type, addr2);
-- 
2.34.1

Re: [PATCH] Fix C++ cast of derived class to base class

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> PR c++/28907 points out that casting from a derived class to a base
Tom> class fails in some situations.  The problem turned out to be a
Tom> missing use of value_embedded_offset.  One peculiarity here is that,
Tom> if you managed to construct a pointer-to-derived with an embedded
Tom> offset of 0, the cast would work -- for example, one of the two new
Tom> tests here passes without the patch.

I'm checking this in now.

Tom

Re: [PATCH] Fix C++ cast of derived class to base class

[Tom de Vries]

On 4/18/22 17:34, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
> Tom> PR c++/28907 points out that casting from a derived class to a base
> Tom> class fails in some situations.  The problem turned out to be a
> Tom> missing use of value_embedded_offset.  One peculiarity here is that,
> Tom> if you managed to construct a pointer-to-derived with an embedded
> Tom> offset of 0, the cast would work -- for example, one of the two new
> Tom> tests here passes without the patch.
> 
> I'm checking this in now.

The added tests fail with target board unix/-m32.

In more detail, with some additional printing:
...
(gdb) print /x &gd^M
$31 = 0xffffc5c8^M
(gdb) PASS: gdb.cp/casts.exp: print /x &gd
print /x (unsigned long long)&gd^M
$32 = 0xffffc5c8^M
(gdb) PASS: gdb.cp/casts.exp: print /x (unsigned long long)&gd
print /x gd_value^M
$33 = 0xffffffffffffc5c8^M
(gdb) PASS: gdb.cp/casts.exp: print /x gd_value
print (unsigned long long) &gd == gd_value^M
$34 = false^M
(gdb) FAIL: gdb.cp/casts.exp: print (unsigned long long) &gd == gd_value
...

This fixes it:
...
diff --git a/gdb/testsuite/gdb.cp/casts.cc b/gdb/testsuite/gdb.cp/casts.cc
index ea4dc961793..f64d13c26df 100644
--- a/gdb/testsuite/gdb.cp/casts.cc
+++ b/gdb/testsuite/gdb.cp/casts.cc
@@ -1,3 +1,5 @@
+#include <cstdint>
+
  struct A
  {
    int a;
@@ -65,7 +67,7 @@ main (int argc, char **argv)
    LeftRight gd;
    gd.left = 23;
    gd.right = 27;
-  unsigned long long gd_value = (unsigned long long) &gd;
+  unsigned long long gd_value = (unsigned long long) (std::uintptr_t)&gd;
    unsigned long long r_value = (unsigned long long) (Right *) &gd;

    return 0;  /* breakpoint spot: casts.exp: 1 */
...

WDYT?

Thanks,
- Tom

[committed][gdb/testsuite] Fix gdb.cp/casts.exp with -m32

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 1966 bytes --]

[ was: [PATCH] Fix C++ cast of derived class to base class ]

On 5/4/22 19:27, Tom de Vries via Gdb-patches wrote:
> On 4/18/22 17:34, Tom Tromey wrote:
>>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
>>
>> Tom> PR c++/28907 points out that casting from a derived class to a base
>> Tom> class fails in some situations.  The problem turned out to be a
>> Tom> missing use of value_embedded_offset.  One peculiarity here is that,
>> Tom> if you managed to construct a pointer-to-derived with an embedded
>> Tom> offset of 0, the cast would work -- for example, one of the two new
>> Tom> tests here passes without the patch.
>>
>> I'm checking this in now.
> 
> The added tests fail with target board unix/-m32.
> 
> In more detail, with some additional printing:
> ...
> (gdb) print /x &gd^M
> $31 = 0xffffc5c8^M
> (gdb) PASS: gdb.cp/casts.exp: print /x &gd
> print /x (unsigned long long)&gd^M
> $32 = 0xffffc5c8^M
> (gdb) PASS: gdb.cp/casts.exp: print /x (unsigned long long)&gd
> print /x gd_value^M
> $33 = 0xffffffffffffc5c8^M
> (gdb) PASS: gdb.cp/casts.exp: print /x gd_value
> print (unsigned long long) &gd == gd_value^M
> $34 = false^M
> (gdb) FAIL: gdb.cp/casts.exp: print (unsigned long long) &gd == gd_value
> ...
> 
> This fixes it:
> ...
> diff --git a/gdb/testsuite/gdb.cp/casts.cc b/gdb/testsuite/gdb.cp/casts.cc
> index ea4dc961793..f64d13c26df 100644
> --- a/gdb/testsuite/gdb.cp/casts.cc
> +++ b/gdb/testsuite/gdb.cp/casts.cc
> @@ -1,3 +1,5 @@
> +#include <cstdint>
> +
>   struct A
>   {
>     int a;
> @@ -65,7 +67,7 @@ main (int argc, char **argv)
>     LeftRight gd;
>     gd.left = 23;
>     gd.right = 27;
> -  unsigned long long gd_value = (unsigned long long) &gd;
> +  unsigned long long gd_value = (unsigned long long) (std::uintptr_t)&gd;
>     unsigned long long r_value = (unsigned long long) (Right *) &gd;
> 
>     return 0;  /* breakpoint spot: casts.exp: 1 */
> ...
> 
> WDYT?
> 

Committed.

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-testsuite-Fix-gdb.cp-casts.exp-with-m32.patch --]
[-- Type: text/x-patch, Size: 1803 bytes --]

[gdb/testsuite] Fix gdb.cp/casts.exp with -m32

When running test-case gdb.cp/casts.exp with target board unix/-m32, I run
into:
...
(gdb) print (unsigned long long) &gd == gd_value^M
$31 = false^M
(gdb) FAIL: gdb.cp/casts.exp: print (unsigned long long) &gd == gd_value
...

With some additional printing, we can see in more detail why the comparison
fails:
...
(gdb) print /x &gd^M
$31 = 0xffffc5c8^M
(gdb) PASS: gdb.cp/casts.exp: print /x &gd
print /x (unsigned long long)&gd^M
$32 = 0xffffc5c8^M
(gdb) PASS: gdb.cp/casts.exp: print /x (unsigned long long)&gd
print /x gd_value^M
$33 = 0xffffffffffffc5c8^M
(gdb) PASS: gdb.cp/casts.exp: print /x gd_value
print (unsigned long long) &gd == gd_value^M
$34 = false^M
(gdb) FAIL: gdb.cp/casts.exp: print (unsigned long long) &gd == gd_value
...

The gd_value is set by this assignment:
...
  unsigned long long gd_value = (unsigned long long) &gd;
...

The problem here is directly casting from a pointer to a non-pointer-sized
integer.

Fix this by adding an intermediate cast to std::uintptr_t.

Tested on x86_64-linux with native and target board unix/-m32.

---
 gdb/testsuite/gdb.cp/casts.cc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.cp/casts.cc b/gdb/testsuite/gdb.cp/casts.cc
index ea4dc961793..f64d13c26df 100644
--- a/gdb/testsuite/gdb.cp/casts.cc
+++ b/gdb/testsuite/gdb.cp/casts.cc
@@ -1,3 +1,5 @@
+#include <cstdint>
+
 struct A
 {
   int a;
@@ -65,7 +67,7 @@ main (int argc, char **argv)
   LeftRight gd;
   gd.left = 23;
   gd.right = 27;
-  unsigned long long gd_value = (unsigned long long) &gd;
+  unsigned long long gd_value = (unsigned long long) (std::uintptr_t)&gd;
   unsigned long long r_value = (unsigned long long) (Right *) &gd;
 
   return 0;  /* breakpoint spot: casts.exp: 1 */