[PATCH] x86: Update register operand check for AddrPrefixOpReg

H.J. Lu hjl.tools@gmail.com
Thu Oct 1 13:44:40 GMT 2020


On Thu, Oct 1, 2020 at 6:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 01.10.2020 15:07, H.J. Lu wrote:
> > On Thu, Oct 1, 2020 at 4:35 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> On Thu, Oct 1, 2020 at 1:22 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>
> >>> On 01.10.2020 01:30, H.J. Lu via Binutils wrote:
> >>>> I am checking in this patch and backporting it to 2.35 branch.
> >>>
> >>> But this is wrong, as can be seen from e.g. ...
> >>>> + +[a-f0-9]+: 66 0f 38 f8 0d 00 00 00 00      movdir64b 0x0\(%rip\),%rcx        #.*
> >>>> + +[a-f0-9]+: 67 66 0f 38 f8 0d 00 00 00 00   movdir64b 0x0\(%eip\),%ecx        #.*
> >>>> + +[a-f0-9]+: 67 66 0f 38 f8 0d 00 00 00 00   movdir64b 0x0\(%eip\),%ecx        #.*
> >>>
> >>> ... the middle line here not matching up with
> >>>
> >>>> +     movdir64b foo(%rip),%rcx
> >>>> +     movdir64b foo(%rip),%ecx
> >>>> +     movdir64b foo(%eip),%ecx
> >>>
> >>> ... what was written here. Without your change this
> >>>
> >>>         movdir64b (%rbp), %rax
> >>>         movdir64b (%rbp), %eax
> >>>         movdir64b (%ebp), %rax
> >>>         movdir64b (%ebp), %eax
> >>>
> >>>         movdir64b (%rip), %rax
> >>>         movdir64b (%rip), %eax
> >>>         movdir64b (%eip), %rax
> >>>         movdir64b (%eip), %eax
> >>>
> >>> yields consistent results for both blocks - the middle two entries
> >>> get an error issued.
> >>>
> >>> Please revert, and once again please don't commit (let alone
> >>> backport) patches without giving people at least _a little bit_ of
> >>> time to look at them.
> >>>
> >>
> >> The fix is correct.   This specific case came from
> >>
> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97257
> >>
> >> The address prefix changes the register operand in these instructions.
> >> (%rip) is a special case.
> >>
> >
> > Update the fix for DISP and SYMBOL.
>
> The description wrongly says:
>
> "When the address size prefix applies to both the memory and the register
>  operand, we need to extract the address size prefix from the register
>  operand if the memory operand has no real registers, like symbol, DISP
>  or symbol(%rip)."
>
> And then similarly a code comment. To the assembler, %rip _is_ a real
> register (and %eip could be used in its stead, when 32-bit addressing is
> desired).

GNU assembler serves GCC.  GCC doesn't use (%eip) and GCC always
uses (%rip) for RIP-relative addressing.

> Again - the previous change was wrong and needs undoing. I agree (largely,
> not fully anymore, despite my earlier indication of agreement) the plain
> symbol/disp case needs fixing; I'm unconvinced though that this is the
> place where the fix is to be made (but I didn't invest any time in looking
> for alternatives). As a general observation, such extended conditionals to
> address isolated cases are often, not always, a good sign that they're
> sitting in the wrong place.

This is a special case where the address size prefix applies to both the
memory and the register operands.  Since GCC always uses (%rip) for
RIP-relative addressing, we need to extract the address size from the
register operand in this case.

> As an aside - I wonder how gcc gets away with not using foo(%eip) for
> -mx32. But I assume this is due to what I'd call overly heavy restrictions
> in the x32 ABI spec.
>

--
H.J.


More information about the Binutils mailing list