[PATCH] x86: Avoid abort on invalid broadcast

[H.J. Lu]

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.

> >>> --- 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
>


-- 
H.J.