[PATCH v2 5/9] x86: improve handling of insns with ambiguous operand sizes
Jan Beulich
jbeulich@suse.com
Fri Nov 22 13:46:00 GMT 2019
On 22.11.2019 09:22, H.J. Lu wrote:
> 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?
Yes, and I suppose that's why it had got DefaultSize added (which
made be assume further such additions would be fine).
>> 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?
I have patches for the first three issues about ready, just need
to write the ChangeLog-s (hopefully on Monday).
Jan
More information about the Binutils
mailing list