Re: [PATCH 1/2] Fix C++ virtual method pointer resolution

[Patrick Palka <patrick at parcs dot ath dot cx>]

On Sun, Sep 21, 2014 at 11:07 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> C++ virtual method pointer resolution is currently broken, in that a
> virtual method pointer will sometimes resolve to the wrong symbol.  A
> minimal testcase is the following program:
>
>   struct x
>   {
>     virtual void f ();
>     virtual void g ();
>   }
>
>   void x::f () { }
>   void x::g () { }
>
> (gdb) print &x::f
> $1 = &virtual x::f()
> (gdb) print &x::g
> $1 = &virtual x::f()
>
> As you can see, &x::f correctly resolves to x::f(), but &x::g
> incorrectly resolves to x::f() instead of x::g().
>
> The issue lies in the initial creation of the virtual method pointer as
> seen by GDB.  In gnuv3_make_method_ptr() we fail to shift the vtable
> offset when storing the first word of a virtual method pointer.  This is
> important because functions that read this first word (namely the
> callers of gnuv3_decode_method_ptr()) expect that the vtable offset is a
> multiple of "sizeof (vtable_ptrdiff_t)".  Also it ensures that the vbit
> tag does not collide with the bits used to store the actual offset.
>
> So when writing the virtual method pointer contents we need to shift the
> vtable offset so as to be in symmetry with what the readers of the
> vtable offset do -- which is, xor the vbit tag and then shift back the
> offset.  (The prominent readers of the vtable offset are
> gnuv3_print_method_ptr() and gnuv3_method_ptr_to_value().)
>
> gdb/ChangeLog
>         * gnu-v3-abi.c (gnuv3_make_method_ptr): Shift the vtable offset
>         before setting the virtual bit.
>
> gdb/testsuite/ChangeLog
>         * gdb.cp/method-ptr.exp: New test.
>         * gdb.cp/method-ptr.cc: New testcase.
>
> Tested on x86_64-unknown-linux-gnu.
> ---
>  gdb/gnu-v3-abi.c                    |  7 ++++-
>  gdb/testsuite/gdb.cp/method-ptr.cc  | 58 +++++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.cp/method-ptr.exp | 60 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 124 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/testsuite/gdb.cp/method-ptr.cc
>  create mode 100644 gdb/testsuite/gdb.cp/method-ptr.exp
>
> diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c
> index d5ed355..ccb0be6 100644
> --- a/gdb/gnu-v3-abi.c
> +++ b/gdb/gnu-v3-abi.c
> @@ -683,7 +683,12 @@ gnuv3_make_method_ptr (struct type *type, gdb_byte *contents,
>
>    if (!gdbarch_vbit_in_delta (gdbarch))
>      {
> -      store_unsigned_integer (contents, size, byte_order, value | is_virtual);
> +      if (is_virtual != 0)
> +       {
> +         value = value * TYPE_LENGTH (vtable_ptrdiff_type (gdbarch));
> +         value = value | 1;
> +       }
> +      store_unsigned_integer (contents, size, byte_order, value);
>        store_unsigned_integer (contents + size, size, byte_order, 0);
>      }
>    else
> diff --git a/gdb/testsuite/gdb.cp/method-ptr.cc b/gdb/testsuite/gdb.cp/method-ptr.cc
> new file mode 100644
> index 0000000..db47484
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/method-ptr.cc
> @@ -0,0 +1,58 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2014 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 x
> +{
> +  virtual void f ();
> +  virtual void g ();
> +  virtual void h ();
> +};
> +
> +void x::f () { }
> +void x::g () { }
> +void x::h () { }
> +
> +struct y : x
> +{
> +  virtual void f ();
> +
> +  virtual void j ();
> +  virtual void k ();
> +};
> +
> +void y::f () { }
> +void y::j () { }
> +void y::k () { }
> +
> +struct z : y
> +{
> +  virtual void g ();
> +  virtual void j ();
> +
> +  virtual void l ();
> +  virtual void m ();
> +};
> +
> +void z::g () { }
> +void z::j () { }
> +void z::l () { }
> +void z::m () { }
> +
> +int
> +main ()
> +{
> +}
> diff --git a/gdb/testsuite/gdb.cp/method-ptr.exp b/gdb/testsuite/gdb.cp/method-ptr.exp
> new file mode 100644
> index 0000000..732b861
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/method-ptr.exp
> @@ -0,0 +1,60 @@
> +# Copyright 2014 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/>.
> +
> +# This file is part of the gdb testsuite
> +
> +if [skip_cplus_tests] { continue }
> +
> +standard_testfile .cc
> +
> +if [prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}] {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +get_debug_format
> +
> +if ![test_debug_format "DWARF 2"] {
> +    return 0
> +}
> +
> +# Check that the virtual method pointer NAME resolves to symbol SYMBOL.
> +proc check_virtual_method_ptr_resolution { name symbol } {
> +    global decimal
> +
> +    # Printing the expression &NAME should show the resolved symbol SYMBOL.
> +    gdb_test "print &$name" "\\$$decimal = &virtual $symbol\\(\\)\\s"
> +}
> +
> +check_virtual_method_ptr_resolution "x::f" "x::f"
> +check_virtual_method_ptr_resolution "x::g" "x::g"
> +check_virtual_method_ptr_resolution "x::h" "x::h"
> +
> +check_virtual_method_ptr_resolution "y::f" "y::f"
> +check_virtual_method_ptr_resolution "y::g" "x::g"
> +check_virtual_method_ptr_resolution "y::h" "x::h"
> +check_virtual_method_ptr_resolution "y::j" "y::j"
> +check_virtual_method_ptr_resolution "y::k" "y::k"
> +
> +check_virtual_method_ptr_resolution "z::f" "y::f"
> +check_virtual_method_ptr_resolution "z::g" "z::g"
> +check_virtual_method_ptr_resolution "z::h" "x::h"
> +check_virtual_method_ptr_resolution "z::j" "z::j"
> +check_virtual_method_ptr_resolution "z::k" "y::k"
> +check_virtual_method_ptr_resolution "z::l" "z::l"
> +check_virtual_method_ptr_resolution "z::m" "z::m"
> --
> 2.1.1.273.g97b8860
>

Ping.