Bug 21285 - R_386_GOT32 for baseless case looks implemented incorrectly.
Summary: R_386_GOT32 for baseless case looks implemented incorrectly.
Status: NEW
Alias: None
Product: binutils
Classification: Unclassified
Component: gold (show other bugs)
Version: 2.29
: P2 normal
Target Milestone: ---
Assignee: H.J. Lu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-22 11:23 UTC by georgerim
Modified: 2025-02-05 00:54 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed: 2017-05-13 00:00:00


Attachments
got32-i386.s.tmp.o (174 bytes, application/x-object)
2017-03-22 11:23 UTC, georgerim
Details

Note You need to log in before you can comment on or make changes to this bug.
Description georgerim 2017-03-22 11:23:57 UTC
Created attachment 9926 [details]
got32-i386.s.tmp.o

According to ABI, R_386_GOT32 (https://github.com/hjl-tools/x86-psABI/wiki/intel386-psABI-draft.pdf) can be calculated as G + A without base register when position-independent code is disabled.

Imagine next asm code:
.text
.globl bar
.type bar, @function
 bar:

.globl foo
.type foo, @function
 foo:

_start:
  movl bar@GOT, %eax
  movl foo@GOT, %ebx
  movl bar@GOT(%eax), %eax
  movl foo@GOT(%ebx), %eax

I used llvm-mc -relax-relocations=false to compile it into got32-i386.s.tmp.o (attached), its disassembly is:
Disassembly of section .text:
00000000 <bar>:
   0:	a1 00 00 00 00       	mov    0x0,%eax
   5:	8b 1d 00 00 00 00    	mov    0x0,%ebx
   b:	8b 80 00 00 00 00    	mov    0x0(%eax),%eax
  11:	8b 83 00 00 00 00    	mov    0x0(%ebx),%eax
Relocation section '.rel.text' at offset 0x8c contains 4 entries:
 Offset     Info    Type            Sym.Value  Sym. Name
00000001  00000203 R_386_GOT32       00000000   bar
00000007  00000303 R_386_GOT32       00000000   foo
0000000d  00000203 R_386_GOT32       00000000   bar
00000013  00000303 R_386_GOT32       00000000   foo

Notice that "mov 0x0,%eax" case is encoded using a1 and not 8b xx. Gold implementation determines baseless case using following check:
baseless = (view[-1] & 0xc7) == 0x5;
(https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gold/i386.cc;h=0b447ef0d68f3ea8965091502d5a1302d69d1b32;hb=HEAD#l2895)

That works incorrectly with "a1 00 00 00 00", gold uses 0xa1 opcode in this check, assuming it is ModRM byte, when it is not. And result calculation seems to be broken:
 08048094 <bar>:
 8048094:	a1 fc ff ff ff       	mov    0xfffffffc,%eax
 8048099:	8d 1d 94 80 04 08    	lea    0x8048094,%ebx
 804809f:	8d 80 a0 e0 ff ff    	lea    -0x1f60(%eax),%eax
 80480a5:	8d 83 a0 e0 ff ff    	lea    -0x1f60(%ebx),%eax

0xfffffffc is offset in GOT, calculated using "G + A - GOT", when it should be value calculated using "G + A" I think.
Comment 1 georgerim 2017-03-23 07:56:58 UTC
So gold implementation assumes that movl bar@GOT, %eax is always encoded using 0x8b, what is wrong, I think.

Because ABI says:
(https://github.com/hjl-tools/x86-psABI/wiki/intel386-psABI-1.1.pdf and https://github.com/hjl-tools/x86-psABI/wiki/intel386-psABI-draft.pdf p37)
"mov name@GOT, %eax must be encoded with opcode 0x8b, not 0xa0, to allow linker
optimization."

R_386_GOT32 is not supposed to be optimized, R_386_GOT32X is used for optimizations. That I believe means that using 0xa1 for encoding mov 0x0,%eax with R_386_GOT32 relocation is correct. And gold behavior is broken now.
Comment 2 Cary Coutant 2017-05-13 15:55:00 UTC
OK, suppose we have the following sequence of instructions:

  cmp  $0x8b, %al
  movl bar@GOT, %eax
  mov  bar@GOT(%ecx), %esp

which assemble to:

   0:	3c 8b                	cmp    $0x8b,%al
   2:	a1 00 00 00 00       	mov    0x0,%eax
   7:	8b a1 00 00 00 00    	mov    0x0(%ecx),%esp

How is the linker supposed to distinguish the two mov instructions? We cannot make this optimization based on looking at the opcodes -- we need separate relocations to tell us which is which, unless it's OK to invalidate 0xa1 as a valid modrm byte in this usage.

Doesn't ld have the same problem?
Comment 3 H.J. Lu 2017-05-15 15:07:25 UTC
(In reply to Cary Coutant from comment #2)
> OK, suppose we have the following sequence of instructions:
> 
>   cmp  $0x8b, %al
>   movl bar@GOT, %eax
>   mov  bar@GOT(%ecx), %esp
> 
> which assemble to:
> 
>    0:	3c 8b                	cmp    $0x8b,%al
>    2:	a1 00 00 00 00       	mov    0x0,%eax
>    7:	8b a1 00 00 00 00    	mov    0x0(%ecx),%esp
> 
> How is the linker supposed to distinguish the two mov instructions? We
> cannot make this optimization based on looking at the opcodes -- we need
> separate relocations to tell us which is which, unless it's OK to invalidate
> 0xa1 as a valid modrm byte in this usage.
> 
> Doesn't ld have the same problem?

movl bar@GOT, %eax

is only supported with the updated i386 psABI, which requires
"mov name@GOT, %eax must be encoded with opcode 0x8b, not 0xa0,
to allow linker optimization."
Comment 4 Cary Coutant 2017-05-15 18:34:17 UTC
> movl bar@GOT, %eax
> 
> is only supported with the updated i386 psABI, which requires
> "mov name@GOT, %eax must be encoded with opcode 0x8b, not 0xa0,
> to allow linker optimization."

How does this answer my question about distinguishing between the two instructions? Right now, gold is treating the 0xa1 as a modrm byte, and taking that as an indication that it's a mov instruction with indexing, without checking for the 0x8b opcode.

But even checking for the 0x8b isn't foolproof, given the instruction sequence I proposed. The only way to distinguish is to be more particular about what we accept as a modrm byte. We'd have to rule out 0xa1 as a valid modrm byte at the very least, so we can take that as the opcode and decline to optimize the instruction. If we see anything other than 0xa1, then we can treat it as a modrm byte, then go back to the previous byte and check for a 0x8b opcode.

Is that reasonable? Should we be even more particular about what we take as an acceptable modrm byte for the 0x8b form?

Side question: How do you force the assembler to generate the "0x8b 0x05 ..." form?
Comment 5 H.J. Lu 2017-05-15 19:08:56 UTC
(In reply to Cary Coutant from comment #4)
> > movl bar@GOT, %eax
> > 
> > is only supported with the updated i386 psABI, which requires
> > "mov name@GOT, %eax must be encoded with opcode 0x8b, not 0xa0,
> > to allow linker optimization."
> 
> How does this answer my question about distinguishing between the two
> instructions? Right now, gold is treating the 0xa1 as a modrm byte, and
> taking that as an indication that it's a mov instruction with indexing,
> without checking for the 0x8b opcode.

The old psABI doesn't support "movl bar@GOT, %eax".  The new psABI
supports "movl bar@GOT, %eax", which must be encoded with opcode 0x8b.
The behavior of "a1 00 00 00 00" is undefined.

> But even checking for the 0x8b isn't foolproof, given the instruction
> sequence I proposed. The only way to distinguish is to be more particular
> about what we accept as a modrm byte. We'd have to rule out 0xa1 as a valid
> modrm byte at the very least, so we can take that as the opcode and decline
> to optimize the instruction. If we see anything other than 0xa1, then we can
> treat it as a modrm byte, then go back to the previous byte and check for a
> 0x8b opcode.

In the case of "movl bar@GOT, %eax", it isn't an optimization.  Linker
can't subtract the GOT address.  Otherwise, the program will crash.

> Is that reasonable? Should we be even more particular about what we take as
> an acceptable modrm byte for the 0x8b form?
> 
> Side question: How do you force the assembler to generate the "0x8b 0x05
> ..." form?

The assembler skips opcode 0xa0 for GOT32 relocation:

     /* Force 0x8b encoding for "mov foo@GOT, %eax".  */
      if (i.reloc[0] == BFD_RELOC_386_GOT32 && t->base_opcode == 0xa0) 
        continue;