[PATCH v3 01/10] x86: improve handling of insns with ambiguous operand sizes

Jan Beulich jbeulich@suse.com
Fri Jan 10 08:54:00 GMT 2020


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

Jan



More information about the Binutils mailing list