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 v2 5/9] x86: improve handling of insns with ambiguous operand sizes


On Thu, Nov 21, 2019 at 12:05 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 15.11.2019 09:40, Jan Beulich wrote:
> > In any even - I'll see about finding time to investigate in how
> > far I can sensibly avoid at least some of the DefaultSize
> > additions.
>
> So I did some initial investigations, and there are pre-existing
> anomalies that should be taken care of:
> - PUSH/POP with a segment register don't get an operand size prefix
>   in .code16gcc mode. This is unlike _all_ other stack manipulating
>   instructions (with IRET as an explicitly intended exception). It
>   is clear why this is happening (and hence also clear how to fix
>   it), but I guess such a fix would need to be accompanied by a
>   warning, such that people using this in inline assembly would
>   become aware of the changed behavior (albeit I'd expect them to
>   use explicit suffixes anyway, or else they'd screw up their
>   stacks).

It sounds right.

> - SYSRET and XBEGIN get operand size prefixes for no apparent
>   reason. I think their DefaultSize should simply be dropped, for
>   them not accessing the stack. For SYSRET I guess this is to
>   suppress the "ambiguous operand size" diagnostic, but XBEGIN
>   doesn't allow any suffixes in the first place, and hence can't
>   trigger that diagnostic.

It sounds right.

> - ShortForm PUSH/POP have DefaultSize for no apparent reason -
>   process_suffix() will invent one based on the register used, and
>   hence both .defaultsize accesses are unreachable for them.
> - FNSTENV et al should have DefaultSize dropped as well. Their
>   explicit exclusion in process_suffix() stackop_size handling is
>   specifically why I assumed it would be okay to add further
>   exceptions here. It should be an all-or-nothing model, though.

Yes.  Will it trigger the "ambiguous operand size" error?

> I'd likely make addressing the first three prereqs to the patch
> here, and the 4th one a follow-up. Thoughts?
>

It sounds good.  Did you include a patch?

Thanks.

-- 
H.J.


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