[PATCH] x86: Avoid abort on invalid broadcast

[Jan Beulich]

On 19.08.2021 16:45, H.J. Lu wrote:
> On Thu, Aug 19, 2021 at 7:18 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 19.08.2021 16:02, H.J. Lu via Binutils wrote:
>>> --- /dev/null
>>> +++ b/gas/testsuite/gas/i386/bad-bcast.s
>>> @@ -0,0 +1,2 @@
>>> +     .text
>>> +     .byte 0x62, 0xc3, 0x8c, 0x1d, 0x66, 0x90, 0x66, 0x90, 0x66, 0x90
>>
>> Would you mind adding a comment indicating what this resembles?
> 
> I will add some comments.

Hmm, I guess I wasn't explicit enough: While the comment you add
now states the EVEX.W aspect, what I was after is to save readers
from having to look up what opcode this is and hence why broadcast
is invalid here with EVEX.W in the first place. Due to the rubbish
the disassembler produces its output isn't any help here either.
So what I was looking to have is a commented instruction with
operands (which the assembler would reject), plus expressing of
the EVEX.W aspect in whatever way is suitable. And I actually
wonder whether the insn you've chosen isn't invalid even without
broadcast, but with EVEX.W set. In which case I wouldn't be
convinced of the usefulness of the test - it would become invalid
once EVEX.W gains a meaning for the major opcode in question,
would be removed at that point, and the logic in question wouldn't
be tested anymore at all.

>>> --- a/opcodes/i386-dis.c
>>> +++ b/opcodes/i386-dis.c
>>> @@ -11912,7 +11912,7 @@ OP_E_memory (int bytemode, int sizeflag)
>>>          {
>>>            if (vex.w)
>>>              {
>>> -              abort ();
>>> +           oappend ("{bad}");
>>
>> I can see that this is encoding dependent, so indeed shouldn't be
>> abort().
>>
>>> @@ -11928,7 +11928,7 @@ OP_E_memory (int bytemode, int sizeflag)
>>>                    oappend ("{1to32}");
>>>                    break;
>>>                  default:
>>> -                  abort ();
>>> +               oappend ("{bad}");
>>>                  }
>>>              }
>>>          }
>>> @@ -11948,7 +11948,7 @@ OP_E_memory (int bytemode, int sizeflag)
>>>             oappend ("{1to8}");
>>>             break;
>>>           default:
>>> -           abort ();
>>> +           oappend ("{bad}");
>>>           }
>>>       }
>>>        else if (bytemode == x_mode
>>> @@ -11966,7 +11966,7 @@ OP_E_memory (int bytemode, int sizeflag)
>>>             oappend ("{1to16}");
>>>             break;
>>>           default:
>>> -           abort ();
>>> +           oappend ("{bad}");
>>>           }
>>>       }
>>>        else
>>
>> All of these, otoh, assume that EVEX.L'L=3 was filtered out earlier,
>> so I think abort() is legitimate there.
> 
> I am putting 3 aborts.

Thanks.

Jan