From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19498 invoked by alias); 30 Sep 2014 21:49:29 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 19488 invoked by uid 89); 30 Sep 2014 21:49:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 X-HELO: mail-la0-f45.google.com Received: from mail-la0-f45.google.com (HELO mail-la0-f45.google.com) (209.85.215.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 30 Sep 2014 21:49:27 +0000 Received: by mail-la0-f45.google.com with SMTP id q1so9480792lam.32 for ; Tue, 30 Sep 2014 14:49:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=eCQXG904S8lXJfzBoxgz++ac/EUn9EmA/G3fjk8PPgM=; b=a28y0loCUCgXyNZRx5Xt4sN5N9Tt23CnDkdyvN2xdZtNSA1SXFEMsC1HxIeFfaSN5b 9711Tbq5ElditV45TlVq95ZjgXbUu0ogKmco3NpMzo2c9Hc2Z1TblNapGP2OIVkbrCL+ K9znrheOUX6BZAXrXYo+RiummpHwyvQEG9atSR4pZOMcO1cgm3XtN1VvgAjWa6aTBKe9 6Q0d5kLNSVP0vG1E/UVLmvy7r/IVcOqdSJqtU0WTwz0eBhNOUnDjw3LeBBZk9/SjjTue CkpI6/CrtH5mM3mKFrZOf2T7C3lV8dOf/zaV4sEHUSQagDAmDfNTUFWl+YwDOfvXzA2g ERlQ== X-Gm-Message-State: ALoCoQmU+utTYDBpCEm+fU9rF7HHTeB7T+/6DccUeIKDFeL1RcILYTYxcGwtk2RTSGhgSTTQdj24 MIME-Version: 1.0 X-Received: by 10.112.14.199 with SMTP id r7mr47720283lbc.58.1412113763663; Tue, 30 Sep 2014 14:49:23 -0700 (PDT) Received: by 10.25.30.79 with HTTP; Tue, 30 Sep 2014 14:49:23 -0700 (PDT) In-Reply-To: <1411429189-17235-1-git-send-email-patrick@parcs.ath.cx> References: <1411355257-10861-1-git-send-email-patrick@parcs.ath.cx> <1411429189-17235-1-git-send-email-patrick@parcs.ath.cx> Date: Tue, 30 Sep 2014 21:49:00 -0000 Message-ID: Subject: Re: [PATCH 2/2] Support setting breakpoints on C++ method pointers From: Patrick Palka To: gdb-patches@sourceware.org Cc: Patrick Palka Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-09/txt/msg00912.txt.bz2 On Mon, Sep 22, 2014 at 7:39 PM, Patrick Palka 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.