[PING][PATCH v2 PR gdb/16841] virtual inheritance via typedef cannot find base

[PING][PATCH v2 PR gdb/16841] virtual inheritance via typedef cannot find base

[Wei-min Pan]

On 8/23/2018 2:04 PM, Weimin Pan wrote:
> 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. A 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
> $2 = 0
> (gdb) p derived::base::i
> $3 = 0
> (gdb) p derived::i
> $4 = 4196392
>
> To fix the problem, we need to use "vptr" in the virtual class object,
> which indexes to its derived class's vtable, to fetch the virtual base
> offset. The offset is then added to the virtual class object to access
> its member in value_struct_elt_for_reference().
>
> The new function add_virtual_base_offset searches recursively for the
> base class along the class hierarchy and returns the virtual base offset
> for the virtual class.
>
> Tested on amd64-linux-gnu. No regressions.
> ---
>   gdb/ChangeLog                      |    7 +++++
>   gdb/testsuite/ChangeLog            |    6 ++++
>   gdb/testsuite/gdb.cp/virtbase2.cc  |   21 +++++++++++++++
>   gdb/testsuite/gdb.cp/virtbase2.exp |   50 ++++++++++++++++++++++++++++++++++++
>   gdb/valops.c                       |   38 +++++++++++++++++++++++++++
>   5 files changed, 122 insertions(+), 0 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.cp/virtbase2.cc
>   create mode 100644 gdb/testsuite/gdb.cp/virtbase2.exp
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index c47c111..9d86f1c 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,10 @@
> +2018-08-23  Weimin Pan  <weimin.pan@oracle.com>
> +
> +	PR gdb/16841
> +	* valops.c (add_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-06-29  Pedro Alves  <palves@redhat.com>
>   
>   	* gdb/amd64-tdep.h (amd64_create_target_description): Add
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 93c849c..7be2e1d 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-08-23  Weimin Pan  <weimin.pan@oracle.com>
> +
> +	PR gdb/16841
> +	* gdb.cp/virtbase2.cc: New file.
> +	* gdb.cp/virtbase2.exp: New file.
> +
>   2018-06-29  Pedro Alves  <palves@redhat.com>
>   
>   	* gdb.threads/names.exp: Adjust expected "info threads" output.
> diff --git a/gdb/testsuite/gdb.cp/virtbase2.cc b/gdb/testsuite/gdb.cp/virtbase2.cc
> new file mode 100644
> index 0000000..4f7631e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/virtbase2.cc
> @@ -0,0 +1,21 @@
> +struct base {
> +  int i; double d;
> +  base() : i(55), d(6.6) {}
> +};
> +typedef base tbase;
> +struct derived: 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..c29ff1c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/virtbase2.exp
> @@ -0,0 +1,50 @@
> +# 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 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
> +}
> +
> +gdb_breakpoint "derived::func_d"
> +gdb_continue_to_breakpoint "continue to derived::func_d"
> +gdb_test "print i" " = 55" "i in base class"
> +gdb_test "print derived::i" " = 55" "i in base class"
> +gdb_test "print derived::base::i" " = 55" "i in base class"
> +gdb_test "print base::i" " = 55" "i in base class"
> +gdb_test "print d" " = 6.5999999999999996" "d in base class"
> +gdb_test "print derived::d" " = 6.5999999999999996" "d in base class"
> +gdb_test "print derived::base::d" " = 6.5999999999999996" "d in base class"
> +gdb_test "print base::d" " = 6.5999999999999996" "d in base class"
> +gdb_breakpoint "foo::func_f"
> +gdb_continue_to_breakpoint "continue to foo::func_f"
> +gdb_test "print i" " = 55" "i in base class"
> +gdb_test "print derived::i" " = 55" "i in base class"
> +gdb_test "print derived::base::i" " = 55" "i in base class"
> +gdb_test "print base::i" " = 55" "i in base class"
> +gdb_test "print d" " = 6.5999999999999996" "d in base class"
> +gdb_test "print derived::d" " = 6.5999999999999996" "d in base class"
> +gdb_test "print derived::base::d" " = 6.5999999999999996" "d in base class"
> +gdb_test "print base::d" " = 6.5999999999999996" "d in base class"
> diff --git a/gdb/valops.c b/gdb/valops.c
> index 9bdbf22..754e7d0 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -3329,6 +3329,35 @@ 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 and return its offset,
> +   relative to VT, if it is a virtual base member.  */
> +
> +static int
> +add_virtual_base_offset (struct type *vt, struct type *cls,
> +                            struct value *v, int &boffs)
> +{
> +  for (int i = 0; i < TYPE_N_BASECLASSES (vt); i++)
> +    {
> +      struct type *ftype = TYPE_FIELD_TYPE (vt, i);
> +      if (check_typedef (ftype) == 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);
> +            }
> +          return true;
> +        }
> +
> +      if (add_virtual_base_offset (ftype, cls, v, boffs))
> +        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
> @@ -3393,6 +3422,15 @@ 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)
> +		    {
> +		      struct value *v2 = value_of_this_silent (current_language);
> +		      struct type *vtype = check_typedef (value_type (v2));
> +		      struct type *vt = check_typedef (TYPE_TARGET_TYPE (vtype));
> +    		      int base_offs = 0;
> +	    	      if (add_virtual_base_offset (vt, curtype, v, base_offs))
> +      		        mem_offset += base_offs;
> +		    }
>   		  tmp = lookup_pointer_type (TYPE_TARGET_TYPE (type));
>   		  result = value_from_pointer (tmp,
>   					       value_as_long (v) + mem_offset);