This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] [ARC] Fix handling of cpu=... disassembler option value
- From: Pedro Alves <palves at redhat dot com>
- To: Anton Kolesov <Anton dot Kolesov at synopsys dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Cc: Francois Bedard <Francois dot Bedard at synopsys dot com>, Claudiu Zissulescu <Claudiu dot Zissulescu at synopsys dot com>, Cupertino Miranda <Cupertino dot Miranda at synopsys dot com>
- Date: Fri, 16 Jun 2017 13:10:38 +0100
- Subject: Re: [PATCH] [ARC] Fix handling of cpu=... disassembler option value
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 6A0CEC074554
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6A0CEC074554
- References: <20170615145946.4158-1-Anton.Kolesov@synopsys.com> <33d0cfea-776a-09a9-0b59-200fa33c99f5@redhat.com> <39A54937CC95F24AA2F794E2D2B66B135875A427@DE02WEMBXB.internal.synopsys.com>
On 06/16/2017 12:44 PM, Anton Kolesov wrote:
>> Couldn't this use disassembler_options_cmp /
>> FOR_EACH_DISASSEMBLER_OPTION?
>> See e.g. ppc-dis.c:ppc_parse_cpu for example.
>
> FOR_EACH_DISASSEMBLER_OPTION advances pointer to the beginning of next option,
> but it doesn't create a new string with \0 at the end. So, for example, this
> code:
>
> const char* option;
> FOR_EACH_DISASSEMBLER_OPTION (option, "fpuda,fpud,fpus")
> {
> printf ("option: %s\n", option);
> }
>
> will print:
>
> option: fpuda,fpud,fpus
> option: fpud,fpus
> option: fpus
>
> As a result this macro can be used to improve existing code in arc-dis.c,
> however it doesn't address the issue that this particular patch is trying to
> fix.
Hmm, the description you originally send only talked about parsing the
option value incorrectly. If you change arc-dis.c:parse_cpu_option
to use disassembler_options_cmp for comparing options, then I think
it should fix the original issue you described? Guess the issue then would
be that disassembler_options_cmp is case sensitive. I wonder whether
architectures really want to be different here..
Oh well.
IMO, it's arguable whether to consider printing the whole string starting at
what failed to be parsed as a bug. I find it fine. ppc-dis.c:powerpc_init_dialect
also prints the whole string starting from the invalid option, AFAICS.
(And GDB does that in many other cases too.)
Actually I find it a bug that this code even prints to stderr directly
in the first place. That'll be the wrong thing to do when this code
is being called by GDB -- a gdb Python script can disassemble with output
redirected to a string, for example.
Thanks,
Pedro Alves