MCObjectStreamer: assign MCSymbols in the dummy fragment to offset 0.

In MCObjectStreamer, when there is no current fragment, initially symbols are created in a "pending" state and assigned to a dummy empty fragment. Previously, they were not being assigned an offset, and thus evaluateAbsolute would fail if trying to evaluate an expression 'a - b', where both 'a' and 'b' were in this pending state. Also slightly refactored the EmitLabel overload which takes an MCFragment for clarity. Fixes: https://llvm.org/PR41825 Differential Revision: https://reviews.llvm.org/D70062
llvm · Nov 16, 2019 · bf142fc · bf142fc
1 parent 5987cc1
commit bf142fc
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 20 deletions.
diff --git a/llvm/include/llvm/MC/MCELFStreamer.h b/llvm/include/llvm/MC/MCELFStreamer.h
@@ -41,7 +41,8 @@ class MCELFStreamer : public MCObjectStreamer {
   void InitSections(bool NoExecStack) override;
   void ChangeSection(MCSection *Section, const MCExpr *Subsection) override;
   void EmitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
-  void EmitLabel(MCSymbol *Symbol, SMLoc Loc, MCFragment *F) override;
+  void EmitLabelAtPos(MCSymbol *Symbol, SMLoc Loc, MCFragment *F,
+                      uint64_t Offset) override;
   void EmitAssemblerFlag(MCAssemblerFlag Flag) override;
   void EmitThumbFunc(MCSymbol *Func) override;
   void EmitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override;

diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -107,7 +107,8 @@ class MCObjectStreamer : public MCStreamer {
   /// @{
 
   void EmitLabel(MCSymbol *Symbol, SMLoc Loc = SMLoc()) override;
-  virtual void EmitLabel(MCSymbol *Symbol, SMLoc Loc, MCFragment *F);
+  virtual void EmitLabelAtPos(MCSymbol *Symbol, SMLoc Loc, MCFragment *F,
+                              uint64_t Offset);
   void EmitAssignment(MCSymbol *Symbol, const MCExpr *Value) override;
   void EmitValueImpl(const MCExpr *Value, unsigned Size,
                      SMLoc Loc = SMLoc()) override;

diff --git a/llvm/lib/MC/MCELFStreamer.cpp b/llvm/lib/MC/MCELFStreamer.cpp
@@ -106,9 +106,10 @@ void MCELFStreamer::EmitLabel(MCSymbol *S, SMLoc Loc) {
     Symbol->setType(ELF::STT_TLS);
 }
 
-void MCELFStreamer::EmitLabel(MCSymbol *S, SMLoc Loc, MCFragment *F) {
+void MCELFStreamer::EmitLabelAtPos(MCSymbol *S, SMLoc Loc, MCFragment *F,
+                                   uint64_t Offset) {
   auto *Symbol = cast<MCSymbolELF>(S);
-  MCObjectStreamer::EmitLabel(Symbol, Loc, F);
+  MCObjectStreamer::EmitLabelAtPos(Symbol, Loc, F, Offset);
 
   const MCSectionELF &Section =
       static_cast<const MCSectionELF &>(*getCurrentSectionOnly());

diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -243,18 +243,32 @@ void MCObjectStreamer::EmitLabel(MCSymbol *Symbol, SMLoc Loc) {
     Symbol->setFragment(F);
     Symbol->setOffset(F->getContents().size());
   } else {
+    // Assign all pending labels to offset 0 within the dummy "pending"
+    // fragment. (They will all be reassigned to a real fragment in
+    // flushPendingLabels())
+    Symbol->setOffset(0);
     PendingLabels.push_back(Symbol);
   }
 }
 
-void MCObjectStreamer::EmitLabel(MCSymbol *Symbol, SMLoc Loc, MCFragment *F) {
+// Emit a label at a previously emitted fragment/offset position. This must be
+// within the currently-active section.
+void MCObjectStreamer::EmitLabelAtPos(MCSymbol *Symbol, SMLoc Loc,
+                                      MCFragment *F, uint64_t Offset) {
+  assert(F->getParent() == getCurrentSectionOnly());
+
   MCStreamer::EmitLabel(Symbol, Loc);
   getAssembler().registerSymbol(*Symbol);
   auto *DF = dyn_cast_or_null<MCDataFragment>(F);
-  if (DF)
+  Symbol->setOffset(Offset);
+  if (DF) {
     Symbol->setFragment(F);
-  else
+  } else {
+    assert(isa<MCDummyFragment>(F) &&
+           "F must either be an MCDataFragment or the pending MCDummyFragment");
+    assert(Offset == 0);
     PendingLabels.push_back(Symbol);
+  }
 }
 
 void MCObjectStreamer::EmitULEB128Value(const MCExpr *Value) {

diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
@@ -659,11 +659,10 @@ class ARMELFStreamer : public MCELFStreamer {
                          uint64_t Offset) {
     auto *Symbol = cast<MCSymbolELF>(getContext().getOrCreateSymbol(
         Name + "." + Twine(MappingSymbolCounter++)));
-    EmitLabel(Symbol, Loc, F);
+    EmitLabelAtPos(Symbol, Loc, F, Offset);
     Symbol->setType(ELF::STT_NOTYPE);
     Symbol->setBinding(ELF::STB_LOCAL);
     Symbol->setExternal(false);
-    Symbol->setOffset(Offset);
   }
 
   void EmitThumbFunc(MCSymbol *Func) override {

diff --git a/llvm/test/MC/AsmParser/assembler-expressions.s b/llvm/test/MC/AsmParser/assembler-expressions.s
@@ -5,7 +5,7 @@
 
 # OBJDATA: Contents of section .data
 # OBJDATA-NEXT: 0000 aa0506ff
-	
+
 foo2:
 # ASM-ERR: [[@LINE+1]]:5: error: expected absolute expression
 .if . - foo2 == 0
@@ -26,17 +26,22 @@ foo3:
 .byte 0xff
 
 # nop is a fixed size instruction so this should pass.
-	
+
 # OBJTEXT: Contents of section .text
-# OBJTEXT-NEXT: 0000 9090ff34 25000000 00909090 78563412
-# OBJTEXT-NEXT: 0010 78563412 90
+# OBJTEXT-NEXT: 0000 909090ff 34250000 00009090 90785634
+# OBJTEXT-NEXT: 0010 12785634 1290
 
 .text
-
 text1:
+# ASM-ERR: [[@LINE+1]]:5: error: expected absolute expression
+.if . - text1 == 0
+	nop
+.endif
+
+text2:
         nop
 # ASM-ERR: [[@LINE+1]]:5: error: expected absolute expression
-.if . - text1 == 1
+.if . - text2 == 1
 	nop
 .else
 	ret
@@ -46,15 +51,14 @@ text1:
 	nop
 	nop
 # No additional errors.
-#	
+#
 # ASM-ERR-NOT: {{[0-9]+}}:{{[0-9]+}}: error:
 
-
 
-text2:
-	.long 0x12345678
 text3:
-	.fill (text3-text2)/4, 4, 0x12345678
+	.long 0x12345678
+text4:
+	.fill (text4-text3)/4, 4, 0x12345678
 	nop