[lld][PPC64] Fix location of the least significant byte when fixing up R_PPC64_ADDR14 for little-endian target

[lkail]

Unable to find a real world case, looks this relocation is seldom used now.

[nemanjai]

lld/ELF/Arch/PPC64.cpp

1224

Is this backwards? Shouldn't it be offset by 2 bytes for LE?

[lkail]

lld/ELF/Arch/PPC64.cpp

1224

We should agree with aalk is at loc[0] for LE. Then we should also agree with loc[1] contains the most significant 8-bits of the intermediate number. write16(loc, ...) writes loc[0] and loc[1] while write(loc + 2, ...) writes loc[2] and loc[3]. IIUC, when it's LE, where we want to write at should be loc[0] and loc[1].

[MaskRay]

This subtle change needs a test case to prevent future regression. If the assembler doesn't generate the problematic bit pattern, you can use .reloc directive.

[lkail]

This subtle change needs a test case to prevent future regression. If the assembler doesn't generate the problematic bit pattern, you can use .reloc directive.

I find lld doesn't handle R_PPC64_ADDR14 relocation in PPC64::getRelExpr at all.

t_le.o:	file format elf64-powerpcle

RELOCATION RECORDS FOR [.text]:
OFFSET           TYPE                     VALUE
0000000000000004 R_PPC64_ADDR14           .text


ld.lld: error: t_le.o:(.text+0x4): unknown relocation (7) against symbol

Shall we proceed to add support for it or remove handling R_PPC64_ADDR14 from PPC64::relocate? In fact, what we are using mostly is R_PPC64_REL14 which is well handled.

[lkail]

lld/ELF/Arch/PPC64.cpp

1224

Here's an example

t_be.o:	file format elf64-powerpc

Disassembly of section .text:

0000000000000000 <bca_target>:
       0: 4e 80 00 20  	blr

0000000000000004 <check_reloc_addr14>:
       4: 42 9f 00 02  	bca 20, 31, 0

t_le.o:	file format elf64-powerpcle

Disassembly of section .text:

0000000000000000 <bca_target>:
       0: 20 00 80 4e  	blr

0000000000000004 <check_reloc_addr14>:
       4: 02 00 9f 42  	bca 20, 31, 0

[MaskRay]

In D157365#4571720, @lkail wrote:

This subtle change needs a test case to prevent future regression. If the assembler doesn't generate the problematic bit pattern, you can use .reloc directive.

I find lld doesn't handle R_PPC64_ADDR14 relocation in PPC64::getRelExpr at all.

t_le.o:	file format elf64-powerpcle

RELOCATION RECORDS FOR [.text]:
OFFSET           TYPE                     VALUE
0000000000000004 R_PPC64_ADDR14           .text


ld.lld: error: t_le.o:(.text+0x4): unknown relocation (7) against symbol

Shall we proceed to add support for it or remove handling R_PPC64_ADDR14 from PPC64::relocate? In fact, what we are using mostly is R_PPC64_REL14 which is well handled.

We can probably remove the incomplete linker support for R_PPC64_ADDR14. It seems that MC assembler doesn't produce R_PPC64_ADDR14.

commit 3c8cc677e6dbf29e59343142a5f1627427f58844 (2015) added R_PPC64_ADDR14 without a test, and the code path might be dead.

[lkail]

It seems that MC assembler doesn't produce R_PPC64_ADDR14

Actually, it does for following code.

bca_target:
  blr

check_reloc_addr14:
  bca 20,31,bca_target

llvm-mc -filetype=obj t.s -o t.o -triple=powerpc64le, and llvm-objdump -r t.o outputs

t.o:	file format elf64-powerpcle

RELOCATION RECORDS FOR [.text]:
OFFSET           TYPE                     VALUE
0000000000000004 R_PPC64_ADDR14           .text

lld/ELF/Arch/PPC64.cpp

void PPC64::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
   // For dynamic thread pointer relative, toc-relative, and got-indirect
   // relocations, proceed in terms of the corresponding ADDR16 relocation type.
   std::tie(type, val) = toAddr16Rel(type, val);
 
   switch (type) {
   case R_PPC64_ADDR14: {
     checkAlignment(loc, val, 4, rel);
     // Preserve the AA/LK bits in the branch instruction
-    uint8_t aalk = loc[3];
-    write16(loc + 2, (aalk & 3) | (val & 0xfffc));
+    uint8_t aalk = loc[config->isLE ? 0 : 3];
+    write16(config->isLE ? loc : loc + 2, (aalk & 3) | (val & 0xfffc));
     break;
   }
   case R_PPC64_ADDR16:
     checkIntUInt(loc, val, 16, rel);
     write16(loc, val);
     break;
   case R_PPC64_ADDR32:
     checkIntUInt(loc, val, 32, rel);