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 6/6] x86: fold RegEip/RegRip and RegEiz/RegRiz


>>> On 03.08.18 at 17:33, <hjl.tools@gmail.com> wrote:
> On Thu, Aug 2, 2018 at 11:55 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 02.08.18 at 18:33, <hjl.tools@gmail.com> wrote:
>>> On Thu, Aug 2, 2018 at 9:06 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> On 02.08.18 at 14:49, <hjl.tools@gmail.com> wrote:
>>>>> We get an error today:
>>>>>
>>>>> [hjl@gnu-tools-1 tmp]$ cat x.s
>>>>> bndmk (%eip), %bnd2
>>>>> [hjl@gnu-tools-1 tmp]$ gcc -c x.s
>>>>> x.s: Assembler messages:
>>>>> x.s:1: Error: 32-bit address isn't allowed in 64-bit MPX instructions.
>>>>> [hjl@gnu-tools-1 tmp]$
>>>>>
>>>>> With your change, what do we get?
>>>>
>>>> `(%eip)' cannot be used here
>>>
>>>  .*  Error: 32-bit address isn't allowed in 64-bit MPX instructions\.
>>>  [      ]*[1-9][0-9]*[  ]+4C1903
>>>  [      ]*[1-9][0-9]*[  ]+bndmk \(%rip\), %bnd3
>>> -[      ]*[1-9][0-9]*[  ]+\?\?\?\? 67F30F1B             bndmk \(%eip\), %bnd2
>>> -.*  Error: 32-bit address isn't allowed in 64-bit MPX instructions\.
>>> -[      ]*[1-9][0-9]*[  ]+15000000
>>> -[      ]*[1-9][0-9]*[  ]+00
>>> +[      ]*[1-9][0-9]*[  ]+bndmk \(%eip\), %bnd2
>>>  [      ]*[1-9][0-9]*[  ]+
>>>  [      ]*[1-9][0-9]*[  ]+\#\#\# bndmov
>>>  [      ]*[1-9][0-9]*[  ]+\?\?\?\? 6766410F             bndmov \(%r8d\), %bnd1
>>>
>>> Why is there no error message for bndmk (%eip), %bnd2"?  Also
>>
>> Well, I've explained this already, but maybe the connection isn't
>> as obvious to you as I thought it would be: _Any_ errors appearing
>> in the listing are an indication of a problem, because it means the
>> assembler didn't simply bail after finding an error. In the case here
>> it is
>>
>>   if (i.tm.cpu_flags.bitfield.cpumpx)
>>     {
>>       if (flag_code == CODE_64BIT && i.prefix[ADDR_PREFIX])
>>         as_bad (_("32-bit address isn't allowed in 64-bit MPX instructions."));
>>       else if (flag_code != CODE_16BIT
>>                ? i.prefix[ADDR_PREFIX]
>>                : i.mem_operands && !i.prefix[ADDR_PREFIX])
>>         as_bad (_("16-bit address isn't allowed in MPX instructions"));
>>     }
>>
>> Note how there no "return" here. That's the very inconsistency I
> 
> We can add a return here.

And that's my plan going forward, as said.

>> did explain before (see the parenthesized statements in my second
>> most recent response, still visible in quoted context above), and
>> that I mean to get to fixing eventually.
>>
>> Bottom line - an error disappearing from the listing should not be
>> considered a problem, but an improvement (the only alternative
>> view I could accept would be if _all_ errors appeared in the listing).
>>
>>> `(%eip)' cannot be used here
>>>
>>> contains less info than
>>>
>>> 32-bit address isn't allowed in 64-bit MPX instructions
>>
>> Why that? Everyone ought to know that %eip is 32 bits. And once
>> again - it being wrong to use 32-bit _or_ IP-relative addressing,
>> there shouldn't be a requirement which of the two gets reported.
>> So in the end this change of error message is because the error
>> now comes from parse_operands(), in which case md_assemble()
>> bails rather than reaching the piece of quoted code above. I view
>> it as entirely unreasonable to complicate the code in
>> i386_index_check() just to be able to emit that other diagnostic.
>>
> 
> We should have the same error for both %eip and %eax for MPX
> in 64-bit mode.

I'm sorry, but I'm not going to cripple the change just so that your
unreasonable requirement can be met. Let's take a different example:

	add	(%cr1), (%cr0)
	add	(,%xmm1), (,%xmm0)

Are you telling me it matters whether the assembler complains about
the first or second operand being wrong, or there being two memory
operands when just one is allowed? Once again - when there are
multiple things wrong for a single insn, there should be no expectation
whatsoever as to which of the possible error messages actually gets
emitted, as that's an implementation detail of the assembler. If you
disagree here, then I'm looking forward for a very good explanation
of your standpoint.

Jan


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