[PATCH] D71100: [lld][RISCV] Fixup PC-relative relocations to undefined weak symbols.

Thu Dec 5 17:01:39 PST 2019

bsdjhb marked 2 inline comments as done.
bsdjhb added a comment.

This is needed to link a FreeBSD/riscv64 kernel which uses undefined weak symbols in the uart(4) driver.

================
Comment at: lld/ELF/InputSection.cpp:999
+          break;
+        case R_RISCV_JAL:
+          write32le(bufLoc, insn & 0x00000fff);
----------------
These cases do not handle compressed jump and compressed branch relocations.  I tried to use 'c.j' to in the new test case but the assembler turned it into a 4-byte 'j' instruction instead (even when forced to riscv32 and using '-mattr=+c'.  Right now those relocations would at least exit with an error, but if I can fix the test case to exercise them I'm happy to fix it.

================
Comment at: lld/ELF/InputSection.cpp:1019
+      if (!config->isPic) {
+        const Relocation *hiRel = getRISCVPCRelHi20(rel.sym, rel.addend);
+        if (hiRel && hiRel->type == R_RISCV_PCREL_HI20
----------------
This can result in duplicate warnings from getRISCVPCRelHi20().  One option @jrtc27 suggested was that perhaps we could lift the logic to compute the targetVA from hiRel out of getRelocTargetVA() to here so that we only call getRISCVPCRelHi20() once.  Another option might be to make the nested call in getRelocTargetVA() for 'hiRel' to instead return a targetVA of 0 by changing the `R_PC` for undefined weak symbols in RISC-V to return 0 instead of `dest - p`.  We kind of do this now for EM_PPC it seems by setting 'dest' to 'p' so that it effectively returns 0.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71100