x86: Don't set eh->local_ref to 1 for linker defined symbols

Message ID 20180517233054.GA8864@intel.com
State New
Headers show
Series
  • x86: Don't set eh->local_ref to 1 for linker defined symbols
Related show

Commit Message

H.J. Lu May 17, 2018, 11:30 p.m.
Since symbols created by HIDDEN and PROVIDE_HIDDEN assignments in
linker script may be marked as defined, but not hidden, we can't
set eh->local_ref to 1 in _bfd_x86_elf_link_symbol_references_local.

Also R_386_GOT32X should be handled as just like R_386_GOT32 when
relocating a section.  The input R_386_GOT32X relocations, which
can be relaxed, should have been converted to R_386_PC32, R_386_32
or R_386_GOTOFF.

Any comments?

H.J.
---
bfd/

	PR ld/23189
	* elf32-i386.c (elf_i386_relocate_section): Handle R_386_GOT32X
	like R_386_GOT32.
	* elfxx-x86.c (_bfd_x86_elf_link_symbol_references_local): Don't
	set eh->local_ref to 1 for linker defined symbols.

ld/

	PR ld/23189
	* testsuite/ld-i386/i386.exp: Run pr23189.
	* testsuite/ld-x86-64/x86-64.exp: Likewise.
	* testsuite/ld-i386/pr23189.d: New file.
	* testsuite/ld-i386/pr23189.s: Likewise.
	* testsuite/ld-i386/pr23189.t: Likewise.
	* testsuite/ld-x86-64/pr23189.d: Likewise.
	* testsuite/ld-x86-64/pr23189.s: Likewise.
	* testsuite/ld-x86-64/pr23189.t: Likewise.
---
 bfd/elf32-i386.c                  | 59 -------------------------------
 bfd/elfxx-x86.c                   |  6 +++-
 ld/testsuite/ld-i386/i386.exp     |  1 +
 ld/testsuite/ld-i386/pr23189.d    |  7 ++++
 ld/testsuite/ld-i386/pr23189.s    |  5 +++
 ld/testsuite/ld-i386/pr23189.t    | 11 ++++++
 ld/testsuite/ld-x86-64/pr23189.d  |  7 ++++
 ld/testsuite/ld-x86-64/pr23189.s  |  5 +++
 ld/testsuite/ld-x86-64/pr23189.t  | 11 ++++++
 ld/testsuite/ld-x86-64/x86-64.exp |  1 +
 10 files changed, 53 insertions(+), 60 deletions(-)
 create mode 100644 ld/testsuite/ld-i386/pr23189.d
 create mode 100644 ld/testsuite/ld-i386/pr23189.s
 create mode 100644 ld/testsuite/ld-i386/pr23189.t
 create mode 100644 ld/testsuite/ld-x86-64/pr23189.d
 create mode 100644 ld/testsuite/ld-x86-64/pr23189.s
 create mode 100644 ld/testsuite/ld-x86-64/pr23189.t

-- 
2.17.0

Comments

H.J. Lu May 21, 2018, 2:43 p.m. | #1
On Thu, May 17, 2018 at 4:30 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> Since symbols created by HIDDEN and PROVIDE_HIDDEN assignments in

> linker script may be marked as defined, but not hidden, we can't

> set eh->local_ref to 1 in _bfd_x86_elf_link_symbol_references_local.

>

> Also R_386_GOT32X should be handled as just like R_386_GOT32 when

> relocating a section.  The input R_386_GOT32X relocations, which

> can be relaxed, should have been converted to R_386_PC32, R_386_32

> or R_386_GOTOFF.

>

> Any comments?

>

> H.J.

> ---

> bfd/

>

>         PR ld/23189

>         * elf32-i386.c (elf_i386_relocate_section): Handle R_386_GOT32X

>         like R_386_GOT32.

>         * elfxx-x86.c (_bfd_x86_elf_link_symbol_references_local): Don't

>         set eh->local_ref to 1 for linker defined symbols.

>

> ld/

>

>         PR ld/23189

>         * testsuite/ld-i386/i386.exp: Run pr23189.

>         * testsuite/ld-x86-64/x86-64.exp: Likewise.

>         * testsuite/ld-i386/pr23189.d: New file.

>         * testsuite/ld-i386/pr23189.s: Likewise.

>         * testsuite/ld-i386/pr23189.t: Likewise.

>         * testsuite/ld-x86-64/pr23189.d: Likewise.

>         * testsuite/ld-x86-64/pr23189.s: Likewise.

>         * testsuite/ld-x86-64/pr23189.t: Likewise.


I am backporting it to binutils 2.30 branch.

> ---

>  bfd/elf32-i386.c                  | 59 -------------------------------

>  bfd/elfxx-x86.c                   |  6 +++-

>  ld/testsuite/ld-i386/i386.exp     |  1 +

>  ld/testsuite/ld-i386/pr23189.d    |  7 ++++

>  ld/testsuite/ld-i386/pr23189.s    |  5 +++

>  ld/testsuite/ld-i386/pr23189.t    | 11 ++++++

>  ld/testsuite/ld-x86-64/pr23189.d  |  7 ++++

>  ld/testsuite/ld-x86-64/pr23189.s  |  5 +++

>  ld/testsuite/ld-x86-64/pr23189.t  | 11 ++++++

>  ld/testsuite/ld-x86-64/x86-64.exp |  1 +

>  10 files changed, 53 insertions(+), 60 deletions(-)

>  create mode 100644 ld/testsuite/ld-i386/pr23189.d

>  create mode 100644 ld/testsuite/ld-i386/pr23189.s

>  create mode 100644 ld/testsuite/ld-i386/pr23189.t

>  create mode 100644 ld/testsuite/ld-x86-64/pr23189.d

>  create mode 100644 ld/testsuite/ld-x86-64/pr23189.s

>  create mode 100644 ld/testsuite/ld-x86-64/pr23189.t

>

> diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c

> index 580a591816..3dd709a10e 100644

> --- a/bfd/elf32-i386.c

> +++ b/bfd/elf32-i386.c

> @@ -2451,66 +2451,7 @@ skip_ifunc:

>        switch (r_type)

>         {

>         case R_386_GOT32X:

> -         /* Avoid optimizing _DYNAMIC since ld.so may use its

> -            link-time address.  */

> -         if (h == htab->elf.hdynamic)

> -           goto r_386_got32;

> -

> -         if (bfd_link_pic (info))

> -           {

> -             /* It is OK to convert mov to lea and convert indirect

> -                branch to direct branch.  It is OK to convert adc,

> -                add, and, cmp, or, sbb, sub, test, xor only when PIC

> -                is false.   */

> -             unsigned int opcode, addend;

> -             addend = bfd_get_32 (input_bfd, contents + rel->r_offset);

> -             if (addend != 0)

> -               goto r_386_got32;

> -             opcode = bfd_get_8 (input_bfd, contents + rel->r_offset - 2);

> -             if (opcode != 0x8b && opcode != 0xff)

> -               goto r_386_got32;

> -           }

> -

> -         /* Resolve "mov GOT[(%reg)], %reg",

> -            "call/jmp *GOT[(%reg)]", "test %reg, foo@GOT[(%reg)]"

> -            and "binop foo@GOT[(%reg)], %reg".  */

> -         if (h == NULL

> -             || (h->plt.offset == (bfd_vma) -1

> -                 && h->got.offset == (bfd_vma) -1)

> -             || htab->elf.sgotplt == NULL)

> -           abort ();

> -

> -         offplt = (htab->elf.sgotplt->output_section->vma

> -                   + htab->elf.sgotplt->output_offset);

> -

> -         /* It is relative to .got.plt section.  */

> -         if (h->got.offset != (bfd_vma) -1)

> -           /* Use GOT entry.  Mask off the least significant bit in

> -              GOT offset which may be set by R_386_GOT32 processing

> -              below.  */

> -           relocation = (htab->elf.sgot->output_section->vma

> -                         + htab->elf.sgot->output_offset

> -                         + (h->got.offset & ~1) - offplt);

> -         else

> -           /* Use GOTPLT entry.  */

> -           relocation = (h->plt.offset / plt_entry_size

> -                         - htab->plt.has_plt0 + 3) * 4;

> -

> -         if (!bfd_link_pic (info))

> -           {

> -             /* If not PIC, add the .got.plt section address for

> -                baseless addressing.  */

> -             unsigned int modrm;

> -             modrm = bfd_get_8 (input_bfd, contents + rel->r_offset - 1);

> -             if ((modrm & 0xc7) == 0x5)

> -               relocation += offplt;

> -           }

> -

> -         unresolved_reloc = FALSE;

> -         break;

> -

>         case R_386_GOT32:

> -r_386_got32:

>           /* Relocation is to the entry for this symbol in the global

>              offset table.  */

>           if (htab->elf.sgot == NULL)

> diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c

> index 936b4c67d2..f4dbddf15e 100644

> --- a/bfd/elfxx-x86.c

> +++ b/bfd/elfxx-x86.c

> @@ -2047,7 +2047,11 @@ _bfd_x86_elf_link_symbol_references_local (struct bfd_link_info *info,

>        return TRUE;

>      }

>

> -  eh->local_ref = 1;

> +  /* Symbols created by HIDDEN and PROVIDE_HIDDEN assignments in linker

> +     script aren't forced local here yet.  */

> +  if (!h->root.ldscript_def)

> +    eh->local_ref = 1;

> +

>    return FALSE;

>  }

>

> diff --git a/ld/testsuite/ld-i386/i386.exp b/ld/testsuite/ld-i386/i386.exp

> index 0d50b63d44..7aec5345cf 100644

> --- a/ld/testsuite/ld-i386/i386.exp

> +++ b/ld/testsuite/ld-i386/i386.exp

> @@ -479,6 +479,7 @@ run_dump_test "pr18815"

>  run_dump_test "pr19939a"

>  run_dump_test "pr19939b"

>  run_dump_test "tlsdesc2"

> +run_dump_test "pr23189"

>

>  proc undefined_weak {cflags ldflags} {

>      set testname "Undefined weak symbol"

> diff --git a/ld/testsuite/ld-i386/pr23189.d b/ld/testsuite/ld-i386/pr23189.d

> new file mode 100644

> index 0000000000..9345b42bd0

> --- /dev/null

> +++ b/ld/testsuite/ld-i386/pr23189.d

> @@ -0,0 +1,7 @@

> +#as: --32  -mrelax-relocations=yes

> +#ld: -shared -melf_i386 -T pr23189.t

> +#readelf: -r --wide

> +

> +Relocation section '.rel.dyn' at offset 0x[0-9a-f]+ contains 1 entry:

> + Offset     Info    Type                Sym. Value  Symbol's Name

> +[0-9a-f]+ +[0-9a-f]+ +R_386_RELATIVE +

> diff --git a/ld/testsuite/ld-i386/pr23189.s b/ld/testsuite/ld-i386/pr23189.s

> new file mode 100644

> index 0000000000..4a6c9cd078

> --- /dev/null

> +++ b/ld/testsuite/ld-i386/pr23189.s

> @@ -0,0 +1,5 @@

> +       .globl  _start

> +       .type   _start, @function

> +_start:

> +       movl    __hidden_sym@GOT(%eax), %eax

> +       .size   _start, .-_start

> diff --git a/ld/testsuite/ld-i386/pr23189.t b/ld/testsuite/ld-i386/pr23189.t

> new file mode 100644

> index 0000000000..0e6ff655b2

> --- /dev/null

> +++ b/ld/testsuite/ld-i386/pr23189.t

> @@ -0,0 +1,11 @@

> +EXTERN(_start)

> +ENTRY(_start)

> +

> +SECTIONS

> +{

> +  .text :

> +  {

> +    HIDDEN (__hidden_sym = .);

> +    *(.text*)

> +  }

> +}

> diff --git a/ld/testsuite/ld-x86-64/pr23189.d b/ld/testsuite/ld-x86-64/pr23189.d

> new file mode 100644

> index 0000000000..7951c7acc3

> --- /dev/null

> +++ b/ld/testsuite/ld-x86-64/pr23189.d

> @@ -0,0 +1,7 @@

> +#as: --64  -mrelax-relocations=yes

> +#ld: -shared -melf_x86_64 -T pr23189.t

> +#readelf: -r --wide

> +

> +Relocation section '.rela.dyn' at offset 0x[0-9a-f]+ contains 1 entry:

> +    Offset             Info             Type               Symbol's Value  Symbol's Name \+ Addend

> +[0-9a-f]+ +[0-9a-f]+ +R_X86_64_RELATIVE +[0-9a-f]+

> diff --git a/ld/testsuite/ld-x86-64/pr23189.s b/ld/testsuite/ld-x86-64/pr23189.s

> new file mode 100644

> index 0000000000..971ad8f892

> --- /dev/null

> +++ b/ld/testsuite/ld-x86-64/pr23189.s

> @@ -0,0 +1,5 @@

> +       .globl  _start

> +       .type   _start, @function

> +_start:

> +       movq    __hidden_sym@GOTPCREL(%rip), %rax

> +       .size   _start, .-_start

> diff --git a/ld/testsuite/ld-x86-64/pr23189.t b/ld/testsuite/ld-x86-64/pr23189.t

> new file mode 100644

> index 0000000000..0e6ff655b2

> --- /dev/null

> +++ b/ld/testsuite/ld-x86-64/pr23189.t

> @@ -0,0 +1,11 @@

> +EXTERN(_start)

> +ENTRY(_start)

> +

> +SECTIONS

> +{

> +  .text :

> +  {

> +    HIDDEN (__hidden_sym = .);

> +    *(.text*)

> +  }

> +}

> diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp

> index 5a06947057..563e782e2e 100644

> --- a/ld/testsuite/ld-x86-64/x86-64.exp

> +++ b/ld/testsuite/ld-x86-64/x86-64.exp

> @@ -606,6 +606,7 @@ run_dump_test "pr20253-5b"

>  run_dump_test "tlsdesc2"

>  run_dump_test "pr22048"

>  run_dump_test "pr22929"

> +run_dump_test "pr23189"

>

>  proc undefined_weak {cflags ldflags} {

>      set testname "Undefined weak symbol"

> --

> 2.17.0

>




-- 
H.J.

Patch

diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
index 580a591816..3dd709a10e 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -2451,66 +2451,7 @@  skip_ifunc:
       switch (r_type)
 	{
 	case R_386_GOT32X:
-	  /* Avoid optimizing _DYNAMIC since ld.so may use its
-	     link-time address.  */
-	  if (h == htab->elf.hdynamic)
-	    goto r_386_got32;
-
-	  if (bfd_link_pic (info))
-	    {
-	      /* It is OK to convert mov to lea and convert indirect
-		 branch to direct branch.  It is OK to convert adc,
-		 add, and, cmp, or, sbb, sub, test, xor only when PIC
-		 is false.   */
-	      unsigned int opcode, addend;
-	      addend = bfd_get_32 (input_bfd, contents + rel->r_offset);
-	      if (addend != 0)
-		goto r_386_got32;
-	      opcode = bfd_get_8 (input_bfd, contents + rel->r_offset - 2);
-	      if (opcode != 0x8b && opcode != 0xff)
-		goto r_386_got32;
-	    }
-
-	  /* Resolve "mov GOT[(%reg)], %reg",
-	     "call/jmp *GOT[(%reg)]", "test %reg, foo@GOT[(%reg)]"
-	     and "binop foo@GOT[(%reg)], %reg".  */
-	  if (h == NULL
-	      || (h->plt.offset == (bfd_vma) -1
-		  && h->got.offset == (bfd_vma) -1)
-	      || htab->elf.sgotplt == NULL)
-	    abort ();
-
-	  offplt = (htab->elf.sgotplt->output_section->vma
-		    + htab->elf.sgotplt->output_offset);
-
-	  /* It is relative to .got.plt section.  */
-	  if (h->got.offset != (bfd_vma) -1)
-	    /* Use GOT entry.  Mask off the least significant bit in
-	       GOT offset which may be set by R_386_GOT32 processing
-	       below.  */
-	    relocation = (htab->elf.sgot->output_section->vma
-			  + htab->elf.sgot->output_offset
-			  + (h->got.offset & ~1) - offplt);
-	  else
-	    /* Use GOTPLT entry.  */
-	    relocation = (h->plt.offset / plt_entry_size
-			  - htab->plt.has_plt0 + 3) * 4;
-
-	  if (!bfd_link_pic (info))
-	    {
-	      /* If not PIC, add the .got.plt section address for
-		 baseless addressing.  */
-	      unsigned int modrm;
-	      modrm = bfd_get_8 (input_bfd, contents + rel->r_offset - 1);
-	      if ((modrm & 0xc7) == 0x5)
-		relocation += offplt;
-	    }
-
-	  unresolved_reloc = FALSE;
-	  break;
-
 	case R_386_GOT32:
-r_386_got32:
 	  /* Relocation is to the entry for this symbol in the global
 	     offset table.  */
 	  if (htab->elf.sgot == NULL)
diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
index 936b4c67d2..f4dbddf15e 100644
--- a/bfd/elfxx-x86.c
+++ b/bfd/elfxx-x86.c
@@ -2047,7 +2047,11 @@  _bfd_x86_elf_link_symbol_references_local (struct bfd_link_info *info,
       return TRUE;
     }
 
-  eh->local_ref = 1;
+  /* Symbols created by HIDDEN and PROVIDE_HIDDEN assignments in linker
+     script aren't forced local here yet.  */
+  if (!h->root.ldscript_def)
+    eh->local_ref = 1;
+
   return FALSE;
 }
 
diff --git a/ld/testsuite/ld-i386/i386.exp b/ld/testsuite/ld-i386/i386.exp
index 0d50b63d44..7aec5345cf 100644
--- a/ld/testsuite/ld-i386/i386.exp
+++ b/ld/testsuite/ld-i386/i386.exp
@@ -479,6 +479,7 @@  run_dump_test "pr18815"
 run_dump_test "pr19939a"
 run_dump_test "pr19939b"
 run_dump_test "tlsdesc2"
+run_dump_test "pr23189"
 
 proc undefined_weak {cflags ldflags} {
     set testname "Undefined weak symbol"
diff --git a/ld/testsuite/ld-i386/pr23189.d b/ld/testsuite/ld-i386/pr23189.d
new file mode 100644
index 0000000000..9345b42bd0
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr23189.d
@@ -0,0 +1,7 @@ 
+#as: --32  -mrelax-relocations=yes
+#ld: -shared -melf_i386 -T pr23189.t
+#readelf: -r --wide
+
+Relocation section '.rel.dyn' at offset 0x[0-9a-f]+ contains 1 entry:
+ Offset     Info    Type                Sym. Value  Symbol's Name
+[0-9a-f]+ +[0-9a-f]+ +R_386_RELATIVE +
diff --git a/ld/testsuite/ld-i386/pr23189.s b/ld/testsuite/ld-i386/pr23189.s
new file mode 100644
index 0000000000..4a6c9cd078
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr23189.s
@@ -0,0 +1,5 @@ 
+	.globl	_start
+	.type	_start, @function
+_start:
+	movl	__hidden_sym@GOT(%eax), %eax
+	.size	_start, .-_start
diff --git a/ld/testsuite/ld-i386/pr23189.t b/ld/testsuite/ld-i386/pr23189.t
new file mode 100644
index 0000000000..0e6ff655b2
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr23189.t
@@ -0,0 +1,11 @@ 
+EXTERN(_start)
+ENTRY(_start)
+
+SECTIONS
+{
+  .text :
+  {
+    HIDDEN (__hidden_sym = .);
+    *(.text*)
+  }
+}
diff --git a/ld/testsuite/ld-x86-64/pr23189.d b/ld/testsuite/ld-x86-64/pr23189.d
new file mode 100644
index 0000000000..7951c7acc3
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr23189.d
@@ -0,0 +1,7 @@ 
+#as: --64  -mrelax-relocations=yes
+#ld: -shared -melf_x86_64 -T pr23189.t
+#readelf: -r --wide
+
+Relocation section '.rela.dyn' at offset 0x[0-9a-f]+ contains 1 entry:
+    Offset             Info             Type               Symbol's Value  Symbol's Name \+ Addend
+[0-9a-f]+ +[0-9a-f]+ +R_X86_64_RELATIVE +[0-9a-f]+
diff --git a/ld/testsuite/ld-x86-64/pr23189.s b/ld/testsuite/ld-x86-64/pr23189.s
new file mode 100644
index 0000000000..971ad8f892
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr23189.s
@@ -0,0 +1,5 @@ 
+	.globl	_start
+	.type	_start, @function
+_start:
+	movq	__hidden_sym@GOTPCREL(%rip), %rax
+	.size	_start, .-_start
diff --git a/ld/testsuite/ld-x86-64/pr23189.t b/ld/testsuite/ld-x86-64/pr23189.t
new file mode 100644
index 0000000000..0e6ff655b2
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr23189.t
@@ -0,0 +1,11 @@ 
+EXTERN(_start)
+ENTRY(_start)
+
+SECTIONS
+{
+  .text :
+  {
+    HIDDEN (__hidden_sym = .);
+    *(.text*)
+  }
+}
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index 5a06947057..563e782e2e 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -606,6 +606,7 @@  run_dump_test "pr20253-5b"
 run_dump_test "tlsdesc2"
 run_dump_test "pr22048"
 run_dump_test "pr22929"
+run_dump_test "pr23189"
 
 proc undefined_weak {cflags ldflags} {
     set testname "Undefined weak symbol"