This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/2] Fix C++ virtual method pointer resolution
- From: Yao Qi <yao at codesourcery dot com>
- To: Patrick Palka <patrick at parcs dot ath dot cx>
- Cc: <gdb-patches at sourceware dot org>
- Date: Fri, 21 Nov 2014 19:35:10 +0800
- Subject: Re: [PATCH 1/2] Fix C++ virtual method pointer resolution
- Authentication-results: sourceware.org; auth=none
- References: <1411355243-10812-1-git-send-email-patrick at parcs dot ath dot cx>
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 (éå)