Fixed PPC32 local symbol relocation

[vit9696]

Summary

This fixes compatibility with llvm after r273499 and r273595, when LLVM started to emit PLT relocations even in static mode causing a regression.

[hfinkel]

If we're going to specifically comment on LLVM's behavior here, I think it is worth mentioning that GNU ld does the same thing. The comments should explain what the linker is doing, and why (e.g., support directly resolving PLT-relative relocations when statically linking).

Also, this will need a test case.

[vit9696]

Thanks, updated the patch.

[ruiu]

ELF/Arch/PPC.cpp

36

Does this say that this is not exactly what we are supposed to do to relocations of this type? If so, please state that first and what we should actually do (what type should actually be returned from this function?).

[vit9696]

@ruiu, alright, rewrote it more explicitly.

[ruiu]

Thanks! The new comment is much much better. With that, I can perfectly understand the situation.

Hal, are you okay with this change? I'll let you sign off.

[hfinkel]

In D39226#905510, @ruiu wrote:

Thanks! The new comment is much much better. With that, I can perfectly understand the situation.

Hal, are you okay with this change? I'll let you sign off.

Yep. LGTM.

[vit9696]

Ping

[nemanjai]

Sorry, why are you pinging the review? It is approved so you can commit it. Is the issue that you don't have commit access and need someone to commit it? If so, you should state that rather than just pinging the patch.

[vit9696]

Yes, I actually meant to ask someone to commit this for me. Given the timespan I thought it was evident :)

[vit9696]

This got merged, thx to tnorthover.

ELF/Arch/PPC.cpp

 };
 } // namespace
 
 RelExpr PPC::getRelExpr(RelType Type, const SymbolBody &S,
                         const uint8_t *Loc) const {
   switch (Type) {
   case R_PPC_REL24:
   case R_PPC_REL32:
     return R_PC;
+  // In general case R_PPC_PLTREL24 should result in R_PLT_PC, however, since
+  // PLT support is currently not available for PPC32 this workaround at least 
+  // allows lld to resolve local symbols when performing static linkage after
+  // LLVM started to forcibly use PLT relocations by default (see D38554).
+  // Non-local symbols will need a full PLT implementation, but once it lands
+  // local symbols should still avoid PLT table with static relocation model.
+  // This is the optimisation that bfd and gold are doing by default as well.
+  case R_PPC_PLTREL24:
+    return R_PC;
   default:
     return R_ABS;
   }
 }
 
 void PPC::relocateOne(uint8_t *Loc, RelType Type, uint64_t Val) const {
   switch (Type) {
   case R_PPC_ADDR16_HA:
     write16be(Loc, (Val + 0x8000) >> 16);
     break;
   case R_PPC_ADDR16_HI:
     write16be(Loc, Val >> 16);
     break;
   case R_PPC_ADDR16_LO:
     write16be(Loc, Val);
     break;
   case R_PPC_ADDR32:
   case R_PPC_REL32:
     write32be(Loc, Val);
     break;
+  case R_PPC_PLTREL24:
   case R_PPC_REL24:
     write32be(Loc, read32be(Loc) | (Val & 0x3FFFFFC));
     break;
   default:
     error(getErrorLocation(Loc) + "unrecognized reloc " + Twine(Type));
   }
 }
 
 TargetInfo *elf::getPPCTargetInfo() {
   static PPC Target;
   return &Target;
 }

test/ELF/ppc-relocs.s

 .FR_PPC_ADDR32:
   .long .Lfoox2
 .section .R_PPC_ADDR32_2,"ax",@progbits
 .Lfoox2:
 
 # CHECK: Disassembly of section .R_PPC_ADDR32:
 # CHECK: .FR_PPC_ADDR32:
 # CHECK:    11024:       00 01 10 28
+
+.align  2
+.section .R_PPC_PLTREL24,"ax",@progbits
+.globl .R_PPC_PLTREL24
+.FR_PPC_PLTREL24:
+  b .Lfoox4@PLT
+.section .R_PPC_PLTREL24_2,"ax",@progbits
+.Lfoox4:
+
+# CHECK: Disassembly of section .R_PPC_PLTREL24:
+# CHECK: .R_PPC_PLTREL24:
+# CHECK:    11028:       48 00 00 04     b .+4