This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/2] Support setting breakpoints on C++ method pointers
- From: Patrick Palka <patrick at parcs dot ath dot cx>
- To: gdb-patches at sourceware dot org
- Cc: Patrick Palka <patrick at parcs dot ath dot cx>
- Date: Tue, 30 Sep 2014 17:49:23 -0400
- Subject: Re: [PATCH 2/2] Support setting breakpoints on C++ method pointers
- Authentication-results: sourceware.org; auth=none
- References: <1411355257-10861-1-git-send-email-patrick at parcs dot ath dot cx> <1411429189-17235-1-git-send-email-patrick at parcs dot ath dot cx>
On Mon, Sep 22, 2014 at 7:39 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> (Oops, I accidentally sent the older version of this patch a 2nd time.
> Sorry for the noise.)
>
> { v2: I noticed that non-virtual method pointers are also affected by
> this commit. That is, breaking on non-virtual method pointers now also
> works whereas previously it didn't. So I rewrote the commit message and
> augmented the testcase to also test non-virtual method pointers. I also
> tweaked the documentation for the function cplus_method_ptr_to_value(). }
>
> -- >8 --
>
> This patch adds support for setting breakpoints on C++ method pointers,
> both virtual and non-virtual. For example:
>
> struct x
> {
> virtual void f ();
> void g ();
> }
>
> void x::f () { }
> void x::g () { }
>
> struct y : x
> {
> }
>
> (gdb) break *&y::f
> Value can't be converted to integer.
> (gdb) break *&y::g
> Value can't be converted to integer.
>
> Breaking on these expressions doesn't currently work because GDB doesn't
> know how to convert a METHODPTR to an address in the function
> value_as_address(). We have to teach value_as_address() how to extract
> a symbolic address out from a METHODPTR in order for the above example
> to work.
>
> This patch tweaks value_as_address() to call cplus_method_ptr_to_value()
> in order to extract a pointer value out of a METHODPTR. The latter
> function does most of the work, but it needs a few tweaks. Firstly,
> this patch makes the first argument of gnuv3_method_ptr_to_value(), i.e.
> the argument corresponding to a "this" object pointer, optional.
> Secondly, when the "this" pointer is omitted and a virtual method
> pointer is passed in then we attempt to extract a pointer value by doing
> a lookup of the virtual method pointer's symbolic name in the symbol
> table.
>
> Tested on x86_64-unknown-linux-gnu.
>
> gdb/ChangeLog
> * cp-abi.h (cplus_method_ptr_to_value): Document behavior for a
> THIS_P that's NULL.
> * gnu-v3-abi.c (gnuv3_method_ptr_to_value): Support method
> resolution without a given "this" object.
> * value.c (value_as_address): Handle TYPE_CODE_METHODPTR values.
>
> gdb/testsuite/ChangeLog
> * gdb.cp/method-ptr.exp: Add tests for breaking on non-virtual
> method pointers.
> (check_virtual_method_ptr_resolution): New parameter VIRTUAL.
> Also test that setting a breakpoint on the given method pointer
> works correctly.
> * gdb.cp/method-ptr.cc: Introduce a few non-virtual methods
> to test.
> ---
> gdb/cp-abi.h | 4 ++-
> gdb/gnu-v3-abi.c | 65 +++++++++++++++++++++++++------------
> gdb/testsuite/gdb.cp/method-ptr.cc | 6 ++++
> gdb/testsuite/gdb.cp/method-ptr.exp | 23 +++++++++++--
> gdb/value.c | 8 +++++
> 5 files changed, 81 insertions(+), 25 deletions(-)
>
> diff --git a/gdb/cp-abi.h b/gdb/cp-abi.h
> index 7d4b7f3..1f38605 100644
> --- a/gdb/cp-abi.h
> +++ b/gdb/cp-abi.h
> @@ -162,7 +162,9 @@ void cplus_print_method_ptr (const gdb_byte *contents,
> int cplus_method_ptr_size (struct type *to_type);
>
> /* Return the method which should be called by applying METHOD_PTR to
> - *THIS_P, and adjust *THIS_P if necessary. */
> + *THIS_P, and adjust *THIS_P if necessary. If THIS_P is NULL then
> + return the method that would be called if METHOD_PTR was applied
> + to an object of METHOD_PTR's domain type (e.g. the type X in &X::f). */
> struct value *cplus_method_ptr_to_value (struct value **this_p,
> struct value *method_ptr);
>
> diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c
> index ccb0be6..718694b 100644
> --- a/gdb/gnu-v3-abi.c
> +++ b/gdb/gnu-v3-abi.c
> @@ -719,36 +719,59 @@ gnuv3_method_ptr_to_value (struct value **this_p, struct value *method_ptr)
> gdbarch = get_type_arch (domain_type);
> vbit = gnuv3_decode_method_ptr (gdbarch, contents, &ptr_value, &adjustment);
>
> - /* First convert THIS to match the containing type of the pointer to
> - member. This cast may adjust the value of THIS. */
> - *this_p = value_cast (final_type, *this_p);
> + if (this_p != NULL)
> + {
> + /* First convert THIS to match the containing type of the pointer to
> + member. This cast may adjust the value of THIS. */
> + *this_p = value_cast (final_type, *this_p);
>
> - /* Then apply whatever adjustment is necessary. This creates a somewhat
> - strange pointer: it claims to have type FINAL_TYPE, but in fact it
> - might not be a valid FINAL_TYPE. For instance, it might be a
> - base class of FINAL_TYPE. And if it's not the primary base class,
> - then printing it out as a FINAL_TYPE object would produce some pretty
> - garbage.
> + /* Then apply whatever adjustment is necessary. This creates a somewhat
> + strange pointer: it claims to have type FINAL_TYPE, but in fact it
> + might not be a valid FINAL_TYPE. For instance, it might be a
> + base class of FINAL_TYPE. And if it's not the primary base class,
> + then printing it out as a FINAL_TYPE object would produce some pretty
> + garbage.
>
> - But we don't really know the type of the first argument in
> - METHOD_TYPE either, which is why this happens. We can't
> - dereference this later as a FINAL_TYPE, but once we arrive in the
> - called method we'll have debugging information for the type of
> - "this" - and that'll match the value we produce here.
> + But we don't really know the type of the first argument in
> + METHOD_TYPE either, which is why this happens. We can't
> + dereference this later as a FINAL_TYPE, but once we arrive in the
> + called method we'll have debugging information for the type of
> + "this" - and that'll match the value we produce here.
>
> - You can provoke this case by casting a Base::* to a Derived::*, for
> - instance. */
> - *this_p = value_cast (builtin_type (gdbarch)->builtin_data_ptr, *this_p);
> - *this_p = value_ptradd (*this_p, adjustment);
> - *this_p = value_cast (final_type, *this_p);
> + You can provoke this case by casting a Base::* to a Derived::*, for
> + instance. */
> + *this_p = value_cast (builtin_type (gdbarch)->builtin_data_ptr, *this_p);
> + *this_p = value_ptradd (*this_p, adjustment);
> + *this_p = value_cast (final_type, *this_p);
> + }
>
> if (vbit)
> {
> LONGEST voffset;
>
> voffset = ptr_value / TYPE_LENGTH (vtable_ptrdiff_type (gdbarch));
> - return gnuv3_get_virtual_fn (gdbarch, value_ind (*this_p),
> - method_type, voffset);
> +
> + /* If we don't have a "this" object to apply the method pointer to,
> + then retrieve the value of the virtual method by looking up its
> + symbolic name within the symbol table. */
> + if (this_p == NULL)
> + {
> + const char *physname;
> + struct symbol *sym;
> +
> + physname = gnuv3_find_method_in (domain_type, voffset, adjustment);
> + if (physname == NULL)
> + return NULL;
> +
> + sym = lookup_symbol (physname, NULL, VAR_DOMAIN, NULL);
> + if (sym == NULL)
> + return NULL;
> +
> + return value_of_variable (sym, NULL);
> + }
> + else
> + return gnuv3_get_virtual_fn (gdbarch, value_ind (*this_p),
> + method_type, voffset);
> }
> else
> return value_from_pointer (lookup_pointer_type (method_type), ptr_value);
> diff --git a/gdb/testsuite/gdb.cp/method-ptr.cc b/gdb/testsuite/gdb.cp/method-ptr.cc
> index db47484..4e1524a 100644
> --- a/gdb/testsuite/gdb.cp/method-ptr.cc
> +++ b/gdb/testsuite/gdb.cp/method-ptr.cc
> @@ -20,11 +20,14 @@ struct x
> virtual void f ();
> virtual void g ();
> virtual void h ();
> +
> + void a ();
> };
>
> void x::f () { }
> void x::g () { }
> void x::h () { }
> +void x::a () { }
>
> struct y : x
> {
> @@ -32,11 +35,14 @@ struct y : x
>
> virtual void j ();
> virtual void k ();
> +
> + void b ();
> };
>
> void y::f () { }
> void y::j () { }
> void y::k () { }
> +void y::b () { }
>
> struct z : y
> {
> diff --git a/gdb/testsuite/gdb.cp/method-ptr.exp b/gdb/testsuite/gdb.cp/method-ptr.exp
> index 732b861..ca6be4b 100644
> --- a/gdb/testsuite/gdb.cp/method-ptr.exp
> +++ b/gdb/testsuite/gdb.cp/method-ptr.exp
> @@ -33,23 +33,38 @@ 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 } {
> +# Check that the method pointer NAME resolves to symbol SYMBOL. Set VIRTUAL
> +# to 1 if NAME is a virtual method pointer (default), 0 otherwise.
> +proc check_virtual_method_ptr_resolution { name symbol {virtual 1} } {
> global decimal
>
> # Printing the expression &NAME should show the resolved symbol SYMBOL.
> - gdb_test "print &$name" "\\$$decimal = &virtual $symbol\\(\\)\\s"
> + if {$virtual != 0} {
> + gdb_test "print &$name" "\\s&virtual $symbol\\(\\)\\s"
> + } else {
> + gdb_test "print &$name" "\\s<$symbol\\(\\)>\\s"
> + }
> +
> + # Breaking on the expression &NAME should create a breakpoint on the symbol
> + # SYMBOL.
> + set breakpoint_line [gdb_get_line_number $symbol]
> + gdb_test "break *&$name" \
> + "Breakpoint $decimal at .*, line $breakpoint_line\\.\\s"
> + delete_breakpoints
> }
>
> 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 "x::a" "x::a" 0
>
> 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 "y::a" "x::a" 0
> +check_virtual_method_ptr_resolution "y::b" "y::b" 0
>
> check_virtual_method_ptr_resolution "z::f" "y::f"
> check_virtual_method_ptr_resolution "z::g" "z::g"
> @@ -58,3 +73,5 @@ 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"
> +check_virtual_method_ptr_resolution "z::a" "x::a" 0
> +check_virtual_method_ptr_resolution "z::b" "y::b" 0
> diff --git a/gdb/value.c b/gdb/value.c
> index fdc8858d..63ff363 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -2639,6 +2639,14 @@ value_as_address (struct value *val)
> return gdbarch_addr_bits_remove (gdbarch, value_as_long (val));
> #else
>
> + if (TYPE_CODE (value_type (val)) == TYPE_CODE_METHODPTR)
> + {
> + val = cplus_method_ptr_to_value (NULL, val);
> + if (val == NULL)
> + error (_("Method pointer can't be converted to an address."));
> + }
> +
> +
> /* There are several targets (IA-64, PowerPC, and others) which
> don't represent pointers to functions as simply the address of
> the function's entry point. For example, on the IA-64, a
> --
> 2.1.1.273.g97b8860
>
Ping.