[PATCH] D102712: BPF: Add more relocation kinds

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:954
   case ELF::R_BPF_NONE:
+  case ELF::R_BPF_64_ALLOC32:
     break;
----------------
This needs an ExecutionEngine test. Otherwise, don't touch the file in this patch.


================
Comment at: llvm/lib/Target/BPF/MCTargetDesc/BPFELFObjectWriter.cpp:45
     llvm_unreachable("invalid fixup kind!");
-  case FK_SecRel_8:
+  case FK_SecRel_4:
+    // LD_imm64 instruction.
----------------
FK_SecRel_4 is used by PE-COFF to represent SECREL relocation types.

"The 32-bit offset of the target from the beginning of its section. This is used to support debugging information and static thread local storage."

Why does it map to a regular looking R_BPF_64_64?


================
Comment at: llvm/lib/Target/BPF/MCTargetDesc/BPFELFObjectWriter.cpp:72
           if ((Flags & ELF::SHF_ALLOC) && (Flags & ELF::SHF_EXECINSTR))
-            return ELF::R_BPF_NONE;
+            return ELF::R_BPF_64_ALLOC32;
         } else {
----------------
This doesn't make sense to me. The relocation type names shouldn't be aware of SHF_ALLOC/non-SHF_ALLOC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102712