PR16794, gold ignores R_386_GOTOFF addend

Message ID 20190926052338.GH19242@bubble.grove.modra.org
State New
Headers show
Series
  • PR16794, gold ignores R_386_GOTOFF addend
Related show

Commit Message

Alan Modra Sept. 26, 2019, 5:23 a.m.
An R_386_GOTOFF relocation has an addend, typically used when a
symbol can be replaced by its section symbol plus an offset.
psymval->value(object,0) is quite wrong then, fix it.  The same goes
for R_X86_64_GOTOFF64.

Note that R_X86_64_GOTOFF64 isn't easy to generate from gcc, and may
in fact be impossible nowadays.  Also, x86 gas won't reduce symbols to
section symbols on R_386_GOTOFF and R_X86_64_GOTOFF64.  I suspect that
isn't necessary, but I'm leaving that to HJ to investigate.  (Keeping
the original symbol is necessary for most GOT relocs in order to
generate the proper GOT entry because the ABI says to put the symbol
in the GOT entry, not symbol plus addend.  The addend becomes a
useless offset from the GOT entry.  But anyway, these two relocs don't
generate a GOT entry.)

OK?

	PR 16794
	* i386.cc (Target_i386::Relocate::relocate <R_386_GOTOFF>): Don't
	ignore addend, apply using pcrel32.
	* x86_64.cc (Target_x86_64::Relocate::relocate <R_X86_64_GOTOFF64>):
	Similarly use pcrel64.


-- 
Alan Modra
Australia Development Lab, IBM

Comments

H.J. Lu Sept. 26, 2019, 4:09 p.m. | #1
On Wed, Sep 25, 2019 at 10:23 PM Alan Modra <amodra@gmail.com> wrote:
>

> An R_386_GOTOFF relocation has an addend, typically used when a

> symbol can be replaced by its section symbol plus an offset.

> psymval->value(object,0) is quite wrong then, fix it.  The same goes

> for R_X86_64_GOTOFF64.

>

> Note that R_X86_64_GOTOFF64 isn't easy to generate from gcc, and may

> in fact be impossible nowadays.  Also, x86 gas won't reduce symbols to

> section symbols on R_386_GOTOFF and R_X86_64_GOTOFF64.  I suspect that

> isn't necessary, but I'm leaving that to HJ to investigate.  (Keeping

> the original symbol is necessary for most GOT relocs in order to

> generate the proper GOT entry because the ABI says to put the symbol

> in the GOT entry, not symbol plus addend.  The addend becomes a

> useless offset from the GOT entry.  But anyway, these two relocs don't

> generate a GOT entry.)

>


I got

[hjl@gnu-cfl-1 tmp]$ cat y.s
.text
.globl foo
.type foo, @function
foo:
leal 2+bar@GOTOFF(%eax), %eax
.size foo, .-foo
.data
.align 4
.type bar, @object
.size bar, 10
bar:
.string "\001"
.zero 8
[hjl@gnu-cfl-1 tmp]$ gcc -c -m32 y.s
[hjl@gnu-cfl-1 tmp]$ readelf -r y.o

Relocation section '.rel.text' at offset 0x110 contains 1 entry:
 Offset     Info    Type            Sym.Value  Sym. Name
00000002  00000209 R_386_GOTOFF      00000000   .data
[hjl@gnu-cfl-1 tmp]$

GOTOFF relocation against the local symbol is reduced to
section.

-- 
H.J.
Alan Modra Sept. 27, 2019, 6:52 a.m. | #2
On Thu, Sep 26, 2019 at 09:09:26AM -0700, H.J. Lu wrote:
> On Wed, Sep 25, 2019 at 10:23 PM Alan Modra <amodra@gmail.com> wrote:

> >

> > An R_386_GOTOFF relocation has an addend, typically used when a

> > symbol can be replaced by its section symbol plus an offset.

> > psymval->value(object,0) is quite wrong then, fix it.  The same goes

> > for R_X86_64_GOTOFF64.

> >

> > Note that R_X86_64_GOTOFF64 isn't easy to generate from gcc, and may

> > in fact be impossible nowadays.  Also, x86 gas won't reduce symbols to

> > section symbols on R_386_GOTOFF and R_X86_64_GOTOFF64.  I suspect that

> > isn't necessary, but I'm leaving that to HJ to investigate.  (Keeping

> > the original symbol is necessary for most GOT relocs in order to

> > generate the proper GOT entry because the ABI says to put the symbol

> > in the GOT entry, not symbol plus addend.  The addend becomes a

> > useless offset from the GOT entry.  But anyway, these two relocs don't

> > generate a GOT entry.)

> >

> 

> I got

> 

> [hjl@gnu-cfl-1 tmp]$ cat y.s

> .text

> .globl foo

> .type foo, @function

> foo:

> leal 2+bar@GOTOFF(%eax), %eax

> .size foo, .-foo

> .data

> .align 4

> .type bar, @object

> .size bar, 10

> bar:

> .string "\001"

> .zero 8

> [hjl@gnu-cfl-1 tmp]$ gcc -c -m32 y.s

> [hjl@gnu-cfl-1 tmp]$ readelf -r y.o

> 

> Relocation section '.rel.text' at offset 0x110 contains 1 entry:

>  Offset     Info    Type            Sym.Value  Sym. Name

> 00000002  00000209 R_386_GOTOFF      00000000   .data

> [hjl@gnu-cfl-1 tmp]$

> 

> GOTOFF relocation against the local symbol is reduced to

> section.


Huh, so it is.  Anyway, do you agree that the gold patches are correct?

This case isn't reduced to a section symbol:
 .text
 .globl foo
 .type foo, @function
foo:
 leal 2+bar@GOTOFF(%eax), %eax
 .size foo, .-foo
 .section .rodata.str.1,"ams",1
 .type bar, @object
 .size bar, 4
bar:
 .string "abc"

Nor this:
 .text
 .global _start
_start:
 nop
 nop
.L1:
 nop
 nop
 nop
 nop
.L2:
 nop
 nop
 nop
 nop
 nop
 nop
 nop
 nop
.L3:
 nop
.L4:
 .data
 .dc.a .L1@gotoff
 .dc.a .L2@gotoff
 .dc.a .L3@gotoff
 .dc.a .L4@gotoff

-- 
Alan Modra
Australia Development Lab, IBM
H.J. Lu Sept. 27, 2019, 4:56 p.m. | #3
On Thu, Sep 26, 2019 at 11:52 PM Alan Modra <amodra@gmail.com> wrote:
>

> On Thu, Sep 26, 2019 at 09:09:26AM -0700, H.J. Lu wrote:

> > On Wed, Sep 25, 2019 at 10:23 PM Alan Modra <amodra@gmail.com> wrote:

> > >

> > > An R_386_GOTOFF relocation has an addend, typically used when a

> > > symbol can be replaced by its section symbol plus an offset.

> > > psymval->value(object,0) is quite wrong then, fix it.  The same goes

> > > for R_X86_64_GOTOFF64.

> > >

> > > Note that R_X86_64_GOTOFF64 isn't easy to generate from gcc, and may

> > > in fact be impossible nowadays.  Also, x86 gas won't reduce symbols to

> > > section symbols on R_386_GOTOFF and R_X86_64_GOTOFF64.  I suspect that

> > > isn't necessary, but I'm leaving that to HJ to investigate.  (Keeping

> > > the original symbol is necessary for most GOT relocs in order to

> > > generate the proper GOT entry because the ABI says to put the symbol

> > > in the GOT entry, not symbol plus addend.  The addend becomes a

> > > useless offset from the GOT entry.  But anyway, these two relocs don't

> > > generate a GOT entry.)

> > >

> >

> > I got

> >

> > [hjl@gnu-cfl-1 tmp]$ cat y.s

> > .text

> > .globl foo

> > .type foo, @function

> > foo:

> > leal 2+bar@GOTOFF(%eax), %eax

> > .size foo, .-foo

> > .data

> > .align 4

> > .type bar, @object

> > .size bar, 10

> > bar:

> > .string "\001"

> > .zero 8

> > [hjl@gnu-cfl-1 tmp]$ gcc -c -m32 y.s

> > [hjl@gnu-cfl-1 tmp]$ readelf -r y.o

> >

> > Relocation section '.rel.text' at offset 0x110 contains 1 entry:

> >  Offset     Info    Type            Sym.Value  Sym. Name

> > 00000002  00000209 R_386_GOTOFF      00000000   .data

> > [hjl@gnu-cfl-1 tmp]$

> >

> > GOTOFF relocation against the local symbol is reduced to

> > section.

>

> Huh, so it is.  Anyway, do you agree that the gold patches are correct?


It looks correct to me.   pcrel is used with GOT address , instead of
relocation address.

> This case isn't reduced to a section symbol:

>  .text

>  .globl foo

>  .type foo, @function

> foo:

>  leal 2+bar@GOTOFF(%eax), %eax

>  .size foo, .-foo

>  .section .rodata.str.1,"ams",1

>  .type bar, @object

>  .size bar, 4

> bar:

>  .string "abc"

>

> Nor this:

>  .text

>  .global _start

> _start:

>  nop

>  nop

> .L1:

>  nop

>  nop

>  nop

>  nop

> .L2:

>  nop

>  nop

>  nop

>  nop

>  nop

>  nop

>  nop

>  nop

> .L3:

>  nop

> .L4:

>  .data

>  .dc.a .L1@gotoff

>  .dc.a .L2@gotoff

>  .dc.a .L3@gotoff

>  .dc.a .L4@gotoff

>


These must be some special cases.


-- 
H.J.

Patch

diff --git a/gold/i386.cc b/gold/i386.cc
index dd0b268e10..2d3db7c2c2 100644
--- a/gold/i386.cc
+++ b/gold/i386.cc
@@ -2957,10 +2957,9 @@  Target_i386::Relocate::relocate(const Relocate_info<32, false>* relinfo,
 
     case elfcpp::R_386_GOTOFF:
       {
-	elfcpp::Elf_types<32>::Elf_Addr value;
-	value = (psymval->value(object, 0)
-		 - target->got_plt_section()->address());
-	Relocate_functions<32, false>::rel32(view, value);
+	elfcpp::Elf_types<32>::Elf_Addr reladdr;
+	reladdr = target->got_plt_section()->address();
+	Relocate_functions<32, false>::pcrel32(view, object, psymval, reladdr);
       }
       break;
 
diff --git a/gold/x86_64.cc b/gold/x86_64.cc
index c06a282247..bafd90ef5e 100644
--- a/gold/x86_64.cc
+++ b/gold/x86_64.cc
@@ -4852,10 +4852,9 @@  Target_x86_64<size>::Relocate::relocate(
 
     case elfcpp::R_X86_64_GOTOFF64:
       {
-	typename elfcpp::Elf_types<size>::Elf_Addr value;
-	value = (psymval->value(object, 0)
-		 - target->got_plt_section()->address());
-	Reloc_funcs::rela64(view, value, addend);
+	typename elfcpp::Elf_types<size>::Elf_Addr reladdr;
+	reladdr = target->got_plt_section()->address();
+	Reloc_funcs::pcrela64(view, object, psymval, addend, reladdr);
       }
       break;