[PATCH] D157657: [RISCV] Support R_RISCV_SET_ULEB128/R_RISCV_SUB_ULEB128 for .uleb128 directives

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/include/llvm/MC/MCFixup.h:27
+  FK_Data_8,       ///< A eight-byte fixup.
+  FK_Data_uleb128, ///< A .uleb128 fixup.
+  FK_PCRel_1,      ///< A one-byte pc relative fixup.
----------------
jrtc27 wrote:
> I guess the sign is relevant here but not for the others because we have to know how to pad it? (Though what about FK_Data_8 on a 32-bit target?..)
I agree. Changed to `FK_Data_leb128`.

>  (Though what about FK_Data_8 on a 32-bit target?..)

They usually lead to errors.


================
Comment at: llvm/test/MC/ELF/RISCV/gen-dwarf.s:53
 # RELOC-NEXT:   0xD R_RISCV_64 .text.foo 0x0
+# RELOC-NEXT:   0x15 R_RISCV_SET_ULEB128 <null> 0x0
+# RELOC-NEXT:   0x15 R_RISCV_SUB_ULEB128 .text.foo 0x0
----------------
jrtc27 wrote:
> Is this actually the null symbol or has this just not read in the symbol table properly?
This is a symbol with an empty name, not symbol index 0.

llvm-readobj since 82b4368f7f89e2b16b9eb09787b7a7af07e3b7cd (Jul 2023) prints `<null>`, matching GNU readelf.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157657