From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3464 invoked by alias); 16 Nov 2014 03:57:51 -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 3446 invoked by uid 89); 16 Nov 2014 03:57:45 -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-pa0-f50.google.com Received: from mail-pa0-f50.google.com (HELO mail-pa0-f50.google.com) (209.85.220.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Sun, 16 Nov 2014 03:57:42 +0000 Received: by mail-pa0-f50.google.com with SMTP id eu11so20179704pac.9 for ; Sat, 15 Nov 2014 19:57:40 -0800 (PST) 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:from:date :message-id:subject:to:cc:content-type; bh=oK384DDOPizNzZ4mnfQv3MtBADNat5CGkdBtlTE2s60=; b=Ql1ZYN46GVSzNSccS3ac7cqO8qFuvJkEBP88aWFnNsjYYIBIGJeJGxfg91Q2VerbQ/ 7mlZE5aVNmNk9j1qRsqNsliaANnB7b0cFiXaS2qfnzD6+tZtU8tUFG0g1lcEvuboVhCR c2ttPHbNbGvi3r6gjriXKEBUZhRjPu+R3fAJS5iahn3niT3GB12i6gxa5h9QWuErOmpL zpLkzomldKP+A5O4M0MRFhNCFaFkCnT46f9z0OUzdT43peIczszsr7TXWmfPFxew7mtJ zyOzYXP0c1rkmUPiAJZiVR7FZu7A0y9dhP2ETNZfg2rt5gIFU6UCNhpDN7n4/RNB4FKa IOLQ== X-Gm-Message-State: ALoCoQmScOq9TsEn+abqGhPudkD1ffuQS70cSOngCC7+aRuLfH8c2CW8blVVRejw9OA2mQzhF+sf X-Received: by 10.70.41.13 with SMTP id b13mr20550463pdl.20.1416110260163; Sat, 15 Nov 2014 19:57:40 -0800 (PST) MIME-Version: 1.0 Received: by 10.70.31.100 with HTTP; Sat, 15 Nov 2014 19:57:19 -0800 (PST) In-Reply-To: References: <1411355257-10861-1-git-send-email-patrick@parcs.ath.cx> <1411429189-17235-1-git-send-email-patrick@parcs.ath.cx> From: Patrick Palka Date: Sun, 16 Nov 2014 03:57:00 -0000 Message-ID: Subject: Re: [PATCH 2/2] Support setting breakpoints on C++ method pointers To: gdb-patches@sourceware.org Cc: Patrick Palka Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-11/txt/msg00373.txt.bz2 On Tue, Sep 30, 2014 at 5:49 PM, Patrick Palka wrote: > 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. Ping.