[MC] Support labels as offsets in .reloc directive

[vstefanovic]

Summary

Currently, expressions like

.reloc 1f, R_MIPS_JALR, foo

1: nop

are not allowed, ie. an offset in .reloc can only be absolute value.
This patch adds support for labels as offsets.
If offset is a forward declared label, MCObjectStreamer keeps the fixup locally
and adds it to the fixups vector after the label (and its offset) is defined.
label+number is not supported yet.

[atanasyan]

With this patch I get the following failed test cases.

Clang :: CodeGen/asm-parser-info.S
Clang :: CodeGen/mips-inline-asm-abi.c
Clang :: CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
Clang :: CodeGen/thinlto-distributed-cfi-devirt.ll
Clang :: CodeGen/thinlto-distributed-cfi.ll
Clang :: CodeGen/thinlto-distributed.ll
Clang :: CodeGen/thinlto-multi-module.ll
Clang :: CodeGen/thinlto-split-dwarf.c
Clang :: CodeGen/thinlto_backend.ll
Clang :: Driver/clang-offload-bundler.c
Clang :: Driver/mips-mabs-warning.c
Clang :: Driver/relax.s
Clang :: Misc/cc1as-split-dwarf.s
Clang :: Modules/DebugInfoTransitiveImport.m
Clang :: Modules/ExtDebugInfo.cpp
Clang :: Modules/ModuleDebugInfo.cpp
Clang :: Modules/ModuleDebugInfo.m
Clang :: Modules/debug-info-moduleimport-in-module.m
Clang :: Modules/lsv-debuginfo.cpp
Clang :: Modules/module_file_info.m
Clang :: Modules/pch_container.m
Clang :: PCH/debug-info-pch-path.c

[vstefanovic]

Sorry; thanks for noticing.

[vstefanovic]

Init PendingFixup.Sym to nullptr.

[vstefanovic]

While we're at it, supported multiple pending fixups.

[atanasyan]

LGTM, nit below.

Please delay the commit a bit to get a chance other reviewers to take a look at the patch.

include/llvm/MC/MCObjectStreamer.h

43

The class can be replaced by struct.

47

• Why don't use c++ style class fields initialization?
• MCSym but McFixup. I think it's better to name either MCSym / MCFixup or McSym / McFixup.

lib/MC/MCObjectStreamer.cpp

658

That can be written a bit more briefly:

PendingFixups.emplace_back(&SRE.getSymbol(), DF,
                           MCFixup::create(-1, Expr, Kind, Loc));

[atanasyan]

lib/MC/MCObjectStreamer.cpp

80

One more nit: clear PendingFixups here like we clear PendingLabels in the flushPendingLabels.

[echristo]

It'd be really great if we could handle all of this in the mips backend rather than in general code.

-eric

[vstefanovic]

.reloc directive (and setting its offset as label) isn't mips-only feature, GNU as supports it for all the architectures (or at least for x86, arm, ppc).
But if we don't want this in llvm for other arches than mips, we should move it to mips backend.

[vstefanovic]

Addressed comments.

[vstefanovic]

I assume that we do want to keep this feature for all the architectures, if anyone disagrees, please shout.

[vstefanovic]

Re-uploaded the patch with more context.

include/llvm/MC/MCObjectStreamer.h

 /// to that file format or custom semantics expected by the object writer
 /// implementation.
 class MCObjectStreamer : public MCStreamer {
   std::unique_ptr<MCAssembler> Assembler;
   MCSection::iterator CurInsertionPoint;
   bool EmitEHFrame;
   bool EmitDebugFrame;
   SmallVector<MCSymbol *, 2> PendingLabels;
+  class PendingMCFixup {
+  public:
+    const MCSymbol *Sym;
+    MCFixup Fixup;
+    MCDataFragment *DF;
+    PendingMCFixup(const MCSymbol *MCSym, MCDataFragment *F, MCFixup McFixup) {
+      Sym = MCSym;
+      Fixup = McFixup;
+      DF = F;
+    }
+  };
+  SmallVector<PendingMCFixup, 2> PendingFixups;
 
   virtual void EmitInstToData(const MCInst &Inst, const MCSubtargetInfo&) = 0;
   void EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame) override;
   void EmitCFIEndProcImpl(MCDwarfFrameInfo &Frame) override;
   MCSymbol *EmitCFILabel() override;
   void EmitInstructionImpl(const MCInst &Inst, const MCSubtargetInfo &STI);
+  void resolvePendingFixups();
 
 protected:
   MCObjectStreamer(MCContext &Context, std::unique_ptr<MCAsmBackend> TAB,
                    std::unique_ptr<MCObjectWriter> OW,
                    std::unique_ptr<MCCodeEmitter> Emitter);
   ~MCObjectStreamer();
 
 public:

lib/MC/MCObjectStreamer.cpp

void MCObjectStreamer::flushPendingLabels(MCFragment *F, uint64_t FOffset) {
   }
   for (MCSymbol *Sym : PendingLabels) {
     Sym->setFragment(F);
     Sym->setOffset(FOffset);
   }
   PendingLabels.clear();
 }
 
+// When fixup's offset is a forward declared label, e.g.:
+//
+//   .reloc 1f, R_MIPS_JALR, foo
+// 1: nop
+//
+// postpone adding it to Fixups vector until the label is defined and its offset
+// is known.
+void MCObjectStreamer::resolvePendingFixups() {
+  for (PendingMCFixup &PendingFixup : PendingFixups) {
+    if (!PendingFixup.Sym || PendingFixup.Sym->isUndefined ()) {
+      getContext().reportError(PendingFixup.Fixup.getLoc(),
+                               "unresolved relocation offset");
+      continue;
+    }
+    flushPendingLabels(PendingFixup.DF, PendingFixup.DF->getContents().size());
+    PendingFixup.Fixup.setOffset(PendingFixup.Sym->getOffset());
+    PendingFixup.DF->getFixups().push_back(PendingFixup.Fixup);
+  }
+}
+
 // As a compile-time optimization, avoid allocating and evaluating an MCExpr
 // tree for (Hi - Lo) when Hi and Lo are offsets into the same fragment.
 static Optional<uint64_t>
 absoluteSymbolDiff(MCAssembler &Asm, const MCSymbol *Hi, const MCSymbol *Lo) {
   assert(Hi && Lo);
   if (Asm.getBackendPtr()->requiresDiffExpressionRelocations())
     return None;
 
void MCObjectStreamer::EmitGPRel64Value(const MCExpr *Value) {
   DF->getFixups().push_back(
       MCFixup::create(DF->getContents().size(), Value, FK_GPRel_4));
   DF->getContents().resize(DF->getContents().size() + 8, 0);
 }
 
 bool MCObjectStreamer::EmitRelocDirective(const MCExpr &Offset, StringRef Name,
                                           const MCExpr *Expr, SMLoc Loc,
                                           const MCSubtargetInfo &STI) {
-  int64_t OffsetValue;
-  if (!Offset.evaluateAsAbsolute(OffsetValue))
-    llvm_unreachable("Offset is not absolute");
-
-  if (OffsetValue < 0)
-    llvm_unreachable("Offset is negative");
-
-  MCDataFragment *DF = getOrCreateDataFragment(&STI);
-  flushPendingLabels(DF, DF->getContents().size());
-
   Optional<MCFixupKind> MaybeKind = Assembler->getBackend().getFixupKind(Name);
   if (!MaybeKind.hasValue())
     return true;
 
   MCFixupKind Kind = *MaybeKind;
 
   if (Expr == nullptr)
     Expr =
         MCSymbolRefExpr::create(getContext().createTempSymbol(), getContext());
+
+  MCDataFragment *DF = getOrCreateDataFragment(&STI);
+  flushPendingLabels(DF, DF->getContents().size());
+
+  int64_t OffsetValue;
+  if (Offset.evaluateAsAbsolute(OffsetValue)) {
+    if (OffsetValue < 0)
+      llvm_unreachable(".reloc offset is negative");
     DF->getFixups().push_back(MCFixup::create(OffsetValue, Expr, Kind, Loc));
     return false;
   }
 
+  if (Offset.getKind() != llvm::MCExpr::SymbolRef)
+    llvm_unreachable(".reloc offset is not absolute nor a label");
+
+  const MCSymbolRefExpr &SRE = cast<MCSymbolRefExpr>(Offset);
+  if (SRE.getSymbol().isDefined()) {
+    DF->getFixups().push_back(MCFixup::create(SRE.getSymbol().getOffset(),
+                                              Expr, Kind, Loc));
+    return false;
+  }
+
+  PendingFixups.push_back(PendingMCFixup(&SRE.getSymbol(), DF,
+                                         MCFixup::create(-1, Expr, Kind, Loc)));
+  return false;
+}
+
 void MCObjectStreamer::emitFill(const MCExpr &NumBytes, uint64_t FillValue,
                                 SMLoc Loc) {
   MCDataFragment *DF = getOrCreateDataFragment();
   flushPendingLabels(DF, DF->getContents().size());
 
   assert(getCurrentSectionOnly() && "need a section");
   insert(new MCFillFragment(FillValue, 1, NumBytes, Loc));
 }
void MCObjectStreamer::FinishImpl() {
   // If we are generating dwarf for assembly source files dump out the sections.
   if (getContext().getGenDwarfForAssembly())
     MCGenDwarfInfo::Emit(this);
 
   // Dump out the dwarf file & directory tables and line tables.
   MCDwarfLineTable::Emit(this, getAssembler().getDWARFLinetableParams());
 
   flushPendingLabels();
+  resolvePendingFixups();
   getAssembler().Finish();
 }

lib/MC/MCParser/AsmParser.cpp

bool AsmParser::parseDirectiveAscii(StringRef IDVal, bool ZeroTerminated) {
   return false;
 }
 
 /// parseDirectiveReloc
 ///  ::= .reloc expression , identifier [ , expression ]
 bool AsmParser::parseDirectiveReloc(SMLoc DirectiveLoc) {
   const MCExpr *Offset;
   const MCExpr *Expr = nullptr;
-
-  SMLoc OffsetLoc = Lexer.getTok().getLoc();
   int64_t OffsetValue;
-  // We can only deal with constant expressions at the moment.
+  SMLoc OffsetLoc = Lexer.getTok().getLoc();
 
   if (parseExpression(Offset))
     return true;
 
-  if (check(!Offset->evaluateAsAbsolute(OffsetValue,
-                                        getStreamer().getAssemblerPtr()),
-            OffsetLoc, "expression is not a constant value") ||
-      check(OffsetValue < 0, OffsetLoc, "expression is negative") ||
-      parseToken(AsmToken::Comma, "expected comma") ||
-      check(getTok().isNot(AsmToken::Identifier), "expected relocation name"))
+  if ((Offset->evaluateAsAbsolute(OffsetValue,
+                                  getStreamer().getAssemblerPtr()) &&
+       check(OffsetValue < 0, OffsetLoc, "expression is negative")) ||
+      (check(Offset->getKind() != llvm::MCExpr::Constant &&
+             Offset->getKind() != llvm::MCExpr::SymbolRef,
+             OffsetLoc, "expected non-negative number or a label")) ||
+      (parseToken(AsmToken::Comma, "expected comma") ||
+       check(getTok().isNot(AsmToken::Identifier), "expected relocation name")))
     return true;
 
   SMLoc NameLoc = Lexer.getTok().getLoc();
   StringRef Name = Lexer.getTok().getIdentifier();
   Lex();
 
   if (Lexer.is(AsmToken::Comma)) {
     Lex();

test/MC/Mips/reloc-directive-bad-obj.s

+# RUN: not llvm-mc -triple mips-unknown-linux %s -show-encoding \
+# RUN:     -target-abi=o32 -filetype=obj 2>&1 | FileCheck %s
+  .text
+  nop
+  .reloc foo, R_MIPS_32, .text  # CHECK: :[[@LINE]]:2: error: unresolved relocation offset
+  nop
+  nop
+  .reloc bar, R_MIPS_32, .text  # CHECK: :[[@LINE]]:2: error: unresolved relocation offset
+  nop

test/MC/Mips/reloc-directive-bad.s

-# RUN: not llvm-mc -triple mips-unknown-linux < %s -show-encoding -target-abi=o32 \
-# RUN:     2>&1 | FileCheck %s
+# RUN: not llvm-mc -triple mips-unknown-linux < %s -show-encoding \
+# RUN:     -target-abi=o32 2>&1 | FileCheck %s
   .text
 foo:
-  .reloc 0, R_MIPS_32, .text+.text # CHECK: :[[@LINE]]:23: error: expression must be relocatable
+  .reloc foo+4, R_MIPS_32, .text    # CHECK: :[[@LINE]]:9: error: expected non-negative number or a label
+  .reloc foo+foo, R_MIPS_32, .text  # CHECK: :[[@LINE]]:9: error: expected non-negative number or a label
+  .reloc 0, R_MIPS_32, .text+.text  # CHECK: :[[@LINE]]:23: error: expression must be relocatable
+  .reloc 0 R_MIPS_32, .text         # CHECK: :[[@LINE]]:11: error: expected comma
+  .reloc 0, 0, R_MIPS_32, .text     # CHECK: :[[@LINE]]:12: error: expected relocation name
+  .reloc -1, R_MIPS_32, .text       # CHECK: :[[@LINE]]:9: error: expression is negative
+  .reloc 1b, R_MIPS_32, .text       # CHECK: :[[@LINE]]:9: error: directional label undefined
+  .reloc 1f, R_MIPS_32, .text       # CHECK: :[[@LINE]]:9: error: directional label undefined
   nop

test/MC/Mips/reloc-directive-label-offset.s

+# RUN: llvm-mc -triple mips-unknown-linux %s -show-encoding -target-abi=o32 \
+# RUN:     | FileCheck --check-prefixes=ASM,ASM-32 %s
+# RUN: llvm-mc -triple mips64-unknown-linux %s -show-encoding -target-abi=n32 \
+# RUN:     | FileCheck --check-prefixes=ASM,ASM-64 %s
+# RUN: llvm-mc -triple mips64-unknown-linux %s -show-encoding -target-abi=n64 \
+# RUN:     | FileCheck --check-prefixes=ASM,ASM-64 %s
+# RUN: llvm-mc -triple mips-unknown-linux %s -show-encoding -target-abi=o32 \
+# RUN:     -filetype=obj | llvm-readobj -r | FileCheck -check-prefix=OBJ-O32 %s
+# RUN: llvm-mc -triple mips64-unknown-linux %s -show-encoding -target-abi=n32 \
+# RUN:     -filetype=obj | llvm-readobj -r | FileCheck -check-prefix=OBJ-N32 %s
+# RUN: llvm-mc -triple mips64-unknown-linux %s -show-encoding -target-abi=n64 \
+# RUN:     -filetype=obj | llvm-readobj -r | FileCheck -check-prefix=OBJ-N64 %s
+
+  .text
+foo: # ASM-LABEL: foo:
+  nop
+1:
+  nop
+  .reloc 1b, R_MIPS_NONE, foo       # ASM-32: .reloc ($tmp0), R_MIPS_NONE, foo
+                                    # ASM-64: .reloc .Ltmp0, R_MIPS_NONE, foo
+  nop
+  .reloc 1f, R_MIPS_32, foo         # ASM-32: .reloc ($tmp1), R_MIPS_32, foo
+                                    # ASM-64: .reloc .Ltmp1, R_MIPS_32, foo
+1:
+  nop
+  .reloc 1f, R_MIPS_CALL16, foo     # ASM-32: .reloc ($tmp2), R_MIPS_CALL16, foo
+                                    # ASM-64: .reloc .Ltmp2, R_MIPS_CALL16, foo
+1:
+  nop
+  .reloc 2f, R_MIPS_GOT_DISP, foo   # ASM-32: .reloc ($tmp3), R_MIPS_GOT_DISP, foo
+                                    # ASM-64: .reloc .Ltmp3, R_MIPS_GOT_DISP, foo
+  nop
+
+  .reloc 3f, R_MIPS_GOT_PAGE, foo   # ASM-32: .reloc ($tmp4), R_MIPS_GOT_PAGE, foo
+                                    # ASM-64: .reloc .Ltmp4, R_MIPS_GOT_PAGE, foo
+  nop
+bar:
+2:
+  nop
+3:
+  nop
+  .reloc bar, R_MIPS_GOT_OFST, foo  # ASM: .reloc bar, R_MIPS_GOT_OFST, foo
+  nop
+  .reloc foo, R_MIPS_32, foo        # ASM: .reloc foo, R_MIPS_32, foo
+  nop
+1:
+  nop
+
+# OBJ-O32-LABEL: Relocations [
+# OBJ-O32:           0x0 R_MIPS_32 .text 0x0
+# OBJ-O32-NEXT:      0x4 R_MIPS_NONE .text 0x0
+# OBJ-O32-NEXT:      0xC R_MIPS_32 .text 0x0
+# OBJ-O32-NEXT:      0x10 R_MIPS_CALL16 foo 0x0
+# OBJ-O32-NEXT:      0x1C R_MIPS_GOT_DISP foo 0x0
+# OBJ-O32-NEXT:      0x1C R_MIPS_GOT_OFST .text 0x0
+# OBJ-O32-NEXT:      0x20 R_MIPS_GOT_PAGE .text 0x0
+
+# OBJ-N32-LABEL: Relocations [
+# OBJ-N32:           0x4 R_MIPS_NONE .text 0x0
+# OBJ-N32-NEXT:      0x1C R_MIPS_GOT_OFST .text 0x0
+# OBJ-N32-NEXT:      0x0 R_MIPS_32 .text 0x0
+# OBJ-N32-NEXT:      0xC R_MIPS_32 .text 0x0
+# OBJ-N32-NEXT:      0x10 R_MIPS_CALL16 foo 0x0
+# OBJ-N32-NEXT:      0x1C R_MIPS_GOT_DISP foo 0x0
+# OBJ-N32-NEXT:      0x20 R_MIPS_GOT_PAGE .text 0x0
+
+# OBJ-N64-LABEL: Relocations [
+# OBJ-N64:           0x4 R_MIPS_NONE/R_MIPS_NONE/R_MIPS_NONE .text 0x0
+# OBJ-N64-NEXT:      0x1C R_MIPS_GOT_OFST/R_MIPS_NONE/R_MIPS_NONE .text 0x0
+# OBJ-N64-NEXT:      0x0 R_MIPS_32/R_MIPS_NONE/R_MIPS_NONE .text 0x0
+# OBJ-N64-NEXT:      0xC R_MIPS_32/R_MIPS_NONE/R_MIPS_NONE .text 0x0
+# OBJ-N64-NEXT:      0x10 R_MIPS_CALL16/R_MIPS_NONE/R_MIPS_NONE foo 0x0
+# OBJ-N64-NEXT:      0x1C R_MIPS_GOT_DISP/R_MIPS_NONE/R_MIPS_NONE foo 0x0
+# OBJ-N64-NEXT:      0x20 R_MIPS_GOT_PAGE/R_MIPS_NONE/R_MIPS_NONE .text 0x0

test/MC/Mips/reloc-directive-negative.s

-# RUN: not llvm-mc -triple mips-unknown-linux < %s -show-encoding -target-abi=o32 \
-# RUN:     2>&1 | FileCheck %s
-  .text
-foo:
-  .reloc -1, R_MIPS_32, .text # CHECK: :[[@LINE]]:9: error: expression is negative
-  nop