From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 120784 invoked by alias); 8 Sep 2018 22:06: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 120538 invoked by uid 89); 8 Sep 2018 22:06:50 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.3 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.2 spammy=Supply, I'd, Id, converts X-HELO: sesbmg23.ericsson.net Received: from sesbmg23.ericsson.net (HELO sesbmg23.ericsson.net) (193.180.251.37) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 08 Sep 2018 22:06:47 +0000 DKIM-Signature: v=1; a=rsa-sha256; d=ericsson.com; s=mailgw201801; c=relaxed/simple; q=dns/txt; i=@ericsson.com; t=1536444405; h=From:Sender:Reply-To:Subject:Date:Message-ID:To:Cc:MIME-Version:Content-Type: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=WcYfCq9KyCI33a47DOAkImK8Ea5ZNeGiEqZzF3rCLdo=; b=gE+fsflh33eEbUx5E20+xiP2NUOCUdd2Z3WrcJtyPu+Qra0z/5TEiu70lIR8Q6bA NU0MtTz/g2jscm4HzSw3j9JE53A9Ht96ad9XUVIYku4prxFrFEnSeONtPsGI517E BTBoajUxc7PN7t2qrOvVTOI1DxsbkBqE7YPetUhXsWU=; Received: from ESESSMB501.ericsson.se (Unknown_Domain [153.88.183.119]) by sesbmg23.ericsson.net (Symantec Mail Security) with SMTP id FE.B0.05037.4F7449B5; Sun, 9 Sep 2018 00:06:44 +0200 (CEST) Received: from ESESBMB504.ericsson.se (153.88.183.171) by ESESSMB501.ericsson.se (153.88.183.162) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3; Sun, 9 Sep 2018 00:06:14 +0200 Received: from NAM05-BY2-obe.outbound.protection.outlook.com (153.88.183.157) by ESESBMB504.ericsson.se (153.88.183.171) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3 via Frontend Transport; Sun, 9 Sep 2018 00:06:13 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ericsson.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ewBrjrcwDGbX6B1zNJISmGM2XVM8K0bbEaDnyaaq8KI=; b=ey3zLmmlfZK/3nudJ3o520VwFp5G1MDLlHOhUAu5NvB2J3NopEcMff2Z1dMTY+FAqJgZSrTEQYZzhOvGN5+655K/1eS86nKCUCFP05kHF4OeYsw/3gdjBe45EMD3ALqYQpqyXqxTD7wXDes2433e8vuJ9wqUt77oUt5y4pJCmFg= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; Received: from [100.94.32.129] (192.176.1.81) by DM6PR15MB2395.namprd15.prod.outlook.com (2603:10b6:5:8d::29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1101.18; Sat, 8 Sep 2018 22:06:10 +0000 Subject: Re: [PATCHv4] gdb: Add default frame methods to gdbarch To: Andrew Burgess , References: <20180907212619.27618-1-andrew.burgess@embecosm.com> From: Simon Marchi Message-ID: <77e448ed-d394-88b3-c73a-ab87ec2f8c38@ericsson.com> Date: Sat, 08 Sep 2018 22:06:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180907212619.27618-1-andrew.burgess@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-Path: simon.marchi@ericsson.com Received-SPF: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) X-IsSubscribed: yes X-SW-Source: 2018-09/txt/msg00208.txt.bz2 On 2018-09-07 10:26 PM, Andrew Burgess wrote: > This is a new version of the patch series that I original submitted > here: > > https://sourceware.org/ml/gdb-patches/2018-03/msg00165.html > > That patch did get some feedback, which led to some revisions, the > final version was posted here: > > https://sourceware.org/ml/gdb-patches/2018-06/msg00090.html > > However, this didn't get any feedback :-( Sorry about that, my bad :(. For v2 and subsequent versions, I tend to let the original reviewer do the follow-up. I forgot that this person was me. > In this new version I have changed my approach. Rather than a series > of patches, each supplying a new default gdbarch method and converting > all targets, here I provide one patch that supplies just the new > gdbarch methods, and converts no targets. > > No target should change once this patch is merged, as previously all > the targets had to supply these methods themselves anyway. > > Once this patch is merged then I'll supply a series of patches based > off of: > > https://sourceware.org/ml/gdb-patches/2018-06/msg00092.html > https://sourceware.org/ml/gdb-patches/2018-06/msg00091.html > https://sourceware.org/ml/gdb-patches/2018-06/msg00093.html > https://sourceware.org/ml/gdb-patches/2018-06/msg00094.html > > that converts one target at a time to use the default gdbarch methods > (where that is appropriate). I don't want to waste my time putting > those patches together if I still can't get this patch in, but > hopefully the above links should show you what my general intention > is. > > All feedback welcome. > > Thanks, > Andrew > > --- > > Supply default gdbarch methods for gdbarch_dummy_id, > gdbarch_unwind_pc, and gdbarch_unwind_sp. This patch doesn't actually > convert any targets to use these methods, and so, there will be no > user visible changes after this commit. > > gdb/ChangeLog: > > * gdb/dummy-frame.c (default_dummy_id): Defined new function. > * gdb/dummy-frame.h (default_dummy_id): Declare new function. > * gdb/frame-unwind.c (default_unwind_pc): Define new function. > (default_unwind_sp): Define new function. > * gdb/frame-unwind.h (default_unwind_pc): Declare new function. > (default_unwind_sp): Declare new function. > * gdb/frame.c (frame_unwind_pc): Assume gdbarch_unwind_pc is > available. > (get_frame_sp): Assume that gdbarch_unwind_sp is available. > * gdb/gdbarch.c: Regenerate. > * gdb/gdbarch.h: Regenerate. > * gdb/gdbarch.sh: Update definition of dummy_id, unwind_pc, and > unwind_sp. Add additional header files to be included in > generated file. > --- > gdb/ChangeLog | 17 +++++++ > gdb/dummy-frame.c | 16 +++++++ > gdb/dummy-frame.h | 6 +++ > gdb/frame-unwind.c | 31 +++++++++++++ > gdb/frame-unwind.h | 12 +++++ > gdb/frame.c | 132 ++++++++++++++++++++++++----------------------------- > gdb/gdbarch.c | 41 ++++------------- > gdb/gdbarch.h | 6 --- > gdb/gdbarch.sh | 8 ++-- > 9 files changed, 154 insertions(+), 115 deletions(-) > > diff --git a/gdb/dummy-frame.c b/gdb/dummy-frame.c > index c6f874a3b19..6bb128233d7 100644 > --- a/gdb/dummy-frame.c > +++ b/gdb/dummy-frame.c > @@ -385,6 +385,22 @@ const struct frame_unwind dummy_frame_unwind = > dummy_frame_sniffer, > }; > > +/* Default implementation of gdbarch_dummy_id. Generate frame_id for > + THIS_FRAME assuming that it is a dummy frame. A dummy frame is created > + before an inferior call, the frame_id returned here must match the base > + address returned by gdbarch_push_dummy_call and the frame's pc must > + match the dummy frames breakpoint address. */ Use /* See dummy-frame.h. */ I think that additional info you have would be good in gdbarch.sh, to document gdbarch_dummy_id. > + > +struct frame_id > +default_dummy_id (struct gdbarch *gdbarch, struct frame_info *this_frame) > +{ > + CORE_ADDR sp, pc; > + > + sp = get_frame_sp (this_frame); > + pc = get_frame_pc (this_frame); > + return frame_id_build (sp, pc); > +} > + > static void > fprint_dummy_frames (struct ui_file *file) > { > diff --git a/gdb/dummy-frame.h b/gdb/dummy-frame.h > index 407f398404e..9eaec3492bf 100644 > --- a/gdb/dummy-frame.h > +++ b/gdb/dummy-frame.h > @@ -73,4 +73,10 @@ extern void register_dummy_frame_dtor (frame_id dummy_id, > extern int find_dummy_frame_dtor (dummy_frame_dtor_ftype *dtor, > void *dtor_data); > > +/* Default implementation of gdbarch_dummy_id. Generate a dummy frame_id > + for THIS_FRAME assuming that the frame is a dummy frame. */ > + > +extern struct frame_id default_dummy_id (struct gdbarch *gdbarch, > + struct frame_info *this_frame); > + > #endif /* !defined (DUMMY_FRAME_H) */ > diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c > index e6e63539ad7..df2e5ddc6bd 100644 > --- a/gdb/frame-unwind.c > +++ b/gdb/frame-unwind.c > @@ -193,6 +193,37 @@ default_frame_unwind_stop_reason (struct frame_info *this_frame, > return UNWIND_NO_REASON; > } > > +/* Default unwind of the PC. */ Use /* See frame-unwind.h. */ > + > +CORE_ADDR > +default_unwind_pc (struct gdbarch *gdbarch, struct frame_info *next_frame) > +{ > + struct type *type; > + int pc_regnum; > + CORE_ADDR addr; > + struct value *value; > + > + pc_regnum = gdbarch_pc_regnum (gdbarch); > + value = frame_unwind_register_value (next_frame, pc_regnum); > + type = builtin_type (gdbarch)->builtin_func_ptr; > + addr = extract_typed_address (value_contents_all (value), type); > + addr = gdbarch_addr_bits_remove (gdbarch, addr); It's up to you, but I'd declare the variables when assigning them (same for all the new functions in this patch). > + > + release_value (value); I'm not too familiar with the lifetime of struct value objects, but why is this needed? > + return addr; > +} > + > +/* Default unwind of the SP. */ Use /* See frame-unwind.h. */ Thanks, Simon