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

[Yao Qi <yao@codesourcery.com>]

Patrick Palka <patrick@parcs.ath.cx> writes:

Hi,
Sorry for the delayed review...

> 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().)

I am not familiar with how gdb handle c++ virtual method, but your
analysis looks right to me.  I spend the whole day reading c++ abi, but
still don't know how to connect the abi with the code here :(

> 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));

We need to hoist this shift out of "if" block, so that the path goes to
"else" branch can be covered too.  Otherwise, fails in
gdb.cp/method-ptr.exp can't be fixed on arm-linux target (on which
vbit_in_delta is zero).

> +
> +get_debug_format
> +
> +if ![test_debug_format "DWARF 2"] {
> +    return 0
> +}

Why do we need to check test_debug_format here?

-- 
Yao (齐尧)