[Bug ld/27016] New: x86-64: GOTPCREL relaxation with abs symbol and REX byte creates incorrect code

[matz at suse dot de]

https://sourceware.org/bugzilla/show_bug.cgi?id=27016

            Bug ID: 27016
           Summary: x86-64: GOTPCREL relaxation with abs symbol and REX
                    byte creates incorrect code
           Product: binutils
           Version: 2.36 (HEAD)
            Status: NEW
          Severity: normal
          Priority: P2
         Component: ld
          Assignee: unassigned at sourceware dot org
          Reporter: matz at suse dot de
  Target Milestone: ---

Since the fix for PR ld/25749 and PR ld/25754, i.e. commit 382aae0632 ld
generates incorrect code in the following situation:
a) there's a GOTPCREL relocation (not REX_GOTPCRELX!)
b) the REX byte is necessary
c) the instruction is a mov
d) the relocation is against an absolute symbol

Due to the need of an absolute symbol an executable testcase is a bit
difficult, but this happens in the wild with old object files steming from
enterprise software linked during installation.  Can be reproduced with this:

% cat x.s
        .file   "x.c"
        .text
        .comm   global_int,4,4
        .globl  main
        .type   main, @function
main:
.LFB0:
        .cfi_startproc
        pushq   %rbp
        .cfi_def_cfa_offset 16
        .cfi_offset 6, -16
        movq    %rsp, %rbp
        .cfi_def_cfa_register 6
        movq    thesym@GOTPCREL(%rip), %r11
        movl    (%r11), %eax
        leal    1(%rax), %edx
        movq    thesym@GOTPCREL(%rip), %r11
        movl    %edx, (%r11)
        movl    $0, %eax
        popq    %rbp
        .cfi_def_cfa 7, 8
        ret
        .cfi_endproc
.LFE0:
        .size   main, .-main
        .ident  "GCC: (SUSE Linux) 9.2.1 20190903 [gcc-9-branch revision
275330]"
        .section        .note.GNU-stack,"",@progbits

% cat y.s
.globl thesym
thesym = 0x40402c

I've chose the value of this abs symbol to be the address of "global_int" in
the finally linked executable, so that it can be run.  Note how the main
function uses %r11 as destination register, i.e. the REX byte will be required
and must be correct in the rewritten instruction.

% as -mrelax-relocations=no -o x.o x.s
% as -mrelax-relocations=no -o y.o y.s
% ld-new --build-id --eh-frame-hdr -m elf_x86_64 -dynamic-linker
/lib64/ld-linux-x86-64.so.2 /usr/lib64/crt1.o /usr/lib64/crti.o
/usr/lib64/gcc/x86_64-suse-linux/9/crtbegin.o x.o y.o -lc 
/usr/lib64/gcc/x86_64-suse-linux/9/crtend.o /usr/lib64/crtn.o
% ./a.out
Segmentation fault

This is because the input .o file has this:
   4:   4c 8b 1d 00 00 00 00    mov    0x0(%rip),%r11        # b <main+0xb>
                        7: R_X86_64_GOTPCREL    thesym-0x4

(Note: not REX_GOTPCREL).  And the output a.out has this:

00000000004010f2 <main>:
  4010f2:       55                      push   %rbp
  4010f3:       48 89 e5                mov    %rsp,%rbp
  4010f6:       4c c7 c3 2c 40 40 00    rex.WR mov $0x40402c,%rbx
  4010fd:       41 8b 03                mov    (%r11),%eax

Note how the destination of insn main+4 is %rbx and there's an invalid REX
byte.

This is all because of this hunk in elf_x86_64_convert_load_reloc:

      if (r_type == R_X86_64_REX_GOTPCRELX)
        rex = bfd_get_8 (abfd, contents + roff - 3);
      else
        rex = 0;

      if (opcode == 0x8b)
        {
          if (abs_symbol && local_ref)
            to_reloc_pc32 = FALSE;
          if (to_reloc_pc32)
            // just rewrite into lea, don't touch REX byte
          else
            // rewrite into mov, and fiddle with REX byte

So, with an absolute symbol the code expect to be able to change the REX byte,
but with mere GOTPCREL relocs as here, it can't.  Possible patch for this:

--------------------------------------------------------------------
Fix for bsc#1179341

the movload->movconst relaxation can be done only with REX
rewriting, and hence needs a GOTPCRELX relocation.  With old object
files we might still see GOTPCREL relocs, even with REX bytes available.
We still can't do such rewriting and hence need to stay with the old
rewriting into a lea.
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index 549a8be6a6..b89b0023db 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -1731,7 +1731,7 @@ elf_x86_64_convert_load_reloc (bfd *abfd,

       if (opcode == 0x8b)
        {
-         if (abs_symbol && local_ref)
+         if (abs_symbol && local_ref && rex)
            to_reloc_pc32 = FALSE;

          if (to_reloc_pc32)

-- 
You are receiving this mail because:
You are on the CC list for the bug.