[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