RISCV: adjust handling of relocation emission for RISCV

This re-architects the RISCV relocation handling to bring the
implementation closer in line with the implementation in binutils.  We
would previously aggressively resolve the relocation.  With this
restructuring, we always will emit a paired relocation for any symbolic
difference of the type of S±T[±C] where S and T are labels and C is a
constant.

GAS has a special target hook controlled by `RELOC_EXPANSION_POSSIBLE`
which indicates that a fixup may be expanded into multiple relocations.
This is used by the RISCV backend to always emit a paired relocation -
either ADD[WIDTH] + SUB[WIDTH] for text relocations or SET[WIDTH] +
SUB[WIDTH] for a debug info relocation.  Irrespective of whether linker
relaxation support is enabled, symbolic difference is always emitted as
a paired relocation.

This change also sinks the target specific behaviour down into the
target specific area rather than exposing it to the shared relocation
handling.  In the process, we also sink the "special" handling for debug
information down into the RISCV target.  Although this improves the path
for the other targets, this is not necessarily entirely ideal either.
The changes in the debug info emission could be done through another
type of hook as this functionality would be required by any other target
which wishes to do linker relaxation.  However, as there are no other
targets in LLVM which currently do this, this is a reasonable thing to
do until such time as the code needs to be shared.

Improve the handling of the relocation (and add a reduced test case from
the Linux kernel) to ensure that we handle complex expressions for
symbolic difference.  This ensures that we correct relocate symbols with
the adddends normalized and associated with the addition portion of the
paired relocation.

This change also addresses some review comments from Alex Bradbury about
the relocations meant for use in the DWARF CFA being named incorrectly
(using ADD6 instead of SET6) in the original change which introduced the
relocation type.

This resolves the issues with the symbolic difference emission
sufficiently to enable building the Linux kernel with clang+IAS+lld
(without linker relaxation).

Resolves PR50153, PR50156!
Fixes: ClangBuiltLinux/linux#1023, ClangBuiltLinux/linux#1143

Reviewed By: nickdesaulniers, maskray

Differential Revision: https://reviews.llvm.org/D103539

Author: compnerd

llvm/include/llvm/MC/MCAsmBackend.h

@@ -129,10 +129,6 @@ class MCAsmBackend {
                           uint64_t Value, bool IsResolved,
                           const MCSubtargetInfo *STI) const = 0;
 
-  /// Check whether the given target requires emitting differences of two
-  /// symbols as a set of relocations.
-  virtual bool requiresDiffExpressionRelocations() const { return false; }
-
   /// @}
 
   /// \name Target Relaxation Interfaces
@@ -169,6 +165,16 @@ class MCAsmBackend {
   virtual void relaxInstruction(MCInst &Inst,
                                 const MCSubtargetInfo &STI) const {};
 
+  virtual bool relaxDwarfLineAddr(MCDwarfLineAddrFragment &DF,
+                                  MCAsmLayout &Layout, bool &WasRelaxed) const {
+    return false;
+  }
+
+  virtual bool relaxDwarfCFA(MCDwarfCallFrameFragment &DF, MCAsmLayout &Layout,
+                             bool &WasRelaxed) const {
+    return false;
+  }
+
   /// @}
 
   /// Returns the minimum size of a nop in bytes on this target. The assembler

llvm/include/llvm/MC/MCDwarf.h

@@ -387,13 +387,6 @@ class MCDwarfLineAddr {
   static void Encode(MCContext &Context, MCDwarfLineTableParams Params,
                      int64_t LineDelta, uint64_t AddrDelta, raw_ostream &OS);
 
-  /// Utility function to encode a Dwarf pair of LineDelta and AddrDeltas using
-  /// fixed length operands. Returns (Offset, Size, SetDelta).
-  static std::tuple<uint32_t, uint32_t, bool> fixedEncode(MCContext &Context,
-                                                          int64_t LineDelta,
-                                                          uint64_t AddrDelta,
-                                                          raw_ostream &OS);
-
   /// Utility function to emit the encoding to a streamer.
   static void Emit(MCStreamer *MCOS, MCDwarfLineTableParams Params,
                    int64_t LineDelta, uint64_t AddrDelta);
@@ -667,8 +660,7 @@ class MCDwarfFrameEmitter {
   static void Emit(MCObjectStreamer &streamer, MCAsmBackend *MAB, bool isEH);
   static void EmitAdvanceLoc(MCObjectStreamer &Streamer, uint64_t AddrDelta);
   static void EncodeAdvanceLoc(MCContext &Context, uint64_t AddrDelta,
-                               raw_ostream &OS, uint32_t *Offset = nullptr,
-                               uint32_t *Size = nullptr);
+                               raw_ostream &OS);
 };
 
 } // end namespace llvm

llvm/include/llvm/MC/MCFixup.h

@@ -41,16 +41,6 @@ enum MCFixupKind {
   FK_SecRel_2,    ///< A two-byte section relative fixup.
   FK_SecRel_4,    ///< A four-byte section relative fixup.
   FK_SecRel_8,    ///< A eight-byte section relative fixup.
-  FK_Data_Add_1,  ///< A one-byte add fixup.
-  FK_Data_Add_2,  ///< A two-byte add fixup.
-  FK_Data_Add_4,  ///< A four-byte add fixup.
-  FK_Data_Add_8,  ///< A eight-byte add fixup.
-  FK_Data_Add_6b, ///< A six-bits add fixup.
-  FK_Data_Sub_1,  ///< A one-byte sub fixup.
-  FK_Data_Sub_2,  ///< A two-byte sub fixup.
-  FK_Data_Sub_4,  ///< A four-byte sub fixup.
-  FK_Data_Sub_8,  ///< A eight-byte sub fixup.
-  FK_Data_Sub_6b, ///< A six-bits sub fixup.
 
   FirstTargetFixupKind = 128,
 
@@ -105,28 +95,6 @@ class MCFixup {
     return FI;
   }
 
-  /// Return a fixup corresponding to the add half of a add/sub fixup pair for
-  /// the given Fixup.
-  static MCFixup createAddFor(const MCFixup &Fixup) {
-    MCFixup FI;
-    FI.Value = Fixup.getValue();
-    FI.Offset = Fixup.getOffset();
-    FI.Kind = getAddKindForKind(Fixup.getKind());
-    FI.Loc = Fixup.getLoc();
-    return FI;
-  }
-
-  /// Return a fixup corresponding to the sub half of a add/sub fixup pair for
-  /// the given Fixup.
-  static MCFixup createSubFor(const MCFixup &Fixup) {
-    MCFixup FI;
-    FI.Value = Fixup.getValue();
-    FI.Offset = Fixup.getOffset();
-    FI.Kind = getSubKindForKind(Fixup.getKind());
-    FI.Loc = Fixup.getLoc();
-    return FI;
-  }
-
   MCFixupKind getKind() const { return Kind; }
 
   unsigned getTargetKind() const { return Kind; }
@@ -172,32 +140,6 @@ class MCFixup {
     }
   }
 
-  /// Return the generic fixup kind for an addition with a given size. It
-  /// is an error to pass an unsupported size.
-  static MCFixupKind getAddKindForKind(MCFixupKind Kind) {
-    switch (Kind) {
-    default: llvm_unreachable("Unknown type to convert!");
-    case FK_Data_1: return FK_Data_Add_1;
-    case FK_Data_2: return FK_Data_Add_2;
-    case FK_Data_4: return FK_Data_Add_4;
-    case FK_Data_8: return FK_Data_Add_8;
-    case FK_Data_6b: return FK_Data_Add_6b;
-    }
-  }
-
-  /// Return the generic fixup kind for an subtraction with a given size. It
-  /// is an error to pass an unsupported size.
-  static MCFixupKind getSubKindForKind(MCFixupKind Kind) {
-    switch (Kind) {
-    default: llvm_unreachable("Unknown type to convert!");
-    case FK_Data_1: return FK_Data_Sub_1;
-    case FK_Data_2: return FK_Data_Sub_2;
-    case FK_Data_4: return FK_Data_Sub_4;
-    case FK_Data_8: return FK_Data_Sub_8;
-    case FK_Data_6b: return FK_Data_Sub_6b;
-    }
-  }
-
   SMLoc getLoc() const { return Loc; }
 };
 

llvm/lib/MC/MCAsmBackend.cpp

@@ -95,16 +95,7 @@ const MCFixupKindInfo &MCAsmBackend::getFixupKindInfo(MCFixupKind Kind) const {
       {"FK_SecRel_2", 0, 16, 0},
       {"FK_SecRel_4", 0, 32, 0},
       {"FK_SecRel_8", 0, 64, 0},
-      {"FK_Data_Add_1", 0, 8, 0},
-      {"FK_Data_Add_2", 0, 16, 0},
-      {"FK_Data_Add_4", 0, 32, 0},
-      {"FK_Data_Add_8", 0, 64, 0},
-      {"FK_Data_Add_6b", 0, 6, 0},
-      {"FK_Data_Sub_1", 0, 8, 0},
-      {"FK_Data_Sub_2", 0, 16, 0},
-      {"FK_Data_Sub_4", 0, 32, 0},
-      {"FK_Data_Sub_8", 0, 64, 0},
-      {"FK_Data_Sub_6b", 0, 6, 0}};
+  };
 
   assert((size_t)Kind <= array_lengthof(Builtins) && "Unknown fixup kind");
   return Builtins[Kind];

llvm/lib/MC/MCAssembler.cpp

@@ -792,26 +792,7 @@ MCAssembler::handleFixup(const MCAsmLayout &Layout, MCFragment &F,
     // The fixup was unresolved, we need a relocation. Inform the object
     // writer of the relocation, and give it an opportunity to adjust the
     // fixup value if need be.
-    if (Target.getSymA() && Target.getSymB() &&
-        getBackend().requiresDiffExpressionRelocations()) {
-      // The fixup represents the difference between two symbols, which the
-      // backend has indicated must be resolved at link time. Split up the fixup
-      // into two relocations, one for the add, and one for the sub, and emit
-      // both of these. The constant will be associated with the add half of the
-      // expression.
-      MCFixup FixupAdd = MCFixup::createAddFor(Fixup);
-      MCValue TargetAdd =
-          MCValue::get(Target.getSymA(), nullptr, Target.getConstant());
-      getWriter().recordRelocation(*this, Layout, &F, FixupAdd, TargetAdd,
-                                   FixedValue);
-      MCFixup FixupSub = MCFixup::createSubFor(Fixup);
-      MCValue TargetSub = MCValue::get(Target.getSymB());
-      getWriter().recordRelocation(*this, Layout, &F, FixupSub, TargetSub,
-                                   FixedValue);
-    } else {
-      getWriter().recordRelocation(*this, Layout, &F, Fixup, Target,
-                                   FixedValue);
-    }
+    getWriter().recordRelocation(*this, Layout, &F, Fixup, Target, FixedValue);
   }
   return std::make_tuple(Target, FixedValue, IsResolved);
 }
@@ -1100,6 +1081,11 @@ bool MCAssembler::relaxBoundaryAlign(MCAsmLayout &Layout,
 
 bool MCAssembler::relaxDwarfLineAddr(MCAsmLayout &Layout,
                                      MCDwarfLineAddrFragment &DF) {
+
+  bool WasRelaxed;
+  if (getBackend().relaxDwarfLineAddr(DF, Layout, WasRelaxed))
+    return WasRelaxed;
+
   MCContext &Context = Layout.getAssembler().getContext();
   uint64_t OldSize = DF.getContents().size();
   int64_t AddrDelta;
@@ -1113,33 +1099,17 @@ bool MCAssembler::relaxDwarfLineAddr(MCAsmLayout &Layout,
   raw_svector_ostream OSE(Data);
   DF.getFixups().clear();
 
-  if (!getBackend().requiresDiffExpressionRelocations()) {
-    MCDwarfLineAddr::Encode(Context, getDWARFLinetableParams(), LineDelta,
-                            AddrDelta, OSE);
-  } else {
-    uint32_t Offset;
-    uint32_t Size;
-    bool SetDelta;
-    std::tie(Offset, Size, SetDelta) =
-        MCDwarfLineAddr::fixedEncode(Context, LineDelta, AddrDelta, OSE);
-    // Add Fixups for address delta or new address.
-    const MCExpr *FixupExpr;
-    if (SetDelta) {
-      FixupExpr = &DF.getAddrDelta();
-    } else {
-      const MCBinaryExpr *ABE = cast<MCBinaryExpr>(&DF.getAddrDelta());
-      FixupExpr = ABE->getLHS();
-    }
-    DF.getFixups().push_back(
-        MCFixup::create(Offset, FixupExpr,
-                        MCFixup::getKindForSize(Size, false /*isPCRel*/)));
-  }
-
+  MCDwarfLineAddr::Encode(Context, getDWARFLinetableParams(), LineDelta,
+                          AddrDelta, OSE);
   return OldSize != Data.size();
 }
 
 bool MCAssembler::relaxDwarfCallFrameFragment(MCAsmLayout &Layout,
                                               MCDwarfCallFrameFragment &DF) {
+  bool WasRelaxed;
+  if (getBackend().relaxDwarfCFA(DF, Layout, WasRelaxed))
+    return WasRelaxed;
+
   MCContext &Context = Layout.getAssembler().getContext();
   uint64_t OldSize = DF.getContents().size();
   int64_t AddrDelta;
@@ -1151,20 +1121,7 @@ bool MCAssembler::relaxDwarfCallFrameFragment(MCAsmLayout &Layout,
   raw_svector_ostream OSE(Data);
   DF.getFixups().clear();
 
-  if (getBackend().requiresDiffExpressionRelocations()) {
-    uint32_t Offset;
-    uint32_t Size;
-    MCDwarfFrameEmitter::EncodeAdvanceLoc(Context, AddrDelta, OSE, &Offset,
-                                          &Size);
-    if (Size) {
-      DF.getFixups().push_back(MCFixup::create(
-          Offset, &DF.getAddrDelta(),
-          MCFixup::getKindForSizeInBits(Size /*In bits.*/, false /*isPCRel*/)));
-    }
-  } else {
-    MCDwarfFrameEmitter::EncodeAdvanceLoc(Context, AddrDelta, OSE);
-  }
-
+  MCDwarfFrameEmitter::EncodeAdvanceLoc(Context, AddrDelta, OSE);
   return OldSize != Data.size();
 }
 
@@ -1194,10 +1151,6 @@ bool MCAssembler::relaxPseudoProbeAddr(MCAsmLayout &Layout,
   raw_svector_ostream OSE(Data);
   PF.getFixups().clear();
 
-  // Relocations should not be needed in general except on RISC-V which we are
-  // not targeted for now.
-  assert(!getBackend().requiresDiffExpressionRelocations() &&
-         "cannot relax relocations");
   // AddrDelta is a signed integer
   encodeSLEB128(AddrDelta, OSE, OldSize);
   return OldSize != Data.size();

llvm/lib/MC/MCDwarf.cpp

@@ -734,54 +734,6 @@ void MCDwarfLineAddr::Encode(MCContext &Context, MCDwarfLineTableParams Params,
   }
 }
 
-std::tuple<uint32_t, uint32_t, bool>
-MCDwarfLineAddr::fixedEncode(MCContext &Context, int64_t LineDelta,
-                             uint64_t AddrDelta, raw_ostream &OS) {
-  uint32_t Offset, Size;
-  if (LineDelta != INT64_MAX) {
-    OS << char(dwarf::DW_LNS_advance_line);
-    encodeSLEB128(LineDelta, OS);
-  }
-
-  // Use address delta to adjust address or use absolute address to adjust
-  // address.
-  bool SetDelta;
-  // According to DWARF spec., the DW_LNS_fixed_advance_pc opcode takes a
-  // single uhalf (unencoded) operand. So, the maximum value of AddrDelta
-  // is 65535. We set a conservative upper bound for it for relaxation.
-  if (AddrDelta > 60000) {
-    const MCAsmInfo *asmInfo = Context.getAsmInfo();
-    unsigned AddrSize = asmInfo->getCodePointerSize();
-
-    OS << char(dwarf::DW_LNS_extended_op);
-    encodeULEB128(1 + AddrSize, OS);
-    OS << char(dwarf::DW_LNE_set_address);
-    // Generate fixup for the address.
-    Offset = OS.tell();
-    Size = AddrSize;
-    SetDelta = false;
-    OS.write_zeros(AddrSize);
-  } else {
-    OS << char(dwarf::DW_LNS_fixed_advance_pc);
-    // Generate fixup for 2-bytes address delta.
-    Offset = OS.tell();
-    Size = 2;
-    SetDelta = true;
-    OS << char(0);
-    OS << char(0);
-  }
-
-  if (LineDelta == INT64_MAX) {
-    OS << char(dwarf::DW_LNS_extended_op);
-    OS << char(1);
-    OS << char(dwarf::DW_LNE_end_sequence);
-  } else {
-    OS << char(dwarf::DW_LNS_copy);
-  }
-
-  return std::make_tuple(Offset, Size, SetDelta);
-}
-
 // Utility function to write a tuple for .debug_abbrev.
 static void EmitAbbrev(MCStreamer *MCOS, uint64_t Name, uint64_t Form) {
   MCOS->emitULEB128IntValue(Name);
@@ -1940,54 +1892,28 @@ void MCDwarfFrameEmitter::EmitAdvanceLoc(MCObjectStreamer &Streamer,
 }
 
 void MCDwarfFrameEmitter::EncodeAdvanceLoc(MCContext &Context,
-                                           uint64_t AddrDelta, raw_ostream &OS,
-                                           uint32_t *Offset, uint32_t *Size) {
+                                           uint64_t AddrDelta,
+                                           raw_ostream &OS) {
   // Scale the address delta by the minimum instruction length.
   AddrDelta = ScaleAddrDelta(Context, AddrDelta);
-
-  bool WithFixups = false;
-  if (Offset && Size)
-    WithFixups = true;
+  if (AddrDelta == 0)
+    return;
 
   support::endianness E =
       Context.getAsmInfo()->isLittleEndian() ? support::little : support::big;
-  if (AddrDelta == 0) {
-    if (WithFixups) {
-      *Offset = 0;
-      *Size = 0;
-    }
-  } else if (isUIntN(6, AddrDelta)) {
+
+  if (isUIntN(6, AddrDelta)) {
     uint8_t Opcode = dwarf::DW_CFA_advance_loc | AddrDelta;
-    if (WithFixups) {
-      *Offset = OS.tell();
-      *Size = 6;
-      OS << uint8_t(dwarf::DW_CFA_advance_loc);
-    } else
-      OS << Opcode;
+    OS << Opcode;
   } else if (isUInt<8>(AddrDelta)) {
     OS << uint8_t(dwarf::DW_CFA_advance_loc1);
-    if (WithFixups) {
-      *Offset = OS.tell();
-      *Size = 8;
-      OS.write_zeros(1);
-    } else
-      OS << uint8_t(AddrDelta);
+    OS << uint8_t(AddrDelta);
   } else if (isUInt<16>(AddrDelta)) {
     OS << uint8_t(dwarf::DW_CFA_advance_loc2);
-    if (WithFixups) {
-      *Offset = OS.tell();
-      *Size = 16;
-      OS.write_zeros(2);
-    } else
-      support::endian::write<uint16_t>(OS, AddrDelta, E);
+    support::endian::write<uint16_t>(OS, AddrDelta, E);
   } else {
     assert(isUInt<32>(AddrDelta));
     OS << uint8_t(dwarf::DW_CFA_advance_loc4);
-    if (WithFixups) {
-      *Offset = OS.tell();
-      *Size = 32;
-      OS.write_zeros(4);
-    } else
-      support::endian::write<uint32_t>(OS, AddrDelta, E);
+    support::endian::write<uint32_t>(OS, AddrDelta, E);
   }
 }