[PATCH] x86-64: Treat PC32 relocation with branch as PLT32

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] x86-64: Treat PC32 relocation with branch as PLT32

H.J. Lu-30
Since there is no need to prepare for PLT branch on x86-64, we can treat
PC32 relocation with branch as PLT32 relocation.  It allows building PIE
and shared objects from PC32 relocation with branch.

bfd/

        PR ld/22791
        * elf64-x86-64.c (is_32bit_relative_branch): Moved.
        (elf_x86_64_check_relocs): Treat PC32 relocations with branch
        as PLT32.
        (elf_x86_64_relocate_section): Check PIC relocations in PIE.
        Use PLT for PC32 relocations with branch.

ld/

        PR ld/22791
        * testsuite/ld-x86-64/x86-64.exp: Run PR ld/22791 tests.
        * testsuite/ld-x86-64/pr22791-1.err: New file.
        * testsuite/ld-x86-64/pr22791-1a.c: Likewise.
        * testsuite/ld-x86-64/pr22791-1b.s: Likewise.
        * testsuite/ld-x86-64/pr22791-2.rd: Likewise.
        * testsuite/ld-x86-64/pr22791-2a.c: Likewise.
        * testsuite/ld-x86-64/pr22791-2b.c: Likewise.
        * testsuite/ld-x86-64/pr22791-2c.c: Likewise.
---
 bfd/elf64-x86-64.c                   | 67 +++++++++++++++++++++++-------------
 ld/testsuite/ld-x86-64/pr22791-1.err |  2 ++
 ld/testsuite/ld-x86-64/pr22791-1a.c  |  4 +++
 ld/testsuite/ld-x86-64/pr22791-1b.s  |  6 ++++
 ld/testsuite/ld-x86-64/pr22791-2.rd  |  6 ++++
 ld/testsuite/ld-x86-64/pr22791-2a.c  |  7 ++++
 ld/testsuite/ld-x86-64/pr22791-2b.c  |  7 ++++
 ld/testsuite/ld-x86-64/pr22791-2c.c  |  8 +++++
 ld/testsuite/ld-x86-64/x86-64.exp    | 47 +++++++++++++++++++++++++
 9 files changed, 131 insertions(+), 23 deletions(-)
 create mode 100644 ld/testsuite/ld-x86-64/pr22791-1.err
 create mode 100644 ld/testsuite/ld-x86-64/pr22791-1a.c
 create mode 100644 ld/testsuite/ld-x86-64/pr22791-1b.s
 create mode 100644 ld/testsuite/ld-x86-64/pr22791-2.rd
 create mode 100644 ld/testsuite/ld-x86-64/pr22791-2a.c
 create mode 100644 ld/testsuite/ld-x86-64/pr22791-2b.c
 create mode 100644 ld/testsuite/ld-x86-64/pr22791-2c.c

diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index aad9b85296..ddd6fead4c 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -1747,6 +1747,24 @@ rewrite_modrm_rex:
   return TRUE;
 }
 
+/* Is the instruction before OFFSET in CONTENTS a 32bit relative
+   branch?  */
+
+static bfd_boolean
+is_32bit_relative_branch (bfd_byte *contents, bfd_vma offset)
+{
+  /* Opcode Instruction
+     0xe8 call
+     0xe9 jump
+     0x0f 0x8x conditional jump */
+  return ((offset > 0
+   && (contents [offset - 1] == 0xe8
+       || contents [offset - 1] == 0xe9))
+  || (offset > 1
+      && contents [offset - 2] == 0x0f
+      && (contents [offset - 1] & 0xf0) == 0x80));
+}
+
 /* Look through the relocs for a section during the first phase, and
    calculate needed space in the global offset table, procedure
    linkage table, and dynamic reloc sections.  */
@@ -2068,6 +2086,7 @@ elf_x86_64_check_relocs (bfd *abfd, struct bfd_link_info *info,
   if (h == NULL)
     continue;
 
+need_plt:
   eh->zero_undefweak &= 0x2;
   h->needs_plt = 1;
   h->plt.refcount = 1;
@@ -2088,6 +2107,16 @@ elf_x86_64_check_relocs (bfd *abfd, struct bfd_link_info *info,
   size_reloc = TRUE;
   goto do_size;
 
+ case R_X86_64_PC32:
+ case R_X86_64_PC32_BND:
+  /* NB: Treat PC32 relocation with branch as PLT32 since there
+     is no need to prepare for PLT branch on x86-64.   */
+  if (bfd_link_pic (info)
+      && h != NULL
+      && is_32bit_relative_branch (contents, rel->r_offset))
+    goto need_plt;
+  goto pointer;
+
  case R_X86_64_32:
   if (!ABI_64_P (abfd))
     goto pointer;
@@ -2113,8 +2142,6 @@ elf_x86_64_check_relocs (bfd *abfd, struct bfd_link_info *info,
 
  case R_X86_64_PC8:
  case R_X86_64_PC16:
- case R_X86_64_PC32:
- case R_X86_64_PC32_BND:
  case R_X86_64_PC64:
  case R_X86_64_64:
 pointer:
@@ -2307,24 +2334,6 @@ elf_x86_64_tpoff (struct bfd_link_info *info, bfd_vma address)
   return address - static_tls_size - htab->tls_sec->vma;
 }
 
-/* Is the instruction before OFFSET in CONTENTS a 32bit relative
-   branch?  */
-
-static bfd_boolean
-is_32bit_relative_branch (bfd_byte *contents, bfd_vma offset)
-{
-  /* Opcode Instruction
-     0xe8 call
-     0xe9 jump
-     0x0f 0x8x conditional jump */
-  return ((offset > 0
-   && (contents [offset - 1] == 0xe8
-       || contents [offset - 1] == 0xe9))
-  || (offset > 1
-      && contents [offset - 2] == 0x0f
-      && (contents [offset - 1] & 0xf0) == 0x80));
-}
-
 /* Relocate an x86_64 ELF section.  */
 
 static bfd_boolean
@@ -2986,6 +2995,7 @@ do_ifunc_pointer:
       break;
     }
 
+call_via_plt:
   if (h->plt.offset != (bfd_vma) -1)
     {
       if (htab->plt_second != NULL)
@@ -3023,14 +3033,18 @@ do_ifunc_pointer:
  case R_X86_64_PC32:
  case R_X86_64_PC32_BND:
   /* Don't complain about -fPIC if the symbol is undefined when
-     building executable unless it is unresolved weak symbol or
-     -z nocopyreloc is used.  */
+     building executable unless it is unresolved weak symbol,
+     references a dynamic definition in PIE or -z nocopyreloc
+     is used.  */
   if ((input_section->flags & SEC_ALLOC) != 0
       && (input_section->flags & SEC_READONLY) != 0
       && h != NULL
       && ((bfd_link_executable (info)
    && ((h->root.type == bfd_link_hash_undefweak
  && !resolved_to_zero)
+       || (bfd_link_pie (info)
+   && !h->def_regular
+   && h->def_dynamic)
        || ((info->nocopyreloc
     || (eh->def_protected
  && elf_has_no_copy_on_protected (h->root.u.def.section->owner)))
@@ -3044,7 +3058,14 @@ do_ifunc_pointer:
     || r_type == R_X86_64_PC32_BND)
    && is_32bit_relative_branch (contents, rel->r_offset));
 
-      if (SYMBOL_REFERENCES_LOCAL_P (info, h))
+      /* NB: We can use PLT for PC32 branch since there is no
+ need to prepare for PLT branch on x86-64.   */
+      if (branch
+  && htab->elf.splt != NULL
+  && (h->plt.offset != (bfd_vma) -1
+      || eh->plt_got.offset != (bfd_vma) -1))
+ goto call_via_plt;
+      else if (SYMBOL_REFERENCES_LOCAL_P (info, h))
  {
   /* Symbol is referenced locally.  Make sure it is
      defined locally or for a branch.  */
diff --git a/ld/testsuite/ld-x86-64/pr22791-1.err b/ld/testsuite/ld-x86-64/pr22791-1.err
new file mode 100644
index 0000000000..5500fa55ce
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr22791-1.err
@@ -0,0 +1,2 @@
+.*relocation R_X86_64_PC32 against symbol `foo' can not be used when making a PIE object; recompile with -fPIC
+#...
diff --git a/ld/testsuite/ld-x86-64/pr22791-1a.c b/ld/testsuite/ld-x86-64/pr22791-1a.c
new file mode 100644
index 0000000000..cd0130cacd
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr22791-1a.c
@@ -0,0 +1,4 @@
+void
+foo (void)
+{
+}
diff --git a/ld/testsuite/ld-x86-64/pr22791-1b.s b/ld/testsuite/ld-x86-64/pr22791-1b.s
new file mode 100644
index 0000000000..9751db49aa
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr22791-1b.s
@@ -0,0 +1,6 @@
+ .text
+ .globl main
+ .type main, @function
+main:
+ movl foo(%rip), %eax
+ .size main, .-main
diff --git a/ld/testsuite/ld-x86-64/pr22791-2.rd b/ld/testsuite/ld-x86-64/pr22791-2.rd
new file mode 100644
index 0000000000..70deb30d84
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr22791-2.rd
@@ -0,0 +1,6 @@
+#failif
+#...
+.*\(TEXTREL\).*
+#...
+[0-9a-f ]+R_X86_64_NONE.*
+#...
diff --git a/ld/testsuite/ld-x86-64/pr22791-2a.c b/ld/testsuite/ld-x86-64/pr22791-2a.c
new file mode 100644
index 0000000000..8b23ec8f2b
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr22791-2a.c
@@ -0,0 +1,7 @@
+extern void bar (void);
+
+void
+foo (void)
+{
+  bar ();
+}
diff --git a/ld/testsuite/ld-x86-64/pr22791-2b.c b/ld/testsuite/ld-x86-64/pr22791-2b.c
new file mode 100644
index 0000000000..79ef27c085
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr22791-2b.c
@@ -0,0 +1,7 @@
+#include <stdio.h>
+
+void
+bar (void)
+{
+  puts ("PASS");
+}
diff --git a/ld/testsuite/ld-x86-64/pr22791-2c.c b/ld/testsuite/ld-x86-64/pr22791-2c.c
new file mode 100644
index 0000000000..f1cb6b492b
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr22791-2c.c
@@ -0,0 +1,8 @@
+extern void foo (void);
+
+int
+main (void)
+{
+  foo ();
+  return 0;
+}
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index af3afcc2c7..8dac1c4dfa 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -1152,6 +1152,44 @@ if { [isnative] && [which $CC] != 0 } {
      {readelf -lW pr22393-3b.rd}} \
     "pr22393-3-static" \
  ] \
+ [list \
+    "Build pr22791-1.so" \
+    "-shared" \
+    "-fPIC" \
+    { pr22791-1a.c } \
+    {} \
+    "pr22791-1.so" \
+ ] \
+ [list \
+    "Build pr22791-1" \
+    "-pie -Wl,--no-as-needed tmpdir/pr22791-1.so" \
+    "$NOPIE_CFLAGS" \
+    { pr22791-1b.s } \
+    {{error_output "pr22791-1.err"}} \
+    "pr22791-1" \
+ ] \
+ [list \
+    "Build pr22791-2a.o" \
+    "" \
+    "-fno-PIC $NOPIE_CFLAGS" \
+    { pr22791-2a.c } \
+ ] \
+ [list \
+    "Build pr22791-2.so" \
+    "-shared tmpdir/pr22791-2a.o" \
+    "-fPIC" \
+    { pr22791-2b.c } \
+    {{readelf -drW pr22791-2.rd}} \
+    "pr22791-2.so" \
+ ] \
+ [list \
+    "Build pr22791-2" \
+    "-pie -Wl,--no-as-needed tmpdir/pr22791-2.so" \
+    "$NOPIE_CFLAGS" \
+    { pr22791-2c.c } \
+    {{readelf -drW pr22791-2.rd}} \
+    "pr22791-2" \
+ ] \
     ]
 
     if  {[istarget "x86_64-*-linux*-gnux32"]} {
@@ -1477,6 +1515,15 @@ if { [isnative] && [which $CC] != 0 } {
     "pr22393-3-static" \
     "pass.out" \
  ] \
+ [list \
+    "Run pr22791-2" \
+    "-pie -Wl,--no-as-needed tmpdir/pr22791-2.so" \
+    "" \
+    { pr22791-2c.c } \
+    "pr22791-2" \
+    "pass.out" \
+    "$NOPIE_CFLAGS" \
+ ] \
     ]
 
     # Run-time tests which require working ifunc attribute support.
--
2.14.3

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86-64: Treat PC32 relocation with branch as PLT32

Florian Weimer-5
On 02/06/2018 02:35 PM, H.J. Lu wrote:

> +/* Is the instruction before OFFSET in CONTENTS a 32bit relative
> +   branch?  */
> +
> +static bfd_boolean
> +is_32bit_relative_branch (bfd_byte *contents, bfd_vma offset)
> +{
> +  /* Opcode Instruction
> +     0xe8 call
> +     0xe9 jump
> +     0x0f 0x8x conditional jump */
> +  return ((offset > 0
> +   && (contents [offset - 1] == 0xe8
> +       || contents [offset - 1] == 0xe9))
> +  || (offset > 1
> +      && contents [offset - 2] == 0x0f
> +      && (contents [offset - 1] & 0xf0) == 0x80));
> +}

How is this safe, considering that R_X86_64_PC32 is also used for jump
tables and the like?

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86-64: Treat PC32 relocation with branch as PLT32

Cary Coutant-3
> How is this safe, considering that R_X86_64_PC32 is also used for jump
> tables and the like?

Agreed.

If it's a branch instruction that could be treated as PLT32, the
compiler should emit a PLT32 reloc. We already reduce PLT32 to PC32 in
the linker when the symbol is fully resolved, so the effect is the
same, but without the danger of accidentally changing a non-branch.

-cary
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86-64: Treat PC32 relocation with branch as PLT32

H.J. Lu-30
On Tue, Feb 6, 2018 at 10:15 AM, Cary Coutant <[hidden email]> wrote:

>> How is this safe, considering that R_X86_64_PC32 is also used for jump
>> tables and the like?
>
> Agreed.
>
> If it's a branch instruction that could be treated as PLT32, the
> compiler should emit a PLT32 reloc. We already reduce PLT32 to PC32 in
> the linker when the symbol is fully resolved, so the effect is the
> same, but without the danger of accidentally changing a non-branch.
>
Here is a patch to generate PLT32 reloc in assembler.  No compiler change
is needed.

--
H.J.

0001-x86-64-Generate-branch-with-PLT32-relocation.patch (27K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86-64: Treat PC32 relocation with branch as PLT32

H.J. Lu-30
In reply to this post by Florian Weimer-5
On Tue, Feb 6, 2018 at 6:55 AM, Florian Weimer <[hidden email]> wrote:

> On 02/06/2018 02:35 PM, H.J. Lu wrote:
>>
>> +/* Is the instruction before OFFSET in CONTENTS a 32bit relative
>> +   branch?  */
>> +
>> +static bfd_boolean
>> +is_32bit_relative_branch (bfd_byte *contents, bfd_vma offset)
>> +{
>> +  /* Opcode            Instruction
>> +     0xe8              call
>> +     0xe9              jump
>> +     0x0f 0x8x         conditional jump */
>> +  return ((offset > 0
>> +          && (contents [offset - 1] == 0xe8
>> +              || contents [offset - 1] == 0xe9))
>> +         || (offset > 1
>> +             && contents [offset - 2] == 0x0f
>> +             && (contents [offset - 1] & 0xf0) == 0x80));
>> +}
>
>
> How is this safe, considering that R_X86_64_PC32 is also used for jump
> tables and the like?
>
You are right.  This function should be removed.  Instead, we should generate
R_X86_64_PLT32 for 32-bit PC-relative branches:

https://groups.google.com/forum/#!topic/x86-64-abi/oJq_dXT9on8

I will check in this patch next week.

--
H.J.

0001-x86-64-Generate-branch-with-PLT32-relocation.patch (30K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86-64: Treat PC32 relocation with branch as PLT32

Florian Weimer-5
On 02/08/2018 04:56 PM, H.J. Lu wrote:

> Since there is no need to prepare for PLT branch on x86-64, generate
> R_X86_64_PLT32, instead of R_X86_64_PC32, if possible, which can be
> used as a marker for 32-bit PC-relative branches.
>
> To compiler Linux, this patch:
>
> From: "H.J. Lu"<[hidden email]>
> Subject: [PATCH] x86: Treat R_X86_64_PLT32 as R_X86_64_PC32
>
> On i386, there are 2 types of PLTs, PIC and non-PIC.  PIE and shared

The commit message text seems garbled.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86-64: Treat PC32 relocation with branch as PLT32

H.J. Lu-30
On Fri, Feb 9, 2018 at 6:57 AM, Florian Weimer <[hidden email]> wrote:

> On 02/08/2018 04:56 PM, H.J. Lu wrote:
>>
>> Since there is no need to prepare for PLT branch on x86-64, generate
>> R_X86_64_PLT32, instead of R_X86_64_PC32, if possible, which can be
>> used as a marker for 32-bit PC-relative branches.
>>
>> To compiler Linux, this patch:
>>
>> From: "H.J. Lu"<[hidden email]>
>> Subject: [PATCH] x86: Treat R_X86_64_PLT32 as R_X86_64_PC32
>>
>> On i386, there are 2 types of PLTs, PIC and non-PIC.  PIE and shared
>
>
> The commit message text seems garbled.
>

Looks normal to me:

https://sourceware.org/ml/binutils/2018-02/msg00085.html

--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86-64: Treat PC32 relocation with branch as PLT32

Florian Weimer-5
On 02/09/2018 04:21 PM, H.J. Lu wrote:
> Looks normal to me:
>
> https://sourceware.org/ml/binutils/2018-02/msg00085.html

Ah, lack of indentation and “To compiler Linux” confused me.

(I have not reviewed the patch.)

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86-64: Treat PC32 relocation with branch as PLT32

H.J. Lu-30
On Fri, Feb 9, 2018 at 8:38 AM, Florian Weimer <[hidden email]> wrote:
> On 02/09/2018 04:21 PM, H.J. Lu wrote:
>>
>> Looks normal to me:
>>
>> https://sourceware.org/ml/binutils/2018-02/msg00085.html
>
>
> Ah, lack of indentation and “To compiler Linux” confused me.
>

Oops.  It should read:

    To compile Linux kernel, this patch:

    From: "H.J. Lu" <[hidden email]>
    Subject: [PATCH] x86: Treat R_X86_64_PLT32 as R_X86_64_PC32


--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] x86-64: Treat PC32 relocation with branch as PLT32

H.J. Lu-30
On Fri, Feb 9, 2018 at 8:41 AM, H.J. Lu <[hidden email]> wrote:

> On Fri, Feb 9, 2018 at 8:38 AM, Florian Weimer <[hidden email]> wrote:
>> On 02/09/2018 04:21 PM, H.J. Lu wrote:
>>>
>>> Looks normal to me:
>>>
>>> https://sourceware.org/ml/binutils/2018-02/msg00085.html
>>
>>
>> Ah, lack of indentation and “To compiler Linux” confused me.
>>
>
> Oops.  It should read:
>
>     To compile Linux kernel, this patch:
>
>     From: "H.J. Lu" <[hidden email]>
>     Subject: [PATCH] x86: Treat R_X86_64_PLT32 as R_X86_64_PC32
>
This is what I am checking in.


--
H.J.

0001-x86-64-Generate-branch-with-PLT32-relocation.patch (30K) Download Attachment