Re: [PATCH v2 5/9] x86: improve handling of insns with ambiguous operand sizes

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Mon, Nov 11, 2019 at 11:23 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 11.11.2019 19:27,  H.J. Lu  wrote:
> > On Mon, Nov 11, 2019 at 1:29 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 08.11.2019 16:54,  H.J. Lu  wrote:
> >>> On Fri, Nov 8, 2019 at 12:09 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>> On 07.11.2019 18:47,  H.J. Lu  wrote:
> >>>>> I don't think DefaultSize matters for them either in AT&T syntax.
> >>>>> and I don't think we should add DefaultSize to more instructions.
> >>>>
> >>>> Then _again_ - what is your alternative suggestion?
> >>>
> >>> Don't add DefaultSize to more instructions.
> >>
> >> So I think I've recalled meanwhile: The issue is with us not wanting
> >> to issue diagnostics on e.g. LGDT despite it allowing multiple
> >> operand sizes. Instead, just like for PUSH/POP etc we want to silently
> >> default to the most appropriate operand size for the mode. Hence
> >> adding DefaultSize seems very applicable to me here.
> >>
> >
> > This is a problem only for Intel syntax and DefaultSize affects both
> > Intel syntax and AT&T syntax.
>
> Are you talking about before or after this patch? Recall that the
> changes here bring AT&T syntax handling more in line with Intel
> one, just that for AT&T talk is about absent suffixes while for
> Intel talk is about absent operand size modifiers.
>
> >  Since we can't have both DefaultSize
> > and IgnoreSize at the same time, can they be merged and leave it
> > for Intel syntax to deal with it?
>
> I don't think they can be merged. See the draft patch you've sent
> the other day actually splitting DefaultSize into two bits.
>

The current code has

  else if (i.tm.opcode_modifier.defaultsize
           && !i.suffix
           /* exclude fldenv/frstor/fsave/fstenv */
           && i.tm.opcode_modifier.no_ssuf)
    {
      if (stackop_size == LONG_MNEM_SUFFIX
          && i.tm.base_opcode == 0xcf)
        {
          /* stackop_size is set to LONG_MNEM_SUFFIX for the
             .code16gcc directive to support 16-bit mode with
             32-bit address.  For IRET without a suffix, generate
             16-bit IRET (opcode 0xcf) to return from an interrupt
             handler.  */
          i.suffix = WORD_MNEM_SUFFIX;
          as_warn (_("generating 16-bit `iret' for .code16gcc directive"));
        }
      else
        i.suffix = stackop_size;
    }
  else if (intel_syntax
           && !i.suffix
           && (i.tm.operand_types[0].bitfield.jumpabsolute
               || i.tm.opcode_modifier.jumpbyte
               || i.tm.opcode_modifier.jumpintersegment
               || (i.tm.base_opcode == 0x0f01 /* [ls][gi]dt */
                   && i.tm.extension_opcode <= 3)))
    {

What you did was to move Intel syntax specific check to generic.
We shouldn't do that.

-- 
H.J.