[PATCHv2] opcodes/x86: fix minor missed styling case

[H.J. Lu]

On Thu, Jul 25, 2024, 10:12 AM Jiang, Haochen <haochen.jiang@intel.com>
wrote:

> > -----Original Message-----
> > From: Cui, Lili <lili.cui@intel.com>
> > Sent: Thursday, July 25, 2024 9:53 AM
> > To: Andrew Burgess <aburgess@redhat.com>; Beulich, Jan <
> JBeulich@suse.com>
> > Cc: H.J. Lu <hjl.tools@gmail.com>; Jiang, Haochen <
> haochen.jiang@intel.com>;
> > binutils@sourceware.org
> > Subject: RE: [PATCHv2] opcodes/x86: fix minor missed styling case
> >
> >
> > > > On 24.07.2024 04:31, Cui, Lili wrote:
> > > >>>> I noticed that the x86 instruction:
> > > >>>>
> > > >>>>   sar    $0x1,%rsi
> > > >>>>
> > > >>>> would fail to style the '$0x1' as an immediate.  This commit
> > > >>>> fixes that
> > > case.
> > > >>>>
> > > >>
> > > >> I'm afraid it is not a bug, it is to distinguish between the two
> > > >> formats
> > > below.
> > > >>
> > > >> sar    r/m8, 1
> > > >> sar    r/m8, imm8
> > > >
> > > > It is a bug, but it also is a bug to change 1 to 0x1, as that way
> > > > said distinction goes away. (I also don't immediately see how the
> > > > code change alone would pass the testsuite; I'm pretty sure we have
> > > > expectations which would have required adjustment, which would have
> > > > made more obvious that the change wants doing differently.)
> > >
> > > You are correct, I got sloppy, and I apologise.
> > >
> > > Thanks to everyone who pointed out the mistake.
> > >
> > > Here's an update, _fully_ tested patch.
> > >
> > > Thanks,
> > > Andrew
> > >
> > > ---
> > >
> > > commit ffe8ab67ab81ae24105433145ca6ee40c3fb92e2
> > > Author: Andrew Burgess <aburgess@redhat.com>
> > > Date:   Tue Jul 23 17:10:22 2024 +0100
> > >
> > >     opcodes/x86: fix minor missed styling case
> > >
> > >     I noticed that the x86 instruction:
> > >
> > >       sar    $1,%rsi
> > >
> > >     would fail to style the '$0x1' as an immediate.  This commit fixes
> > >     that case.
> > >
> > > diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c index
> > > bc141f31770..59ec771369a 100644
> > > --- a/opcodes/i386-dis.c
> > > +++ b/opcodes/i386-dis.c
> > > @@ -12415,9 +12415,9 @@ OP_I (instr_info *ins, int bytemode, int
> sizeflag)
> > >        break;
> > >      case const_1_mode:
> > >        if (ins->intel_syntax)
> > > -   oappend (ins, "1");
> > > +   oappend_with_style (ins, "1", dis_style_immediate);
> > >        else
> > > -   oappend (ins, "$1");
> > > +   oappend_with_style (ins, "$1", dis_style_immediate);
> > >        return true;
> > >      default:
> > >        oappend (ins, INTERNAL_DISASSEMBLER_ERROR);
> >
> > I am a little confused, how does this patch fix this case in the
> comment, the
> > output is not changed before and after the patch.
>
> Confused +1. It seems Changelog and the current output doesn't match or I
> misunderstood something.
>

I think you will see the different output only when style is enabled.


> Thx,
> Haochen
>
> >
> > Thanks,
> > Lili.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20240725/d5af6d03/attachment.htm>