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

Patrick Palka
2014-09-30T21:49:06Z

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.