[PATCH] D85782: [X86][ELF] Prefer lowering MC_GlobalAddress operands to .Lfoo$local only for STV_DEFAULT globals

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a subscriber: xiangzhangllvm.
MaskRay added a comment.

In D85782#2235567 <https://reviews.llvm.org/D85782#2235567>, @skan wrote:

> I think this introduces an old issue when lowering the instruction.
>
> Ref: https://sourceware.org/bugzilla/show_bug.cgi?id=13600
>
> **Reproducer:**
>
>   __attribute__((visibility("protected"))) void * foo (void) { return (void *)foo; }
>
>
>
>   clang -c  -O0  -fpic test.c 
>   clang -O0  -fpic test.o  -shared
>
> **Error message:**
> bfd/bin/ld: test.o: relocation R_X86_64_PC32 against protected symbol `foo' can not be used when making a shared object
>
> **Analysis:**
>  Consider visibility in canBenefitFromLocalAlias function limits the MC's ability to use local reference when selecting operand.
>
> Before this patch, the assemble looks like
>
>           .text
>           .file   "test.c"
>           .protected      foo                     # -- Begin function foo
>           .globl  foo
>           .p2align        4, 0x90
>           .type   foo, at function
>   foo:                                    # @foo
>   .Lfoo$local:
>           .cfi_startproc
>   # %bb.0:                                # %entry
>           pushq   %rbp
>           .cfi_def_cfa_offset 16
>           .cfi_offset %rbp, -16
>           movq    %rsp, %rbp
>           .cfi_def_cfa_register %rbp
>           leaq    .Lfoo$local(%rip), %rax
>           popq    %rbp
>           .cfi_def_cfa %rsp, 8
>           retq
>   .Lfunc_end0:
>
> `.Lfoo$local(%rip)` can be calculated directly and is not a relocation when emiting object file, so we no need to worry about the visibility of foo.
>
> After this patch
>
>   leaq    .Lfoo$local(%rip), %rax    ->  leaq    foo(%rip), %rax
>
> The visibility of foo conflicts with the relocation type..

tl;dr This is a longstanding GNU ld bug introduced in binutils 2.26

  % cat a.c
  __attribute__((visibility("protected"))) void * foo (void) { return (void *)foo; }
  % gcc -fpic a.c -shared -fuse-ld=bfd # relocation R_X86_64_PC32 against protected symbol `foo' can not be used when making a shared object

clang does not behave worse than GCC+GNU ld, so there is no regression on our side.

---

Longer answer:

binutils 2.26 introduced a regression: R_X86_64_PC32 can no longer be used against a protected symbol https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=ca3fe95e469b9daec153caa2c90665f5daaec2b5

The original issue is "Copy relocation against protected symbol doesn't work".
I agree with Rich Felker (https://gcc.gnu.org/ml/gcc/2016-04/msg00168.html) and
Cary Coutant (https://sourceware.org/ml/binutils/2016-03/msg00407.html https://gcc.gnu.org/ml/gcc/2016-04/msg00158.html https://gcc.gnu.org/ml/gcc/2016-04/msg00169.html) that we should
keep using direct access against protected symbols and disallow copy relocations against protected symbols.

I appreciate that Cary Coutant and Rafael Ávila de Espíndola added diagnostics to gold and lld, respectively:

- gold (https://sourceware.org/bugzilla/show_bug.cgi?id=19823)
- lld (https://bugs.llvm.org/show_bug.cgi?id=31476)

@xiangzhangllvm Perhaps you are in a good position to change the resolutions to the following issues:)

GCC 5 x86-64 introduced a regression (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65248)
i386 was flagged as a reproduce (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55012)

  __attribute__((visibility("protected"))) int a;
  int foo() { return a; } // GCC>=5 uses R_X86_64_GOTPCREL/R_X86_64_REX_GOTPCRELX instead of R_X86_64_PC32

binutils 2.26 introduced a regression R_X86_64_PC32 can no longer be used against a protected symbol https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=ca3fe95e469b9daec153caa2c90665f5daaec2b5


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85782/new/

https://reviews.llvm.org/D85782