[PATCH v3 01/10] x86: improve handling of insns with ambiguous operand sizes
H.J. Lu
hjl.tools@gmail.com
Mon Jan 13 14:45:00 GMT 2020
On Mon, Jan 13, 2020 at 4:57 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 10.01.2020 21:45, H.J. Lu wrote:
> > On Fri, Jan 10, 2020 at 12:54 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 09.01.2020 23:35, H.J. Lu wrote:
> >>> On Fri, Dec 27, 2019 at 1:22 AM Jan Beulich <JBeulich@suse.com> wrote:
> >>>>
> >>>> Commit b76bc5d54e ("x86: don't default variable shift count insns to
> >>>> 8-bit operand size") pointed out a very bad case, but the underlying
> >>>> problem is, as mentioned on various occasions, much larger: Silently
> >>>> selecting a (nowhere documented afaict) certain default operand size
> >>>> when there's no "sizing" suffix and no suitable register operand(s) is
> >>>> simply dangerous (for the programmer to make mistakes).
> >>>>
> >>>> While in Intel syntax mode such mistakes already lead to an error (which
> >>>> is going to remain that way), AT&T syntax mode now gains warnings in
> >>>> such cases by default, which can be suppressed or promoted to an error
> >>>> if so desired by the programmer. Furthermore at least general purpose
> >>>> insns now consistently have a default applied (alongside the warning
> >>>> emission), rather than accepting some and refusing others.
> >>>>
> >>>> No warnings are (as before) to be generated for "DefaultSize" insns as
> >>>> well as ones acting on selector and other fixed-width values. For
> >>>> SYSRET, however, the DefaultSize needs to be dropped - it had been
> >>>> wrongly put there in the first place, as it's unrelated to .code16gcc
> >>>> (no stack accesses involved).
> >>>>
> >>>> As set forth as a prereq when I first mentioned this intended change a
> >>>> few years back, Linux as well as gcc have meanwhile been patched to
> >>>> avoid (emission of) ambiguous operands (and hence triggering of the new
> >>>> warning).
> >>>>
> >>>> Note that I think that in 64-bit mode IRET and far RET would better get
> >>>> a diagnostic too, as it's reasonably likely that a suffix-less instance
> >>>> really is meant to be a 64-bit one. But I guess I better make this a
> >>>> separate follow-on patch.
> >>>>
> >>>> Note further that floating point operations with integer operands are an
> >>>> exception for now: They continue to use short (16-bit) operands by
> >>>> default even in 32- and 64-bit modes.
> >>>>
> >>>> gas/
> >>>> 2020-01-XX Jan Beulich <jbeulich@suse.com>
> >>>>
> >>>> * config/tc-i386.c (process_suffix): Drop SYSRET special case
> >>>> and an intel_syntax check. Re-write lack-of-suffix processing
> >>>> logic.
> >>>> * doc/c-i386.texi: Document operand size defaults for suffix-
> >>>> less AT&T syntax insns.
> >>>> * testsuite/gas/i386/bundle.s, testsuite/gas/i386/lock-1.s,
> >>>> testsuite/gas/i386/opcode.s, testsuite/gas/i386/sse3.s,
> >>>> testsuite/gas/i386/x86-64-avx-scalar.s,
> >>>> testsuite/gas/i386/x86-64-avx.s,
> >>>> testsuite/gas/i386/x86-64-bundle.s,
> >>>> testsuite/gas/i386/x86-64-intel64.s,
> >>>> testsuite/gas/i386/x86-64-lock-1.s,
> >>>> testsuite/gas/i386/x86-64-opcode.s,
> >>>> testsuite/gas/i386/x86-64-sse2avx.s,
> >>>> testsuite/gas/i386/x86-64-sse3.s: Add missing suffixes.
> >>>> * testsuite/gas/i386/nops.s, testsuite/gas/i386/sse-noavx.s,
> >>>> testsuite/gas/i386/x86-64-nops.s,
> >>>> testsuite/gas/i386/x86-64-ptwrite.s,
> >>>> testsuite/gas/i386/x86-64-simd.s,
> >>>> testsuite/gas/i386/x86-64-sse-noavx.s,
> >>>> testsuite/gas/i386/x86-64-suffix.s: Drop bogus suffix-less
> >>>> insns.
> >>>> * testsuite/gas/i386/noreg16.s, testsuite/gas/i386/noreg32.s,
> >>>> testsuite/gas/i386/noreg64.s: Add further tests.
> >>>> * testsuite/gas/i386/ilp32/x86-64-nops.d,
> >>>> testsuite/gas/i386/nops.d, testsuite/gas/i386/noreg16.d,
> >>>> testsuite/gas/i386/noreg32.d, testsuite/gas/i386/noreg64.d,
> >>>> testsuite/gas/i386/sse-noavx.d,
> >>>> testsuite/gas/i386/x86-64-intel64.d,
> >>>> testsuite/gas/i386/x86-64-nops.d,
> >>>> testsuite/gas/i386/x86-64-opcode.d,
> >>>> testsuite/gas/i386/x86-64-ptwrite-intel.d,
> >>>> testsuite/gas/i386/x86-64-ptwrite.d,
> >>>> testsuite/gas/i386/x86-64-simd-intel.d,
> >>>> testsuite/gas/i386/x86-64-simd-suffix.d,
> >>>> testsuite/gas/i386/x86-64-simd.d,
> >>>> testsuite/gas/i386/x86-64-sse-noavx.d
> >>>> testsuite/gas/i386/x86-64-suffix.d,
> >>>> testsuite/gas/i386/x86-64-suffix-intel.d: Adjust expectations.
> >>>> * testsuite/gas/i386/noreg16.l, testsuite/gas/i386/noreg32.l,
> >>>> testsuite/gas/i386/noreg64.l: New.
> >>>> * testsuite/gas/i386/i386.exp: Run new tests.
> >>>>
> >>>> opcodes/
> >>>> 2020-01-XX Jan Beulich <jbeulich@suse.com>
> >>>>
> >>>> * i386-opc.tbl (sysret): Drop DefaultSize.
> >>>> * i386-tbl.h: Re-generate.
> >>>> ---
> >>>> v3: Don't add any new DefaultSize. Also diagnose SYSRET in 64-bit mode.
> >>>> Re-base.
> >>>> v2: Re-base over NOP addition to testcases modified here. Add IN/OUT
> >>>> tests. Add shift/rotate insn single operand tests. Extend
> >>>> "Instruction Naming" doc section to spell out defaults.
> >>>
> >>> Since we don't change encoding, we shouldn't drop any tests. If a warning is
> >>> a problem, we need to create new tests to silence the warning.
> >>
> >> Could you give a concrete example of a truly dropped test? Some pieces
> >> get moved, but I view this as fulfilling "create new tests to silence
> >> the warning".
> >
> > The patch is OK.
>
> Thanks much! Considering the impending branching of 2.34, personally
It sounds good. I will review the rest of patch series this week.
> I'd think it would be better for me to delay committing, such that
> this pretty significant change will get some more lingering time
> before actually hitting a release. What do you think? (And of course
> it would also seem better if some more of this series went in closely
> together with this patch.)
>
> Jan
--
H.J.
More information about the Binutils
mailing list