[binutils-gdb] Finding data member in virtual base class

[Weimin Pan <wmpan at sourceware dot org>]

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

commit 9f6b697b0efd4ba4e2cb21ac17d2b18a23f81abd
Author: Weimin Pan <weimin.pan@oracle.com>
Date:   Mon Oct 8 21:16:59 2018 +0000

    Finding data member in virtual base class
    
    This patch fixes the original problem - printing member in a virtual base,
    using various expressions, do not yield the same value. Simple test case
    below demonstrates the problem:
    
    % cat t.cc
    struct base { int i; };
    typedef base tbase;
    struct derived: virtual tbase { void func() { } };
    int main() { derived().func(); }
    % g++ -g t.cc
    % gdb a.out
    (gdb) break derived::func
    (gdb) run
    (gdb) p i
    $1 = 0
    (gdb) p base::i
    $3 = 0
    (gdb) p derived::i
    $4 = 4196392
    
    To fix the problem, add function get_baseclass_offset() which searches
    recursively for the base class along the class hierarchy. If the base
    is virtual, it uses "vptr" in virtual class object, which indexes to
    its derived class's vtable, to get and returns the baseclass offset.
    If the base is non-virtual, it returns the accumulated offset of its
    parent classes. The offset is then added to the address of the class
    object to access its member in value_struct_elt_for_reference().

Diff:
---
 gdb/ChangeLog                      |   7 +++
 gdb/testsuite/ChangeLog            |   6 ++
 gdb/testsuite/gdb.cp/virtbase2.cc  |  49 ++++++++++++++++
 gdb/testsuite/gdb.cp/virtbase2.exp | 111 +++++++++++++++++++++++++++++++++++++
 gdb/valops.c                       |  63 ++++++++++++++++++++-
 5 files changed, 235 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0218bc2..2bbccd5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2018-10-08  Weimin Pan  <weimin.pan@oracle.com>
+
+	PR c++/16841
+	* valops.c (get_virtual_base_offset): New function.
+	(value_struct_elt_for_reference): Use it to get virtual base offset
+	and add it in calculating class member address.
+
 2018-10-08   John Darrington <john@darrington.wattle.id.au>
 
     * dwarf2read.c (dwarf2_cu) <producer_is_codewarrior>: New field.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 9473646..d35abb5 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2018-10-08  Weimin Pan  <weimin.pan@oracle.com>
+
+	PR c++/16841
+	* gdb.cp/virtbase2.cc: New file.
+	* gdb.cp/virtbase2.exp: New file.
+
 2018-10-06  Tom Tromey  <tom@tromey.com>
 
 	PR python/19399:
diff --git a/gdb/testsuite/gdb.cp/virtbase2.cc b/gdb/testsuite/gdb.cp/virtbase2.cc
new file mode 100644
index 0000000..8b2fb77
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/virtbase2.cc
@@ -0,0 +1,49 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2018 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+struct super {
+  int w;
+  super () : w(17) {}
+};
+
+struct superbase {
+  int x;
+  superbase () : x(22) {}
+};
+
+struct base : superbase {
+  int i;
+  double d;
+  base() : i(55), d(6.25) {}
+};
+
+typedef base tbase;
+struct derived: virtual super, virtual tbase
+{
+  void func_d() { }
+};
+
+struct foo: virtual derived
+{
+  void func_f() { }
+};
+
+int main()
+{
+  derived().func_d();
+  foo().func_f();
+}
diff --git a/gdb/testsuite/gdb.cp/virtbase2.exp b/gdb/testsuite/gdb.cp/virtbase2.exp
new file mode 100644
index 0000000..cfb48cf
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/virtbase2.exp
@@ -0,0 +1,111 @@
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Make sure printing virtual base class data member works correctly (PR16841)
+
+if { [skip_cplus_tests] } { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+if {![runto_main]} then {
+    perror "couldn't run to main"
+    continue
+}
+
+# From a list of nested scopes, generate all possible ways of accessing something
+# in those scopes.  For example, with the argument {foo bar baz}, this proc will
+# return:
+#  - {} (empty string)
+#  - baz::
+#  - bar::
+#  - bar::baz::
+#  - foo::
+#  - foo::baz::
+#  - foo::bar::
+#  - foo::bar::baz::
+
+proc make_scope_list { scopes } {
+    if { [llength $scopes] == 1 } {
+        return [list "" "${scopes}::"]
+    }
+
+    # Pop the first element, save the first scope.
+    set this_scope [lindex $scopes 0]
+    set scopes [lreplace $scopes 0 0]
+
+    set child_result [make_scope_list $scopes]
+
+    # Add a copy of the child's result without this scope...
+    set result $child_result
+
+    # ... and a copy of the child's result with this scope.
+    foreach r $child_result {
+        lappend result "${this_scope}::$r"
+    }
+
+    return $result
+}
+
+proc test_variables_in_base { scopes } {
+    foreach scope [make_scope_list $scopes] {
+        gdb_test "print ${scope}i" " = 55"
+        gdb_test "print ${scope}d" " = 6.25"
+        gdb_test "print ${scope}x" " = 22"
+    }
+}
+
+proc test_variables_in_superbase { scopes } {
+    foreach scope [make_scope_list $scopes] {
+        gdb_test "print ${scope}x" " = 22"
+    }
+}
+
+proc test_variables_in_super { scopes } {
+    foreach scope [make_scope_list $scopes] {
+        gdb_test "print ${scope}w" " = 17"
+    }
+}
+
+with_test_prefix "derived::func_d" {
+    gdb_breakpoint "derived::func_d"
+    gdb_continue_to_breakpoint "continue to derived::func_d"
+    test_variables_in_base {derived base}
+    test_variables_in_superbase {derived base superbase}
+    test_variables_in_superbase {base superbase}
+    test_variables_in_superbase {derived superbase}
+    test_variables_in_superbase {superbase}
+    test_variables_in_superbase {base}
+    test_variables_in_super {super}
+    test_variables_in_super {derived super}
+}
+
+with_test_prefix "foo::func_f" {
+    gdb_breakpoint "foo::func_f"
+    gdb_continue_to_breakpoint "continue to foo::func_f"
+    test_variables_in_base {foo derived base}
+    test_variables_in_base {foo base}
+    test_variables_in_base {base}
+    test_variables_in_superbase {superbase}
+    test_variables_in_superbase {foo superbase}
+    test_variables_in_superbase {foo derived superbase}
+    test_variables_in_superbase {foo derived base superbase}
+    test_variables_in_super {super}
+    test_variables_in_super {foo super}
+    test_variables_in_super {foo derived super}
+}
diff --git a/gdb/valops.c b/gdb/valops.c
index e6e11a6..99b1275 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -3321,6 +3321,49 @@ compare_parameters (struct type *t1, struct type *t2, int skip_artificial)
   return 0;
 }
 
+/* C++: Given an aggregate type VT, and a class type CLS, search
+   recursively for CLS using value V; If found, store the offset
+   which is either fetched from the virtual base pointer if CLS
+   is virtual or accumulated offset of its parent classes if
+   CLS is non-virtual in *BOFFS, set ISVIRT to indicate if CLS
+   is virtual, and return true.  If not found, return false.  */
+
+static bool
+get_baseclass_offset (struct type *vt, struct type *cls,
+		      struct value *v, int *boffs, bool *isvirt)
+{
+  for (int i = 0; i < TYPE_N_BASECLASSES (vt); i++)
+    {
+      struct type *t = TYPE_FIELD_TYPE (vt, i);
+      if (types_equal (t, cls))
+        {
+          if (BASETYPE_VIA_VIRTUAL (vt, i))
+            {
+	      const gdb_byte *adr = value_contents_for_printing (v);
+	      *boffs = baseclass_offset (vt, i, adr, value_offset (v),
+					 value_as_long (v), v);
+	      *isvirt = true;
+            }
+          else
+	    *isvirt = false;
+          return true;
+        }
+
+      if (get_baseclass_offset (check_typedef (t), cls, v, boffs, isvirt))
+        {
+	  if (*isvirt == false)	/* Add non-virtual base offset.  */
+	    {
+	      const gdb_byte *adr = value_contents_for_printing (v);
+	      *boffs += baseclass_offset (vt, i, adr, value_offset (v),
+					  value_as_long (v), v);
+	    }
+	  return true;
+	}
+    }
+
+  return false;
+}
+
 /* C++: Given an aggregate type CURTYPE, and a member name NAME,
    return the address of this member as a "pointer to member" type.
    If INTYPE is non-null, then it will be the type of the member we
@@ -3374,7 +3417,7 @@ value_struct_elt_for_reference (struct type *domain, int offset,
 	      struct value *v = value_of_this_silent (current_language);
 	      if (v != NULL)
 		{
-		  struct value *ptr;
+		  struct value *ptr, *this_v = v;
 		  long mem_offset;
 		  struct type *type, *tmp;
 
@@ -3385,6 +3428,24 @@ value_struct_elt_for_reference (struct type *domain, int offset,
 		  tmp = lookup_pointer_type (TYPE_SELF_TYPE (type));
 		  v = value_cast_pointers (tmp, v, 1);
 		  mem_offset = value_as_long (ptr);
+		  if (domain != curtype)
+		    {
+		      /* Find class offset of type CURTYPE from either its
+			 parent type DOMAIN or the type of implied this.  */
+		      int boff = 0;
+		      bool isvirt = false;
+		      if (get_baseclass_offset (domain, curtype, v, &boff,
+						&isvirt))
+		        mem_offset += boff;
+		      else
+		        {
+		          struct type *t = check_typedef (value_type (this_v));
+		          t = check_typedef (TYPE_TARGET_TYPE (t));
+		          if (get_baseclass_offset (t, curtype, this_v,
+						    &boff, &isvirt))
+		            mem_offset += boff;
+		        }
+		    }
 		  tmp = lookup_pointer_type (TYPE_TARGET_TYPE (type));
 		  result = value_from_pointer (tmp,
 					       value_as_long (v) + mem_offset);