[PATCH 1/8] Support APX GPR32 with rex2 prefix

[Cui, Lili]

> >>>>>>>   /* REX2.M in rex2 prefix represents map0 or map1.  */
> >>>>>>>   if (*ins.codep == 0x0f || (ins.rex2 & REX2_M))
> >>>>>>>     {
> >>>>>>>       unsigned char threebyte;
> >>>>>>>
> >>>>>>>       if (!ins.rex2)
> >>>>>>>         {
> >>>>>>>           ins.codep++;
> >>>>>>>           if (!fetch_code (info, ins.codep + 1))
> >>>>>>>             goto fetch_error_out;                                                      ---> When
> >>>> there
> >>>>>> are no bytes after 0f, it will jump to fetch error, but no error
> >>>>>> will be
> >>>> reported.
> >>>>>>>         }
> >>>>>>>       threebyte = *ins.codep;
> >>>>>>>       dp = &dis386_twobyte[threebyte];
> >>>>>>>       ins.need_modrm = twobyte_has_modrm[threebyte];
> >>>>>>>       ins.codep++;
> >>>>>>>     }
> >>>>>>>
> >>>>>>> For d5 00 0f
> >>>>>>> Decode to:
> >>>>>>>    0:   d5                      rex2
> >>>>>>>    1:   00 0f                   add    %cl,(%rdi)
> >>>>>>
> >>>>>> But this would better have d5 00 0f all on the first line (it
> >>>>>> definitely needs to have d5 00 on the same line, as the bytes
> >>>>>> belong
> >>>> together), as opposed to ...
> >>>>>>
> >>>>>>> For 40 0f
> >>>>>>> Decode to:
> >>>>>>>    0:   40                      rex
> >>>>>>>    1:   0f                      .byte 0xf
> >>>>>>
> >>>>>> ... this where there truly is a known missing byte before we
> >>>>>> could proceed further. (It's still a little questionable to print
> >>>>>> REX separately in this case, but that's the way the binutils
> >>>>>> disassembler has always worked.)
> >>>>>>
> >>>>>> Yet to restate - to see what I mean, you'd need to populate at
> >>>>>> least one of the two 0f slots in the mentioned arrays. What I'm
> >>>>>> suspecting from the code as this patch version has it is that d5
> >>>>>> 00 0f would wrongly descend into dis386_twobyte[]. Yet you can
> >>>>>> tell that from it correctly using dis386[] only if the two 0f
> >>>>>> slots of these arrays are meaningfully different (or by actually
> >>>>>> looking at things in
> >> e.g.
> >>>>>> a debugger).
> >>>>>>
> >>>>>
> >>>>> I'm confused here, for d5 00 0f when it fetches the next byte
> >>>>> after 0f it will
> >>>> find there is no byte there and then go to fetch_error_out and then
> >>>> it will return from print_insn and I don't have a chance to do
> >>>> anything for it. It cannot reach dis386_twobyte[].
> >>>>
> >>>> But why would it even try to fetch the next byte? 0f already is the
> >>>> major opcode byte in this case. Fetching more can only mean either
> >>>> there's an entry in dis386[] specifying operands, or there's an
> >>>> attempt to index dis386_twobyte[]. Since dis386[] has Bad_Opcode at
> >>>> that slot, I conclude that what you say confirms my suspicion that
> >>>> dis386_twobyte[] is (attempted to
> >>>> be) used here.
> >>>>
> >>>
> >>> I don't know how to identify that 0f is the last byte of the binary,
> >>
> >> That's entirely irrelevant here. I gave the byte sequence d5 00 0f
> >> just as the minimal one required to make my point. My original
> >> concern equally applies to e.g. d5 00 0f 01 00, which may not use
> dis386_twobyte[0x01].
> >>
> > Aha, I  got you. Changed the code to
> >
> >    /* REX2.M in rex2 prefix represents map0 or map1.  */
> > -  if (*ins.codep == 0x0f || (ins.rex2 & REX2_M))
> > +  if ((*ins.codep == 0x0f && ins.last_rex2_prefix < 0) || (ins.rex2 &
> > + REX2_M))
> 
> Would you mind considering
> 
>   if (ins.last_rex2_prefix < 0 ? *ins.codep == 0x0f : (ins.rex2 & REX2_M))
> 
> as an alternative?
> 

Done, thanks!

Lili