This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 06/10] x86: improve handling of insns with ambiguous operand sizes


On 08.08.2019 18:19,  H.J. Lu  wrote:
On Thu, Aug 8, 2019 at 12:56 AM Jan Beulich <JBeulich@suse.com> wrote:

On 08.08.2019 01:58,  H.J. Lu  wrote:
On Wed, Aug 7, 2019 at 1:27 AM Jan Beulich <jbeulich@suse.com> wrote:

On 06.08.2019 23:38,  H.J. Lu  wrote:
On Tue, Aug 6, 2019 at 1:26 PM H.J. Lu <hjl.tools@gmail.com> wrote:

On Tue, Aug 6, 2019 at 7:27 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).

If there may be an ambiguity, a size suffix can be used.  Assembler isn't
responsible for programmer's errors.

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

This punishes the perfectly good assembly sources.

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. The set of
"DefaultSize" one gets slightly widened for the purposes here.

Widen the default size set avoids generate warnings.  It sounds to
me these warnings isn't really necessary.

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 emitting of ambiguous operands (and hence triggering of the new
warning).

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


Instruction suffix has been an issue with AT&T syntax.  But improvements
shouldn't assemble current working assembly sources cleanly.

I guess you mean "should", not "shouldn't"? And "cleanly" to me does
not imply without warnings, just without change to generated code
(unless the generated code was outright wrong). Pointing out possible
issues should not be restricted to cases that didn't assemble without
error so far. Or else this would be one more argument against e.g.
your recent "REP;" handling adjustment.

   It is reasonable
to require that programmers should know what they are doing.  We should

1.  Require suffix if there may be ambiguity.

This would actively break existing code (reading "require" as "emit an
error if it is missing"). After all it's the present _inconsistency_
in behavior that this series tries to address.

2.  Generate a warning if needed under a new option, (-mambiguity-check=?).

First of all this contradicts 1 above: There's no point generating a
warning when we also generate an error. As to a separate option - if
you really don't want me to re-use the existing one (which is a pretty
good fit), I can certainly key this to a new one. But the default will
remain to be for the warning to be enabled.

3.  Document the default operand size for AT&T syntax if there may be ambiguity.

Will do.


Since AT&T syntax has no size info on memory operand, we can't tell memory
size when mnemonic supports more than one memory size.   Currently AT&T
syntax adds suffix to distinguish different memory sizes for these instructions.
For some instructions, there is a default memory size when there is no suffix.
In this sense, there is no ambiguity for assembler.   The only issue is that it
isn't easy for programmer to tell what the default memory size is.

To help programmers with these ambiguities when there is no suffix, we can

1. Document these ambiguities.
2. Issue warning when there is an exception in rules.   Warning should
be kept as few as possible.   For most parts, the default size is 32-bit.

"For most parts" is already telling enough, I think. Recall how

         shl     %cl, (%eax)

used to be wrongly deriving byte memory operand size from the
shift count register used? If things had been working flawlessly
so far, I'd maybe agree that there's no need for warnings by
default. But seeing how inconsistent _and broken_ things have
been, this is simply not an option.

add     $1, (%bx)

shouldn't generate a warning since the destination is 32-bit.

Good example (but of course I don't agree with the "no warning"
statement). Why is it then that

         add     $0x89, (%eax)
         add     $0x12345678, (%eax)

fail to assemble without the patch applied? Once again - may I
ask that you look at the additions to noreg*.s that this and
the subsequent patches do? These are all cases not working
currently, but which should work (and according to you even
without warning), and which this series makes work.

Given that in AT&T syntax needs the suffix for memory operand size,
we should document existing quirks and shouldn't accept more
instructions without suffix.

Why would that be? Why should there not be a mode in which
things work like for the reference assembler (which years ago
you've indicated the Solaris one would be, where their doc
clearly states that insns without suffix default to "long"),
as far is it makes sense, emitting diagnostics whenever gas
behavior may not conform to those original rules (like is
already done in most cases when suffix and register selection
aren't in sync)?

As to documenting existing "quirks": The example above is well
suited to also demonstrate that trying to document such "quirks"
would likely result in even more of a mess. Rather than
documenting anomalies, eliminating them should be preferred, when
this can sensibly be done (which this series demonstrates it can
here).

  We can add
a pseudo prefix {imm32} to request 32-bit immediate operand.

I don't see how this relates here.


We can encode

{imm32} addl   $0x70,(%rax)

with 32-bit immediate.

Sure, but I still don't see how it relates here. The entire series
is unconcerned with control over immediate sizes.

Jan


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]