This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/3] x86: drop VEXI4_Fixup()


On Tue, Nov 14, 2017 at 5:35 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 14.11.17 at 13:27, <hjl.tools@gmail.com> wrote:
>> On Mon, Nov 13, 2017 at 11:31 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>> The low four bits of an immediate being set when the high bits specify a
>>> fourth register operand is not a problem: CPUs ignore these bits rather
>>> than raising #UD. Take care of incrementing codep in OP_EX_VexW()
>>> instead.
>>>
>>> opcodes/
>>> 2017-11-14  Jan Beulich  <jbeulich@suse.com>
>>>
>>>         * i386-dis.c (VEXI4_Fixup, VexI4): Delete.
>>>         (prefix_table, xop_table, vex_len_table): Remove VexI4 uses.
>>>         (OP_EX_VexW): Move setting of vex_w_done. Update codep on 2nd
>>>         pass.
>>>         (OP_REG_VexI4): Drop low 4 bits check.
>>>
>>> --- 2017-11-10.orig/opcodes/i386-dis.c  2017-11-10 08:33:59.614463724 +0100
>>> +++ 2017-11-10/opcodes/i386-dis.c       2017-11-13 17:44:05.279694616 +0100
>>> @@ -95,7 +95,6 @@ static void OP_XMM_VexW (int, int);
>>>  static void OP_Rounding (int, int);
>>>  static void OP_REG_VexI4 (int, int);
>>>  static void PCLMUL_Fixup (int, int);
>>> -static void VEXI4_Fixup (int, int);
>>>  static void VZERO_Fixup (int, int);
>>>  static void VCMP_Fixup (int, int);
>>>  static void VPCMP_Fixup (int, int);
>>> @@ -422,7 +421,6 @@ fetch_data (struct disassemble_info *inf
>>>  #define Vex128 { OP_VEX, vex128_mode }
>>>  #define Vex256 { OP_VEX, vex256_mode }
>>>  #define VexGdq { OP_VEX, dq_mode }
>>> -#define VexI4 { VEXI4_Fixup, 0}
>>>  #define EXdVex { OP_EX_Vex, d_mode }
>>>  #define EXdVexS { OP_EX_Vex, d_swap_mode }
>>>  #define EXdVexScalarS { OP_EX_Vex, d_scalar_swap_mode }
>>> @@ -6746,28 +6744,28 @@ static const struct dis386 prefix_table[
>>>    {
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vfmaddsubps", { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vfmaddsubps", { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>    },
>>>
>>>    /* PREFIX_VEX_0F3A5D */
>>>    {
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vfmaddsubpd", { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vfmaddsubpd", { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>    },
>>>
>>>    /* PREFIX_VEX_0F3A5E */
>>>    {
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vfmsubaddps", { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vfmsubaddps", { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>    },
>>>
>>>    /* PREFIX_VEX_0F3A5F */
>>>    {
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vfmsubaddpd", { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vfmsubaddpd", { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>    },
>>>
>>>    /* PREFIX_VEX_0F3A60 */
>>> @@ -6803,14 +6801,14 @@ static const struct dis386 prefix_table[
>>>    {
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vfmaddps", { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vfmaddps", { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>    },
>>>
>>>    /* PREFIX_VEX_0F3A69 */
>>>    {
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vfmaddpd", { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vfmaddpd", { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>    },
>>>
>>>    /* PREFIX_VEX_0F3A6A */
>>> @@ -6831,14 +6829,14 @@ static const struct dis386 prefix_table[
>>>    {
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vfmsubps", { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vfmsubps", { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>    },
>>>
>>>    /* PREFIX_VEX_0F3A6D */
>>>    {
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vfmsubpd", { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vfmsubpd", { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>    },
>>>
>>>    /* PREFIX_VEX_0F3A6E */
>>> @@ -6859,14 +6857,14 @@ static const struct dis386 prefix_table[
>>>    {
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vfnmaddps", { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vfnmaddps", { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>    },
>>>
>>>    /* PREFIX_VEX_0F3A79 */
>>>    {
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vfnmaddpd", { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vfnmaddpd", { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>    },
>>>
>>>    /* PREFIX_VEX_0F3A7A */
>>> @@ -6887,7 +6885,7 @@ static const struct dis386 prefix_table[
>>>    {
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vfnmsubps", { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vfnmsubps", { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>      { Bad_Opcode },
>>>    },
>>>
>>> @@ -6895,7 +6893,7 @@ static const struct dis386 prefix_table[
>>>    {
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vfnmsubpd", { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vfnmsubpd", { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>    },
>>>
>>>    /* PREFIX_VEX_0F3A7E */
>>> @@ -7855,9 +7853,9 @@ static const struct dis386 xop_table[][2
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vpmacssww",     { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> -    { "vpmacsswd",     { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> -    { "vpmacssdql",    { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vpmacssww",     { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>> +    { "vpmacsswd",     { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>> +    { "vpmacssdql",    { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>      /* 88 */
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> @@ -7865,17 +7863,17 @@ static const struct dis386 xop_table[][2
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vpmacssdd",     { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> -    { "vpmacssdqh",    { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vpmacssdd",     { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>> +    { "vpmacssdqh",    { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>      /* 90 */
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vpmacsww",      { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> -    { "vpmacswd",      { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> -    { "vpmacsdql",     { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vpmacsww",      { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>> +    { "vpmacswd",      { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>> +    { "vpmacsdql",     { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>      /* 98 */
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> @@ -7883,16 +7881,16 @@ static const struct dis386 xop_table[][2
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vpmacsdd",      { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> -    { "vpmacsdqh",     { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vpmacsdd",      { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>> +    { "vpmacsdqh",     { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>      /* a0 */
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vpcmov",        { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> -    { "vpperm",        { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vpcmov",        { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>> +    { "vpperm",        { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vpmadcsswd",    { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vpmadcsswd",    { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>      { Bad_Opcode },
>>>      /* a8 */
>>>      { Bad_Opcode },
>>> @@ -7910,7 +7908,7 @@ static const struct dis386 xop_table[][2
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>>      { Bad_Opcode },
>>> -    { "vpmadcswd",     { XMVexW, Vex, EXVexW, EXVexW, VexI4 }, 0 },
>>> +    { "vpmadcswd",     { XMVexW, Vex, EXVexW, EXVexW }, 0 },
>>>      { Bad_Opcode },
>>>      /* b8 */
>>>      { Bad_Opcode },
>>> @@ -10142,42 +10140,42 @@ static const struct dis386 vex_len_table
>>>
>>>    /* VEX_LEN_0F3A6A_P_2 */
>>>    {
>>> -    { "vfmaddss",      { XMVexW, Vex128, EXdVexW, EXdVexW, VexI4 }, 0 },
>>> +    { "vfmaddss",      { XMVexW, Vex128, EXdVexW, EXdVexW }, 0 },
>>>    },
>>>
>>>    /* VEX_LEN_0F3A6B_P_2 */
>>>    {
>>> -    { "vfmaddsd",      { XMVexW, Vex128, EXqVexW, EXqVexW, VexI4 }, 0 },
>>> +    { "vfmaddsd",      { XMVexW, Vex128, EXqVexW, EXqVexW }, 0 },
>>>    },
>>>
>>>    /* VEX_LEN_0F3A6E_P_2 */
>>>    {
>>> -    { "vfmsubss",      { XMVexW, Vex128, EXdVexW, EXdVexW, VexI4 }, 0 },
>>> +    { "vfmsubss",      { XMVexW, Vex128, EXdVexW, EXdVexW }, 0 },
>>>    },
>>>
>>>    /* VEX_LEN_0F3A6F_P_2 */
>>>    {
>>> -    { "vfmsubsd",      { XMVexW, Vex128, EXqVexW, EXqVexW, VexI4 }, 0 },
>>> +    { "vfmsubsd",      { XMVexW, Vex128, EXqVexW, EXqVexW }, 0 },
>>>    },
>>>
>>>    /* VEX_LEN_0F3A7A_P_2 */
>>>    {
>>> -    { "vfnmaddss",     { XMVexW, Vex128, EXdVexW, EXdVexW, VexI4 }, 0 },
>>> +    { "vfnmaddss",     { XMVexW, Vex128, EXdVexW, EXdVexW }, 0 },
>>>    },
>>>
>>>    /* VEX_LEN_0F3A7B_P_2 */
>>>    {
>>> -    { "vfnmaddsd",     { XMVexW, Vex128, EXqVexW, EXqVexW, VexI4 }, 0 },
>>> +    { "vfnmaddsd",     { XMVexW, Vex128, EXqVexW, EXqVexW }, 0 },
>>>    },
>>>
>>>    /* VEX_LEN_0F3A7E_P_2 */
>>>    {
>>> -    { "vfnmsubss",     { XMVexW, Vex128, EXdVexW, EXdVexW, VexI4 }, 0 },
>>> +    { "vfnmsubss",     { XMVexW, Vex128, EXdVexW, EXdVexW }, 0 },
>>>    },
>>>
>>>    /* VEX_LEN_0F3A7F_P_2 */
>>>    {
>>> -    { "vfnmsubsd",     { XMVexW, Vex128, EXqVexW, EXqVexW, VexI4 }, 0 },
>>> +    { "vfnmsubsd",     { XMVexW, Vex128, EXqVexW, EXqVexW }, 0 },
>>>    },
>>>
>>>    /* VEX_LEN_0F3ADF_P_2 */
>>> @@ -17401,8 +17399,6 @@ OP_EX_VexW (int bytemode, int sizeflag)
>>>
>>>    if (!vex_w_done)
>>>      {
>>> -      vex_w_done = 1;
>>> -
>>>        /* Skip mod/rm byte.  */
>>>        MODRM_CHECK;
>>>        codep++;
>>> @@ -17417,16 +17413,10 @@ OP_EX_VexW (int bytemode, int sizeflag)
>>>      }
>>>
>>>    OP_EX_VexReg (bytemode, sizeflag, reg);
>>> -}
>>>
>>> -static void
>>> -VEXI4_Fixup (int bytemode ATTRIBUTE_UNUSED,
>>> -            int sizeflag ATTRIBUTE_UNUSED)
>>> -{
>>> -  /* Skip the immediate byte and check for invalid bits.  */
>>> -  FETCH_DATA (the_info, codep + 1);
>>> -  if (*codep++ & 0xf)
>>> -    BadOp ();
>>> +  if (vex_w_done)
>>> +    codep++;
>>> +  vex_w_done = 1;
>>>  }
>>>
>>>  static void
>>> @@ -17441,9 +17431,6 @@ OP_REG_VexI4 (int bytemode, int sizeflag
>>>    if (bytemode != x_mode)
>>>      abort ();
>>>
>>> -  if (reg & 0xf)
>>> -      BadOp ();
>>> -
>>>    reg >>= 4;
>>>    if (reg > 7 && address_mode != mode_64bit)
>>>      BadOp ();
>>>
>>>
>>
>> Please use .byte directive to add a testcase to verify that disassembler
>> properly ignores these bits.   OK with that change.
>
> The test for this is part of patch 3. Is that not sufficient?
>

Please break down these tests and include the relevant tests in this patch.

Thanks.

-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]