[PATCH] D152661: [XRay] Make xray_fn_idx entries PC-relative

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D152661#4430847 <https://reviews.llvm.org/D152661#4430847>, @ilammy wrote:

> In D145850#4412388 <https://reviews.llvm.org/D145850#4412388>, @MaskRay wrote:
>
>> `xray_fn_idx` entries should use PC-relative relocations as well, but I never get around to fix it. I wonder whether the change may make this patch unneeded or we are going to find other Mach-O relocation related issues.
>
> D145850 <https://reviews.llvm.org/D145850> is isolation fixes the linker issue for me even with `-fxray-function-index`.
>
> ---
>
> D152661 <https://reviews.llvm.org/D152661> – this change – applied in isolation still causes the same error:
>
>   ld: in section __DATA,xray_instr_map reloc 0: X86_64_RELOC_SUBTRACTOR must have r_extern=1 file '/var/folders/hv/h2_fjzgx1fl2mh85t65xsv6m0000gn/T/main-5630d5.o' for architecture x86_64
>
> with the linker not being able to handle relocations in `xray_instr_map` specifically.
>
> ---
>
> When if I apply D145850 <https://reviews.llvm.org/D145850> and then D152661 <https://reviews.llvm.org/D152661> on top of it, //assembly// appears to fail:
>
>   hello.s:53:8: error: expected relocatable expression
>           .quad   (lxray_sleds_end0-lxray_sleds_start0)>>4
>                   ^
>
> Assembler can't emit a relocation for this expression involving references to local symbols.
>
> EDIT: No idea why it wants it to be relocatable though, since it's effectively a constant. And you can't do relocations with divisions anyway...
>
> EDIT#2: And if I replace it with a literal:
>
>   .quad   12
>
> the linker is now offended by `xray_instr_idx` section:
>
>   ld: in section __DATA,xray_fn_idx reloc 0: X86_64_RELOC_SUBTRACTOR must have r_extern=1 file '/var/folders/hv/h2_fjzgx1fl2mh85t65xsv6m0000gn/T/hello-8f349d.o' for architecture x86_64
>
> since it can't find any extern symbol to latch onto for relocations in `xray_fn_idx`, with all of them referencing local or temporary symbols.

I think we should apply this patch and abandon D145850 <https://reviews.llvm.org/D145850>. The error you see is due to NOT using `L` (private label prefix).

          .section        __DATA,xray_instr_map
  lxray_sleds_start0:
          .space  13
  lxray_sleds_end0:
  
          .section        __DATA,xray_fn_idx
          .p2align        4, 0x90
          .quad   (lxray_sleds_end0-lxray_sleds_start0)>>4
  .subsections_via_symbols
  
  /tmp/x-5cd5fc.s:15:8: error: expected relocatable expression
   .quad (lxray_sleds_end0-lxray_sleds_start0)>>4

If we stick with `.quad   (Lxray_sleds_end0-Lxray_sleds_start0)>>4`, this expression can be resolved at assembly time, leaving no relocation to the linker:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152661