| Message ID | 07638ee5ff984d21a51b468a841f9dba@BN1AFFO11FD045.protection.gbl |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 6951 invoked by alias); 7 Oct 2014 10:16:50 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 6942 invoked by uid 89); 7 Oct 2014 10:16:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.2 required=5.0 tests=AWL, BAYES_40, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS, URIBL_RHS_DOB autolearn=no version=3.3.2 X-HELO: na01-bn1-obe.outbound.protection.outlook.com Received: from mail-bn1on0054.outbound.protection.outlook.com (HELO na01-bn1-obe.outbound.protection.outlook.com) (157.56.110.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 07 Oct 2014 10:16:48 +0000 Received: from BN1AFFO11FD045.protection.gbl (10.58.52.32) by BN1AFFO11HUB048.protection.gbl (10.58.52.107) with Microsoft SMTP Server (TLS) id 15.0.1039.16; Tue, 7 Oct 2014 10:16:44 +0000 Received: from xsj-pvapsmtpgw01 (149.199.60.83) by BN1AFFO11FD045.mail.protection.outlook.com (10.58.53.60) with Microsoft SMTP Server (TLS) id 15.0.1039.16 via Frontend Transport; Tue, 7 Oct 2014 10:16:44 +0000 Received: from unknown-38-66.xilinx.com ([149.199.38.66] helo=xsj-smtp1) by xsj-pvapsmtpgw01 with esmtp (Exim 4.63) (envelope-from <ajit.kumar.agarwal@xilinx.com>) id 1XbRnI-00008M-9H; Tue, 07 Oct 2014 03:15:00 -0700 From: Ajit Kumar Agarwal <ajit.kumar.agarwal@xilinx.com> To: Pedro Alves <palves@redhat.com>, "gdb-patches@sourceware.org" <gdb-patches@sourceware.org> CC: Vinod Kathail <vinodk@xilinx.com>, Vidhumouli Hunsigida <vidhum@xilinx.com>, Nagaraju Mekala <nmekala@xilinx.com> Subject: [Patch, microblaze]: Added cleanup data for invalid target description Date: Tue, 7 Oct 2014 10:16:40 +0000 Content-Type: multipart/mixed; boundary="_002_37378DC5BCD0EE48BA4B082E0B55DFAA40BE3DF7XAPPVEXMBX01xln_" MIME-Version: 1.0 X-RCIS-Action: ALLOW Message-ID: <07638ee5ff984d21a51b468a841f9dba@BN1AFFO11FD045.protection.gbl> X-EOPAttributedMessage: 0 X-Forefront-Antispam-Report: CIP:149.199.60.83; CTRY:US; IPV:NLI; EFV:NLI; SFV:NSPM; SFS:(10009020)(6009001)(438002)(189002)(199003)(377424004)(92566001)(2656002)(85852003)(86362001)(87936001)(99396003)(71186001)(84326002)(76482002)(229853001)(104016003)(512954002)(107046002)(46102003)(108616004)(85306004)(33646002)(80022003)(99936001)(50986999)(95666004)(31966008)(77096002)(106466001)(20776003)(568964001)(74316001)(19580405001)(120916001)(19580395003)(53416004)(6806004)(44976005)(4396001)(54356999)(21056001)(107986001)(24736002)(23106004); DIR:OUT; SFP:1101; SCL:1; SRVR:BN1AFFO11HUB048; H:xsj-pvapsmtpgw01; FPR:; MLV:sfv; PTR:unknown-60-83.xilinx.com; MX:1; A:1; LANG:en; X-Microsoft-Antispam: UriScan:; X-Microsoft-Antispam: BCL:0;PCL:0;RULEID:;SRVR:BN1AFFO11HUB048; X-Forefront-PRVS: 035748864E Received-SPF: Pass (protection.outlook.com: domain of xilinx.com designates 149.199.60.83 as permitted sender) receiver=protection.outlook.com; client-ip=149.199.60.83; helo=xsj-pvapsmtpgw01; Authentication-Results: spf=pass (sender IP is 149.199.60.83) smtp.mailfrom=ajit.kumar.agarwal@xilinx.com; X-OriginatorOrg: xilinx.com |
Commit Message
Ajit Kumar Agarwal
Oct. 7, 2014, 10:16 a.m. UTC
Please find the following patch based on Pedro Suggestion.
[Patch, microblaze]: Added cleanup data for invalid target description.
Cleanup the tdesc data if the target description check is invalid.
2014-10-07 Ajit Agarwal <ajitkum@xilinx.com>
* microblaze-tdep.c (microblaze_gdbarch_init): Use of
tdesc_data_cleanup.
Signed-off-by:Ajit Agarwal ajitkum@xilinx.com
---
gdb/microblaze-tdep.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
--
1.7.1
Thanks & Regards
Ajit
Comments
On 10/07/2014 11:16 AM, Ajit Kumar Agarwal wrote: > > From 00f2692d10e0254366471095516d657693aeff42 Mon Sep 17 00:00:00 2001 > From: Ajit Kumar Agarwal <ajitkum@xhdspdgnu.(none)> > Date: Tue, 7 Oct 2014 15:06:08 +0530 > Subject: [PATCH] [Patch, microblaze]: Added cleanup data for invalid target description. s/Added/Add/. But even better would be saying what this actually intends to do, which is "reject". Note the [PATCH] tag usually end ups stripped when the commit is imported into git, but the redundant [Patch, ...] seems like something you added manually, and is unnecessary. > > Cleanup the tdesc data if the target description check is invalid. > > 2014-10-07 Ajit Agarwal <ajitkum@xilinx.com> > > * microblaze-tdep.c (microblaze_gdbarch_init): Use of > tdesc_data_cleanup. So, I'd write: ~~~ [PATCH] Microblaze: Reject invalid target descriptions We currently validate the target description, but then forget to reject it if found invalid. gdb/ 2014-10-07 Ajit Agarwal <ajitkum@xilinx.com> * microblaze-tdep.c (microblaze_gdbarch_init): If the description isn't valid, release the tdesc arch data and return NULL. ~~~ But, you didn't state how you tested this, which should be part of the commit log too. Did you make sure incorrect descriptions are rejected and GDB warns about them? Did you make sure valid descriptions do end up correctly used? Or does this uncover other bugs? Thanks, Pedro Alves
-----Original Message----- From: Pedro Alves [mailto:palves@redhat.com] Sent: Tuesday, October 07, 2014 9:37 PM To: Ajit Kumar Agarwal; gdb-patches@sourceware.org Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala Subject: Re: [Patch, microblaze]: Added cleanup data for invalid target description On 10/07/2014 11:16 AM, Ajit Kumar Agarwal wrote: > > From 00f2692d10e0254366471095516d657693aeff42 Mon Sep 17 00:00:00 2001 > From: Ajit Kumar Agarwal <ajitkum@xhdspdgnu.(none)> > Date: Tue, 7 Oct 2014 15:06:08 +0530 > Subject: [PATCH] [Patch, microblaze]: Added cleanup data for invalid target description. >>s/Added/Add/. But even better would be saying what this actually intends to do, which is "reject". Note the [PATCH] tag usually end ups stripped when >>the commit is imported into git, but the redundant [Patch, ...] seems like something you added manually, and is unnecessary. Thanks for the suggestion. > > Cleanup the tdesc data if the target description check is invalid. > > 2014-10-07 Ajit Agarwal <ajitkum@xilinx.com> > > * microblaze-tdep.c (microblaze_gdbarch_init): Use of > tdesc_data_cleanup. So, I'd write: ~~~ >>[PATCH] Microblaze: Reject invalid target descriptions >>We currently validate the target description, but then forget to reject it if found invalid. >>gdb/ >>2014-10-07 Ajit Agarwal <ajitkum@xilinx.com> >> * microblaze-tdep.c (microblaze_gdbarch_init): If the description >> isn't valid, release the tdesc arch data and return NULL. ~~~ I will make this Change. Thanks for suggestion. >>But, you didn't state how you tested this, which should be part of the commit log too. I have tested the Microblaze Design with and without stack -protect registers. The gdb command "info registers" displayed the register correctly. If stack protect designs is not selected only core registers are displayed. When the stack-protect register is selected in the design, the core registers along with stack-protect registers are displayed. >>Did you make sure incorrect descriptions are rejected and GDB warns about them? We don't have the invalid Microblaze design with which I can test gdb warning and cleaning up the tdesc data. >>Did you make sure valid descriptions do end up correctly used? Yes. >>Or does this uncover other bugs? It doesn't cover other bugs but I would like to state the following "The gdbserver patch which I have sent to FSF would be using expedite "rpc" and the patch that got committed in FSF for stack-protect registers has the expedite set as "pc". For the gdbserver patch it should be "rpc". I will send a separate patch for this by changing the Makefiles as you have suggested." Thanks & Regards Ajit Thanks, Pedro Alves
On 10/07/2014 05:36 PM, Ajit Kumar Agarwal wrote: > > > -----Original Message----- > From: Pedro Alves [mailto:palves@redhat.com] > Sent: Tuesday, October 07, 2014 9:37 PM > To: Ajit Kumar Agarwal; gdb-patches@sourceware.org > Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala > Subject: Re: [Patch, microblaze]: Added cleanup data for invalid target description > > On 10/07/2014 11:16 AM, Ajit Kumar Agarwal wrote: >> >> From 00f2692d10e0254366471095516d657693aeff42 Mon Sep 17 00:00:00 2001 >> From: Ajit Kumar Agarwal <ajitkum@xhdspdgnu.(none)> >> Date: Tue, 7 Oct 2014 15:06:08 +0530 >> Subject: [PATCH] [Patch, microblaze]: Added cleanup data for invalid target description. > >>> s/Added/Add/. But even better would be saying what this actually intends to do, which is "reject". Note the [PATCH] tag usually end ups stripped when >>the commit is imported into git, but the redundant [Patch, ...] seems like something you added manually, and is unnecessary. > > Thanks for the suggestion. >> >> Cleanup the tdesc data if the target description check is invalid. >> >> 2014-10-07 Ajit Agarwal <ajitkum@xilinx.com> >> >> * microblaze-tdep.c (microblaze_gdbarch_init): Use of >> tdesc_data_cleanup. > > So, I'd write: > > ~~~ >>> [PATCH] Microblaze: Reject invalid target descriptions > >>> We currently validate the target description, but then forget to reject it if found invalid. > >>> gdb/ >>> 2014-10-07 Ajit Agarwal <ajitkum@xilinx.com> > >>> * microblaze-tdep.c (microblaze_gdbarch_init): If the description >>> isn't valid, release the tdesc arch data and return NULL. > ~~~ > > I will make this Change. Thanks for suggestion. > >>> But, you didn't state how you tested this, which should be part of the commit log too. > > I have tested the Microblaze Design with and without stack -protect registers. The gdb command "info registers" displayed the register correctly. If stack protect designs is not selected only core registers are displayed. When the stack-protect register is selected in the design, the core registers along with stack-protect registers are displayed. Please remember to always state this in the commit/submit log. I've now added an explicit bullet item for that to the contribution checklist. > >>> Did you make sure incorrect descriptions are rejected and GDB warns about them? > > We don't have the invalid Microblaze design with which I can test gdb warning and cleaning up the tdesc data. Sure you do. Just hack your target or gdbserver to send a bogus description. Thanks, Pedro Alves
diff --git a/gdb/microblaze-tdep.c b/gdb/microblaze-tdep.c index 6a9f11f..f257b96 100644 --- a/gdb/microblaze-tdep.c +++ b/gdb/microblaze-tdep.c @@ -722,7 +722,13 @@ microblaze_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) MICROBLAZE_SHR_REGNUM, "rshr"); } - } + + if (!valid_p) + { + tdesc_data_cleanup (tdesc_data); + return NULL; + } + } /* Allocate space for the new architecture. */ tdep = XNEW (struct gdbarch_tdep);