[PATCH] Microblaze: Reject invalid target descriptions.

[Ajit Kumar Agarwal <ajit dot kumar dot agarwal at xilinx dot com>]

Hello Pedro:

Please find the updated patch with the feedback incorporated.

[Pedro]: Sure you do.  Just hack your target or gdbserver to send a bogus description.

Tested .

From f69e29dbda0669ee0f5b263c0aab094db6960693 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] Microblaze: Reject invalid target descriptions.

We currently validate the target description, but then forget to reject
it if found invalid.

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.

gdb/
2014-10-08  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.

Signed-off-by:Ajit Agarwal ajitkum@xilinx.com
---
 gdb/microblaze-tdep.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

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);
--
1.7.1

Thanks & Regards
Ajit
-----Original Message-----
From: Pedro Alves [mailto:palves@redhat.com] 
Sent: Tuesday, October 07, 2014 10:39 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 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

Attachment: 0001-Microblaze-Reject-invalid-target-descriptions.patch
Description: 0001-Microblaze-Reject-invalid-target-descriptions.patch