Patchwork Fix dynamic_cast operator

login
register
mail settings
Submitter Siva Chandra Reddy
Date April 8, 2014, 1:09 a.m.
Message ID <CAGyQ6gyX7-JohMxEi0fMKcXUfVAQrJAye+V3Ech9aARFZJAWBg@mail.gmail.com>
Download mbox | patch
Permalink /patch/424/
State New
Headers show

Comments

Siva Chandra Reddy - April 8, 2014, 1:09 a.m.
For my debug methods patch, I have used value_cast instead of
value_dyamic_cast at a few places as the latter was spewing errors.
Digging a bit, I discovered that value_dynamic_cast has been broken
since it was added. The tests have been passing because, 1) some tests
had a strange expected result pattern and, 2) tests are not thorough
enough IMO.

The attached patch fixes the dynamic_cast operator and addresses the
shortcoming #1 of the tests.

Consider the following GDB session to understand #2:

(gdb) p dynamic_cast<B *>(ap)
$1 = (B *) 0x7fffffffdf00

The tests do not verify the hex addresses printed by the dynamic_cast
operator; they merely verify that a hex number is being printed. I
think the tests should verify the address printed (in some direct or
indirect manner). I feel this is important because the target
subobject could be present at different locations in/around the most
derived object based on the type of inheritance. I do not know of a
straight forward way to add this check, but I think it can addressed
in a different patch if required.

ChangeLog:
2014-04-07  Siva Chandra Reddy  <sivachandra@google.com>

        * valops.c (value_dynamic_cast): Fix mixup between rtti_type
        and resolved_type.

        testsuite/
        * gdb.cp/casts.exp: Adjust expected result for few tests.

Patch

diff --git a/gdb/testsuite/gdb.cp/casts.exp b/gdb/testsuite/gdb.cp/casts.exp
index 9122450..dd848db 100644
--- a/gdb/testsuite/gdb.cp/casts.exp
+++ b/gdb/testsuite/gdb.cp/casts.exp
@@ -151,8 +151,7 @@  gdb_test "print dynamic_cast<Alpha &> (derived)" \
     "dynamic_cast simple upcast to reference"
 
 gdb_test "print dynamic_cast<Derived *> (ad)" \
-    " = \\(Derived \\*\\) ${nonzero_hex}( <vtable for Derived.*>)?" \
-    "dynamic_cast simple downcast"
+    " = \\(Derived \\*\\) ${nonzero_hex}" "dynamic_cast simple downcast"
 
 gdb_test "print dynamic_cast<VirtuallyDerived *> (add)" \
     " = \\(VirtuallyDerived \\*\\) $nonzero_hex" \
@@ -167,8 +166,7 @@  gdb_test "print dynamic_cast<VirtuallyDerived &> (*ad)" \
     "dynamic_cast to reference to non-existing base"
 
 gdb_test "print dynamic_cast<DoublyDerived *> (add)" \
-    " = \\(DoublyDerived \\*\\) ${nonzero_hex}( <vtable for DoublyDerived.*>)?" \
-    "dynamic_cast unique downcast"
+    " = \\(DoublyDerived \\*\\) ${nonzero_hex}" "dynamic_cast unique downcast"
 
 gdb_test "print dynamic_cast<Gamma *> (add)" \
     " = \\(Gamma \\*\\) $nonzero_hex" \
diff --git a/gdb/valops.c b/gdb/valops.c
index cf195a3..bd4b963 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -800,13 +800,14 @@  value_dynamic_cast (struct type *type, struct value *arg)
       && TYPE_CODE (TYPE_TARGET_TYPE (resolved_type)) == TYPE_CODE_VOID)
     return value_at_lazy (type, addr);
 
-  tem = value_at (type, addr);
+  tem = value_at (rtti_type, addr);
 
   /* The first dynamic check specified in 5.2.7.  */
   if (is_public_ancestor (arg_type, TYPE_TARGET_TYPE (resolved_type)))
     {
       if (class_types_same_p (rtti_type, TYPE_TARGET_TYPE (resolved_type)))
-	return tem;
+	return value_from_pointer (type, addr);
+
       result = NULL;
       if (dynamic_cast_check_1 (TYPE_TARGET_TYPE (resolved_type),
 				value_contents_for_printing (tem),