This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [Patch, microblaze]: Added cleanup data for invalid target description
- From: Pedro Alves <palves at redhat dot com>
- To: Ajit Kumar Agarwal <ajit dot kumar dot agarwal at xilinx dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Cc: Vinod Kathail <vinodk at xilinx dot com>, Vidhumouli Hunsigida <vidhum at xilinx dot com>, Nagaraju Mekala <nmekala at xilinx dot com>
- Date: Tue, 07 Oct 2014 18:09:08 +0100
- Subject: Re: [Patch, microblaze]: Added cleanup data for invalid target description
- Authentication-results: sourceware.org; auth=none
- References: <07638ee5ff984d21a51b468a841f9dba at BN1AFFO11FD045 dot protection dot gbl> <54340FBE dot 2090508 at redhat dot com> <6216c950de8d45809bad2b931446f6d7 at BN1AFFO11FD006 dot protection dot gbl>
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