[PATCH] x86: Avoid abort on invalid broadcast

[H.J. Lu]

On Fri, Aug 20, 2021 at 3:59 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 19.08.2021 18:22, H.J. Lu wrote:
> > On Thu, Aug 19, 2021 at 9:03 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 19.08.2021 17:35, H.J. Lu wrote:
> >>> On Thu, Aug 19, 2021 at 8:27 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> 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.
> >>>
> >>> The input bytes aren't valid x86 opcodes.   But abort isn't nice.
> >>> If people run into the this, ".byte" should give a clue for invalid
> >>> opcodes.
> >>
> >> So a comment along the lines of
> >>
> >> # "vfpclasspsx $0x90, (%eax){1to8}, %k0" but with EVEX.W set
> >>
> >> is not helpful, you would think?
> >>
> >
> > No.   We need a different byte sequence for vfpclasspsx since the
> > first 2 bytes, 0x62, 0xc3, are invalid.
>
> But that's part of the problem: There are way too many things that
> are all invalid at the same time for this encoding. Hence it is not
> a concise test of the one issue you actually want to test. I've
> passed the sequence (with the one missing opcode byte added as 0xcc)
> to my disassembler, and I get
>
> EVEX.128.0F3A.W1:66 R18{k5}, R14, [r8-6F996F9A]{1toN}, CC
>
> So the issues are (at least)
> - the missing immediate byte
> - the destination being outside the %k0-%k7 range
> - the embedded prefix not representing the 0x66 legacy one
> - EVEX.VVVV not being 0b1111
> - EVEX.W set
>
> Any change to the logic detecting any of the other invalid aspects
> will therefore risk to invalidate the testcase.

Correct.  The comment is mainly for binutils developers.

# Invalid 16-bit broadcast with EVEX.W == 1.

is good enough.


-- 
H.J.