[RISCV] Emit relocation for uleb128

[kito-cheng]

Summary

Support for the uleb128 related relocation[1], ULEB128 format has used
by DWARF and exception handling table, which used ULEB128 to record data
and the distance of two symbols, however the linker relaxation might
change the symbol address, so we need to insert relocations for those
data to make sure they are still correct after linker relaxation.

[1] https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/361

[kito-cheng]

Changes:

• Add testcase.

[craig.topper]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

295

Looks like most other places in RISCV use MCFixupKind(RISCV::fixup_riscv_set_uleb128) instead of the static_cast.

[jrtc27]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

283

encodeULEB128 takes a uint64_t

287

Why not just use encodeULEB128's PadTo and encode 0?

[MaskRay]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

277

delete the blank line. The prevailing LLVM style does not prefer inserting a blank line after a declaration.

287

-1ll is not tested. This needs a test with llvm-readelf -x $name or llvm-objdump -s dumping the section content.

llvm/test/MC/RISCV/fixups-expr.s

30

Suggest: create a new file for .uleb128. Add a test where .uleb128 precedes the label definition (it currently crashes).

Add a test with labels defined relative to two different sections.

I don't have a very strong opinion about whether non-local symbols should be allowed. For psABI, it may be safer to start with more restriction.

[craig.topper]

llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def

57

59 has been claimed by R_RISCV_PLT32 now.

[kito-cheng]

Changes:

• Update relocation number to 60 and 61.
• Move testcase to fixups-expr-uleb128.s
• Reserved enough bytes when the symbol can't evaluated at assembly time.

[kito-cheng]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

283

evaluateAsAbsolute take int64_t &...added a check to make sure that won't overflow.

[kito-cheng]

Changes:

• Rebase
• Add new option to control the relocation emition for ULEB128: -riscv-enable-uleb128
• Default disable to prevent potential incompatible issue with old binutils, will default turn on in future.

[kito-cheng]

NOTE: psABI spec has merged and binutils support also merged into trunk.

[kito-cheng]

ping :)

[MaskRay]

This patch looks fine as to implement the functionality, but I am afraid that the used requiresFixup and evaluateAsAbsolute are not really reliable (https://maskray.me/blog/2021-03-14-the-dark-side-of-riscv-linker-relaxation "Label differences related to text section symbols"). Ideally we should not need`EnableULEB128Reloc`.

I plan to improve the reliability shortly.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

32

[kito-cheng]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

32

The intention of this option is considering the binutils compatibility, binuilts has support that on trunk which will be part of binutils 2.41, but it's still not release yet.

Anyway personally I don't have strong opinion on the default value of this option.

[kito-cheng]

Changes:

• Rebase to trunk.
• Apply @MaskRay's suggesion.

[MaskRay]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

277

evaluateAsAbsolute has now been correct (D153097). I am now mainly concerned with our inaccurate requiresFixups and whether there will be users rely on a possibly buggy behavior...

llvm/test/MC/RISCV/fixups-expr-uleb128.s

60 ↗

(On Diff #538013)

Add -NEXT whenever applicable. Check the preceding line .rela.xxx { and the the pairing } to assert that there is no other relocations.

[kito-cheng]

Changes:

• Update testcase to make sure no other unexpected relocation.

[kito-cheng]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

277

Did you have suggestion on this, I guess I am not really family enough around that :(

[MaskRay]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

277

I took quite some time today to investigate it.... See D155357

I think we need a custom fixup to represent the fixup_riscv_set_uleb128 and fixup_riscv_sub_uleb128 pair.

In .handleAddSubRelocations, split the fixup into 2 relocations.

[MaskRay]

llvm/test/MC/RISCV/fixups-expr-uleb128.s

59 ↗

(On Diff #540288)

The interesting case is when we use a linker-relaxable instruction here.

+relax and -relax can check the behavior. See my llvm/test/MC/RISCV/fixups-expr.s change.

[kito-cheng]

Changes:

• Update testcase
• Based on D155357

[kito-cheng]

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

277

Thanks, I am based on D155357 now :)

[kito-cheng]

ping

[MaskRay]

I am out of town for a few days, but I'll try to read this soon.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

199

Only used once. Inline the variable where it is used.

203

Delete blank line here.

llvm/test/MC/RISCV/fixups-expr-uleb128.s

18 ↗

(On Diff #541010)

For a cl::opt option that defaults to false, we usually don't test the case when we explicitly set it to false. This additional case doesn't pull its weight.

[kito-cheng]

Changes:

• Address @MaskRay's comments.

[MaskRay]

llvm/include/llvm/MC/MCFixup.h

28 ↗

(On Diff #548129)

We do not need this generic fixup.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp

161

These fixup_riscv_{set,add,sub}_* from D103539 are unneeded. I did not put the cleanup into D155357 to minimize the changes in D155357 . I have now removed the unneeded fixup kinds in c8ed138c34ddb22610f18468c1b7938f9e2abae5.

I think you can just use FirstLiteralRelocationKind + ELF::R_RISCV_SET_ULEB128 instead of adding new fixup kinds.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

214

Prefer getContext().reportError for possible error paths. report_fatal_error should be avoided if possible.

llvm/lib/Target/RISCV/MCTargetDesc/RISCVFixupKinds.h

107

Unneeded. See my other comment.

[MaskRay]

I think the right implementation will be quite different from this patch. We need to modify quite a few more places including shouldForceRelocation, MCAssembler::relaxLEB (new function, similar to relaxDwarf*), and MCObjectStreamer::emitULEB128Value.
I am working on an alternative (current draft: https://github.com/MaskRay/llvm-project/tree/mc-uleb128)

[kito-cheng]

@MaskRay Really appropriate your help! it seems like I didn't handle the dwarf stuffs right according your version, and ULEB128 relocation support might be a compatible issue between GNU and LLVM toolchain, so I wondering do you think it's better to split this patch into add relocation number only and add the lld support(D142880) first for LLVM 17 branch?

[MaskRay]

In D142879#4576026, @kito-cheng wrote:

@MaskRay Really appropriate your help! it seems like I didn't handle the dwarf stuffs right according your version, and ULEB128 relocation support might be a compatible issue between GNU and LLVM toolchain, so I wondering do you think it's better to split this patch into add relocation number only and add the lld support(D142880) first for LLVM 17 branch?

Thanks:) Having the ELF_RELOC(R_RISCV_SET_ULEB128, 60) ... 61 change in release/17.x will be useful.

Having the lld change will be good for our compatibility story if we can catch the release schedule. But it seems that the change is quite complex and I am unsure we have sufficient time...

[kito-cheng]

MaskRay has landed a better version :)

llvm/include/llvm/BinaryFormat/ELFRelocs/RISCV.def

 ELF_RELOC(R_RISCV_RELAX,             51)
 ELF_RELOC(R_RISCV_SUB6,              52)
 ELF_RELOC(R_RISCV_SET6,              53)
 ELF_RELOC(R_RISCV_SET8,              54)
 ELF_RELOC(R_RISCV_SET16,             55)
 ELF_RELOC(R_RISCV_SET32,             56)
 ELF_RELOC(R_RISCV_32_PCREL,          57)
 ELF_RELOC(R_RISCV_IRELATIVE,         58)
+ELF_RELOC(R_RISCV_SET_ULEB128,       59)
+ELF_RELOC(R_RISCV_SUB_ULEB128,       60)

llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

const static MCFixupKindInfo Infos[] = {
       {"fixup_riscv_add_32", 0, 32, 0},
       {"fixup_riscv_sub_32", 0, 32, 0},
 
       {"fixup_riscv_add_64", 0, 64, 0},
       {"fixup_riscv_sub_64", 0, 64, 0},
 
       {"fixup_riscv_set_6b", 2, 6, 0},
       {"fixup_riscv_sub_6b", 2, 6, 0},
+
+      {"fixup_riscv_set_uleb128", 0, 0, 0},
+      {"fixup_riscv_sub_uleb128", 0, 0, 0},
   };
   static_assert((std::size(Infos)) == RISCV::NumTargetFixupKinds,
                 "Not all fixup kinds added to Infos array");
 
   // Fixup kinds from .reloc directive are like R_RISCV_NONE. They
   // do not require any extra processing.
   if (Kind >= FirstLiteralRelocationKind)
     return MCAsmBackend::getFixupKindInfo(FK_NONE);
static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
   case RISCV::fixup_riscv_set_16:
   case RISCV::fixup_riscv_add_16:
   case RISCV::fixup_riscv_sub_16:
   case RISCV::fixup_riscv_set_32:
   case RISCV::fixup_riscv_add_32:
   case RISCV::fixup_riscv_sub_32:
   case RISCV::fixup_riscv_add_64:
   case RISCV::fixup_riscv_sub_64:
+  case RISCV::fixup_riscv_set_uleb128:
+  case RISCV::fixup_riscv_sub_uleb128:
   case FK_Data_1:
   case FK_Data_2:
   case FK_Data_4:
   case FK_Data_8:
   case FK_Data_6b:
     return Value;
   case RISCV::fixup_riscv_set_6b:
     return Value & 0x03;

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp

unsigned RISCVELFObjectWriter::getRelocType(MCContext &Ctx,
   case RISCV::fixup_riscv_set_16:
     return ELF::R_RISCV_SET16;
   case RISCV::fixup_riscv_add_16:
     return ELF::R_RISCV_ADD16;
   case RISCV::fixup_riscv_sub_16:
     return ELF::R_RISCV_SUB16;
   case RISCV::fixup_riscv_set_32:
     return ELF::R_RISCV_SET32;
   case RISCV::fixup_riscv_add_32:
     return ELF::R_RISCV_ADD32;
   case RISCV::fixup_riscv_sub_32:
     return ELF::R_RISCV_SUB32;
   case RISCV::fixup_riscv_add_64:
     return ELF::R_RISCV_ADD64;
   case RISCV::fixup_riscv_sub_64:
     return ELF::R_RISCV_SUB64;
+  case RISCV::fixup_riscv_set_uleb128:
+    return ELF::R_RISCV_SET_ULEB128;
+  case RISCV::fixup_riscv_sub_uleb128:
+    return ELF::R_RISCV_SUB_ULEB128;
   }
 }
 
 std::unique_ptr<MCObjectTargetWriter>
 llvm::createRISCVELFObjectWriter(uint8_t OSABI, bool Is64Bit) {
   return std::make_unique<RISCVELFObjectWriter>(OSABI, Is64Bit);
 }

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h

 
 public:
   RISCVELFStreamer(MCContext &C, std::unique_ptr<MCAsmBackend> MAB,
                    std::unique_ptr<MCObjectWriter> MOW,
                    std::unique_ptr<MCCodeEmitter> MCE)
       : MCELFStreamer(C, std::move(MAB), std::move(MOW), std::move(MCE)) {}
 
   void emitValueImpl(const MCExpr *Value, unsigned Size, SMLoc Loc) override;
+  void emitULEB128Value(const MCExpr *Value) override;
 };
 
 namespace llvm {
 
 class RISCVTargetELFStreamer : public RISCVTargetStreamer {
 private:
   enum class AttributeType { Hidden, Numeric, Text, NumericAndText };
 

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCValue.h"
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/RISCVAttributes.h"
 
 using namespace llvm;
 
 // This part is for ELF object output.
 RISCVTargetELFStreamer::RISCVTargetELFStreamer(MCStreamer &S,
                                                const MCSubtargetInfo &STI)
     : RISCVTargetStreamer(S), CurrentVendor("riscv"), STI(STI) {
   MCAssembler &MCA = getStreamer().getAssembler();
   const FeatureBitset &Features = STI.getFeatureBits();
   auto &MAB = static_cast<RISCVAsmBackend &>(MCA.getBackend());
   setTargetABI(RISCVABI::computeTargetABI(STI.getTargetTriple(), Features,
                                           MAB.getTargetOptions().getABIName()));
 }
void RISCVTargetELFStreamer::emitDirectiveVariantCC(MCSymbol &Symbol) {
   getStreamer().getAssembler().registerSymbol(Symbol);
   cast<MCSymbolELF>(Symbol).setOther(ELF::STO_RISCV_VARIANT_CC);
 }
 
 std::pair<unsigned, unsigned>
 RISCVELFStreamer::getRelocPairForSize(unsigned Size) {
   switch (Size) {
   default:
     llvm_unreachable("unsupported fixup size");
   case 1:
     return std::make_pair(RISCV::fixup_riscv_add_8, RISCV::fixup_riscv_sub_8);
   case 2:
     return std::make_pair(RISCV::fixup_riscv_add_16, RISCV::fixup_riscv_sub_16);
   case 4:
     return std::make_pair(RISCV::fixup_riscv_add_32, RISCV::fixup_riscv_sub_32);
   case 8:
     return std::make_pair(RISCV::fixup_riscv_add_64, RISCV::fixup_riscv_sub_64);
   }
 }
 
 bool RISCVELFStreamer::requiresFixups(MCContext &C, const MCExpr *Value,
                                       const MCExpr *&LHS, const MCExpr *&RHS) {
   const auto *MBE = dyn_cast<MCBinaryExpr>(Value);
   if (MBE == nullptr)
     return false;
 
   MCValue E;
   if (!Value->evaluateAsRelocatable(E, nullptr, nullptr))
     return false;
   if (E.getSymA() == nullptr || E.getSymB() == nullptr)
     return false;
 
void RISCVELFStreamer::emitValueImpl(const MCExpr *Value, unsigned Size,
   DF->getFixups().push_back(MCFixup::create(
       DF->getContents().size(), A, static_cast<MCFixupKind>(Add), Loc));
   DF->getFixups().push_back(MCFixup::create(
       DF->getContents().size(), B, static_cast<MCFixupKind>(Sub), Loc));
 
   DF->getContents().resize(DF->getContents().size() + Size, 0);
 }
 
+void RISCVELFStreamer::emitULEB128Value(const MCExpr *Value) {
+  const MCExpr *A, *B;
+
+  if (!requiresFixups(getContext(), Value, A, B))
+    return MCELFStreamer::emitULEB128Value(Value);
+
+  SMLoc Loc = Value->getLoc();
+
+  int64_t IntValue;
+  if (!Value->evaluateAsAbsolute(IntValue, getAssemblerPtr())) {
+    // Emit a placeholder to make sure we'll emit enough space could be filled
+    // by linker later.
+    IntValue = -1ll;
+  }
+  MCDataFragment *DF = getOrCreateDataFragment();
+  flushPendingLabels(DF, DF->getContents().size());
+  MCDwarfLineEntry::make(this, getCurrentSectionOnly());
+
+  DF->getFixups().push_back(MCFixup::create(
+      DF->getContents().size(), A,
+      static_cast<MCFixupKind>(RISCV::fixup_riscv_set_uleb128), Loc));
+  DF->getFixups().push_back(MCFixup::create(
+      DF->getContents().size(), B,
+      static_cast<MCFixupKind>(RISCV::fixup_riscv_sub_uleb128), Loc));
+
+  SmallString<128> Tmp;
+  raw_svector_ostream OSE(Tmp);
+  encodeULEB128(IntValue, OSE);
+  emitBytes(OSE.str());
+}
+
 namespace llvm {
 MCELFStreamer *createRISCVELFStreamer(MCContext &C,
                                       std::unique_ptr<MCAsmBackend> MAB,
                                       std::unique_ptr<MCObjectWriter> MOW,
                                       std::unique_ptr<MCCodeEmitter> MCE,
                                       bool RelaxAll) {
   RISCVELFStreamer *S =
       new RISCVELFStreamer(C, std::move(MAB), std::move(MOW), std::move(MCE));
   S->getAssembler().setRelaxAll(RelaxAll);
   return S;
 }
 } // namespace llvm

llvm/lib/Target/RISCV/MCTargetDesc/RISCVFixupKinds.h

enum Fixups {
   // paired relocations.
   fixup_riscv_sub_64,
   // 6-bit fixup corresponding to R_RISCV_SET6 for local label assignment in
   // DWARF CFA.
   fixup_riscv_set_6b,
   // 6-bit fixup corresponding to R_RISCV_SUB6 for local label assignment in
   // DWARF CFA.
   fixup_riscv_sub_6b,
+  // fixup corresponding to R_RISCV_SET_ULEB128 for local label assignment.
+  fixup_riscv_set_uleb128,
+  // fixup corresponding to R_RISCV_SUB_ULEB128 for local label assignment.
+  fixup_riscv_sub_uleb128,
 
   // Used as a sentinel, must be the last
   fixup_riscv_invalid,
   NumTargetFixupKinds = fixup_riscv_invalid - FirstTargetFixupKind
 };
 } // end namespace llvm::RISCV
 
 #endif

llvm/test/MC/RISCV/fixups-expr.s

 .dword .L2-.L1
 .dword G2-G1
 .word .L2-.L1
 .word G2-G1
 .half .L2-.L1
 .half G2-G1
 .byte .L2-.L1
 .byte G2-G1
+.uleb128 .L2-.L1
+.uleb128 G2-G1
 # RELAX: 0x0 R_RISCV_ADD64 .L2 0x0
 # RELAX: 0x0 R_RISCV_SUB64 .L1 0x0
 # RELAX: 0x8 R_RISCV_ADD64 G2 0x0
 # RELAX: 0x8 R_RISCV_SUB64 G1 0x0
 # RELAX: 0x10 R_RISCV_ADD32 .L2 0x0
 # RELAX: 0x10 R_RISCV_SUB32 .L1 0x0
 # RELAX: 0x14 R_RISCV_ADD32 G2 0x0
 # RELAX: 0x14 R_RISCV_SUB32 G1 0x0
 # RELAX: 0x18 R_RISCV_ADD16 .L2 0x0
 # RELAX: 0x18 R_RISCV_SUB16 .L1 0x0
 # RELAX: 0x1A R_RISCV_ADD16 G2 0x0
 # RELAX: 0x1A R_RISCV_SUB16 G1 0x0
 # RELAX: 0x1C R_RISCV_ADD8 .L2 0x0
 # RELAX: 0x1C R_RISCV_SUB8 .L1 0x0
 # RELAX: 0x1D R_RISCV_ADD8 G2 0x0
 # RELAX: 0x1D R_RISCV_SUB8 G1 0x0
+# RELAX: 0x1E R_RISCV_SET_ULEB128 .L2 0x0
+# RELAX: 0x1E R_RISCV_SUB_ULEB128 .L1 0x0
+# RELAX: 0x1F R_RISCV_SET_ULEB128 G2 0x0
+# RELAX: 0x1F R_RISCV_SUB_ULEB128 G1 0x0