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

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

On Tue, Sep 30, 2014 at 5:48 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> 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.

Ping.