Emit .eh_frame with relocations to functions, rather than sections

[olista01]

Summary

When LLVM emits DWARF call frame information, it currently creates a local, section-relative symbol in the code section, which is pointed to by a relocation on the .eh_frame section. However, for C++ we emit some functions in section groups, and the SysV ABI has some rules to make it easier to remove these sections (http://www.sco.com/developers/gabi/latest/ch4.sheader.html#section_group_rules):

A symbol table entry with STB_LOCAL binding that is defined relative to one of a group's sections, and that is contained in a symbol table section that is not part of the group, must be discarded if the group members are discarded. References to this symbol table entry from outside the group are not allowed.

This means that we need to use the function symbol for the relocation, not a temporary symbol.

There was a comment in the code claiming that the local symbol was used to avoid creating a relocation, but a relocation must be created anyway as the code and CFI are in different sections.

[rengolin]

Hi Oliver,

Looks reasonable to me, with the comment below.

cheers,
--renato

lib/MC/MCParser/AsmParser.cpp

498

I'd call it something similar FuncSymbol to help identify it with it use.

[olista01]

Committed as r221150, with your suggested change.

include/llvm/MC/MCObjectStreamer.h

class MCObjectStreamer : public MCStreamer {
   MCAssembler *Assembler;
   MCSectionData *CurSectionData;
   MCSectionData::iterator CurInsertionPoint;
   bool EmitEHFrame;
   bool EmitDebugFrame;
   SmallVector<MCSymbolData *, 2> PendingLabels;
 
   virtual void EmitInstToData(const MCInst &Inst, const MCSubtargetInfo&) = 0;
-  void EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame) override;
+  void EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame,
+                            MCSymbol *FuncSym) override;
   void EmitCFIEndProcImpl(MCDwarfFrameInfo &Frame) override;
 
   // If any labels have been emitted but not assigned fragments, ensure that
   // they get assigned, either to F if possible or to a new data fragment.
   void flushPendingLabels(MCFragment *F);
 
 protected:
   MCObjectStreamer(MCContext &Context, MCAsmBackend &TAB, raw_ostream &_OS,

include/llvm/MC/MCStreamer.h

class MCStreamer {
 
   /// SectionStack - This is stack of current and previous section
   /// values saved by PushSection.
   SmallVector<std::pair<MCSectionSubPair, MCSectionSubPair>, 4> SectionStack;
 
 protected:
   MCStreamer(MCContext &Ctx);
 
-  virtual void EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame);
+  virtual void EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame, MCSymbol *FuncSym);
   virtual void EmitCFIEndProcImpl(MCDwarfFrameInfo &CurFrame);
 
   WinEH::FrameInfo *getCurrentWinFrameInfo() {
     return CurrentWinFrameInfo;
   }
 
   virtual void EmitWindowsUnwindTables();
 
public:
   // '.loc fileno lineno ...' assembler directive.
   virtual void EmitDwarfLocDirective(unsigned FileNo, unsigned Line,
                                      unsigned Column, unsigned Flags,
                                      unsigned Isa, unsigned Discriminator,
                                      StringRef FileName);
 
   virtual MCSymbol *getDwarfLineTableSymbol(unsigned CUID);
   virtual void EmitCFISections(bool EH, bool Debug);
-  void EmitCFIStartProc(bool IsSimple);
+  void EmitCFIStartProc(bool IsSimple, MCSymbol *FuncSym);
   void EmitCFIEndProc();
   virtual void EmitCFIDefCfa(int64_t Register, int64_t Offset);
   virtual void EmitCFIDefCfaOffset(int64_t Offset);
   virtual void EmitCFIDefCfaRegister(int64_t Register);
   virtual void EmitCFIOffset(int64_t Register, int64_t Offset);
   virtual void EmitCFIPersonality(const MCSymbol *Sym, unsigned Encoding);
   virtual void EmitCFILsda(const MCSymbol *Sym, unsigned Encoding);
   virtual void EmitCFIRememberState();

lib/CodeGen/AsmPrinter/ARMException.cpp

void ARMException::beginFunction(const MachineFunction *MF) {
   Asm->OutStreamer.EmitLabel(Asm->GetTempSymbol("eh_func_begin",
                                                 Asm->getFunctionNumber()));
   // See if we need call frame info.
   AsmPrinter::CFIMoveType MoveType = Asm->needsCFIMoves();
   assert(MoveType != AsmPrinter::CFI_M_EH &&
          "non-EH CFI not yet supported in prologue with EHABI lowering");
   if (MoveType == AsmPrinter::CFI_M_Debug) {
     shouldEmitCFI = true;
-    Asm->OutStreamer.EmitCFIStartProc(false);
+    Asm->OutStreamer.EmitCFIStartProc(false, Asm->CurrentFnSym);
   }
 }
 
 /// endFunction - Gather and emit post-function exception information.
 ///
 void ARMException::endFunction(const MachineFunction *) {
   if (shouldEmitCFI)
     Asm->OutStreamer.EmitCFIEndProc();

lib/CodeGen/AsmPrinter/DwarfCFIException.cpp

void DwarfCFIException::beginFunction(const MachineFunction *MF) {
 
   unsigned LSDAEncoding = TLOF.getLSDAEncoding();
   shouldEmitLSDA = shouldEmitPersonality &&
     LSDAEncoding != dwarf::DW_EH_PE_omit;
 
   if (!shouldEmitPersonality && !shouldEmitMoves)
     return;
 
-  Asm->OutStreamer.EmitCFIStartProc(/*IsSimple=*/false);
+  Asm->OutStreamer.EmitCFIStartProc(/*IsSimple=*/false, Asm->CurrentFnSym);
 
   // Indicate personality routine, if any.
   if (!shouldEmitPersonality)
     return;
 
   const MCSymbol *Sym =
       TLOF.getCFIPersonalitySymbol(Per, *Asm->Mang, Asm->TM, MMI);
   Asm->OutStreamer.EmitCFIPersonality(Sym, PerEncoding);

lib/MC/MCAsmStreamer.cpp

private:
   SmallString<128> CommentToEmit;
   raw_svector_ostream CommentStream;
 
   unsigned IsVerboseAsm : 1;
   unsigned ShowInst : 1;
   unsigned UseDwarfDirectory : 1;
 
   void EmitRegisterName(int64_t Register);
-  void EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame) override;
+  void EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame,
+                            MCSymbol *FuncSym) override;
   void EmitCFIEndProcImpl(MCDwarfFrameInfo &Frame) override;
 
 public:
   MCAsmStreamer(MCContext &Context, formatted_raw_ostream &os,
                 bool isVerboseAsm, bool useDwarfDirectory,
                 MCInstPrinter *printer, MCCodeEmitter *emitter,
                 MCAsmBackend *asmbackend, bool showInst)
       : MCStreamer(Context), OS(os), MAI(Context.getAsmInfo()),
if (Debug)
       OS << ", .debug_frame";
   } else if (Debug) {
     OS << ".debug_frame";
   }
 
   EmitEOL();
 }
 
-void MCAsmStreamer::EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame) {
+void MCAsmStreamer::EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame,
+                                         MCSymbol *FuncSym) {
   OS << "\t.cfi_startproc";
   if (Frame.IsSimple)
     OS << " simple";
   EmitEOL();
 }
 
 void MCAsmStreamer::EmitCFIEndProcImpl(MCDwarfFrameInfo &Frame) {
   MCStreamer::EmitCFIEndProcImpl(Frame);

lib/MC/MCObjectStreamer.cpp

if (Value->EvaluateAsAbsolute(AbsValue, getAssembler())) {
     return;
   }
   DF->getFixups().push_back(
       MCFixup::Create(DF->getContents().size(), Value,
                       MCFixup::getKindForSize(Size, false), Loc));
   DF->getContents().resize(DF->getContents().size() + Size, 0);
 }
 
-void MCObjectStreamer::EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame) {
-  // We need to create a local symbol to avoid relocations.
-  Frame.Begin = getContext().CreateTempSymbol();
-  EmitLabel(Frame.Begin);
+void MCObjectStreamer::EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame,
+                                            MCSymbol *FuncSym) {
+  if (!FuncSym) {
+    FuncSym = getContext().CreateTempSymbol();
+    EmitLabel(FuncSym);
+  }
+  Frame.Begin = FuncSym;
 }
 
 void MCObjectStreamer::EmitCFIEndProcImpl(MCDwarfFrameInfo &Frame) {
   Frame.End = getContext().CreateTempSymbol();
   EmitLabel(Frame.End);
 }
 
 void MCObjectStreamer::EmitLabel(MCSymbol *Symbol) {

lib/MC/MCParser/AsmParser.cpp

private:
   unsigned AssemblerDialect;
 
   /// \brief is Darwin compatibility enabled?
   bool IsDarwin;
 
   /// \brief Are we parsing ms-style inline assembly?
   bool ParsingInlineAsm;
 
+  /// \brief The last symbol we emitted, used for call frame information.
+  MCSymbol *LastSymbol;
+
 public:
   AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
             const MCAsmInfo &MAI);
   virtual ~AsmParser();
 
   bool Run(bool NoInitialTextSection, bool NoFinalize = false) override;
 
   void addDirectiveHandler(StringRef Directive,

 
 enum { DEFAULT_ADDRSPACE = 0 };
 
 AsmParser::AsmParser(SourceMgr &_SM, MCContext &_Ctx, MCStreamer &_Out,
                      const MCAsmInfo &_MAI)
     : Lexer(_MAI), Ctx(_Ctx), Out(_Out), MAI(_MAI), SrcMgr(_SM),
       PlatformParser(nullptr), CurBuffer(_SM.getMainFileID()),
       MacrosEnabledFlag(true), HadError(false), CppHashLineNumber(0),
-      AssemblerDialect(~0U), IsDarwin(false), ParsingInlineAsm(false) {
+      AssemblerDialect(~0U), IsDarwin(false), ParsingInlineAsm(false),
+      LastSymbol(nullptr) {
   // Save the old handler.
   SavedDiagHandler = SrcMgr.getDiagHandler();
   SavedDiagContext = SrcMgr.getDiagContext();
   // Set our own handler which calls the saved handler.
   SrcMgr.setDiagHandler(DiagHandler, this);
   Lexer.setBuffer(SrcMgr.getMemoryBuffer(CurBuffer)->getBuffer());
 
   // Initialize the platform / file format parser.
if (LocalLabelVal == -1) {
       Sym = Ctx.CreateDirectionalLocalSymbol(LocalLabelVal);
     if (!Sym->isUndefined() || Sym->isVariable())
       return Error(IDLoc, "invalid symbol redefinition");
 
     // Emit the label.
     if (!ParsingInlineAsm)
       Out.EmitLabel(Sym);
 
+    // Record the symbol, so that it can be used for call frame information
+    LastSymbol = Sym;
+
     // If we are generating dwarf for assembly source files then gather the
     // info to make a dwarf label entry for this label if needed.
     if (getContext().getGenDwarfForAssembly())
       MCGenDwarfLabelEntry::Make(Sym, &getStreamer(), getSourceManager(),
                                  IDLoc);
 
     getTargetParser().onLabelParsed(Sym);
 

 /// parseDirectiveCFIStartProc
 /// ::= .cfi_startproc [simple]
 bool AsmParser::parseDirectiveCFIStartProc() {
   StringRef Simple;
   if (getLexer().isNot(AsmToken::EndOfStatement))
     if (parseIdentifier(Simple) || Simple != "simple")
       return TokError("unexpected token in .cfi_startproc directive");
 
-  getStreamer().EmitCFIStartProc(!Simple.empty());
+  getStreamer().EmitCFIStartProc(!Simple.empty(), LastSymbol);
   return false;
 }
 
 /// parseDirectiveCFIEndProc
 /// ::= .cfi_endproc
 bool AsmParser::parseDirectiveCFIEndProc() {
   getStreamer().EmitCFIEndProc();
   return false;

lib/MC/MCStreamer.cpp

void MCStreamer::EmitLabel(MCSymbol *Symbol) {
   if (TS)
     TS->emitLabel(Symbol);
 }
 
 void MCStreamer::EmitCFISections(bool EH, bool Debug) {
   assert(EH || Debug);
 }
 
-void MCStreamer::EmitCFIStartProc(bool IsSimple) {
+void MCStreamer::EmitCFIStartProc(bool IsSimple, MCSymbol *FuncSym) {
   MCDwarfFrameInfo *CurFrame = getCurrentDwarfFrameInfo();
   if (CurFrame && !CurFrame->End)
     report_fatal_error("Starting a frame before finishing the previous one!");
 
   MCDwarfFrameInfo Frame;
   Frame.IsSimple = IsSimple;
-  EmitCFIStartProcImpl(Frame);
+  EmitCFIStartProcImpl(Frame, FuncSym);
 
   const MCAsmInfo* MAI = Context.getAsmInfo();
   if (MAI) {
     for (const MCCFIInstruction& Inst : MAI->getInitialFrameState()) {
       if (Inst.getOperation() == MCCFIInstruction::OpDefCfa ||
           Inst.getOperation() == MCCFIInstruction::OpDefCfaRegister) {
         Frame.CurrentCfaRegister = Inst.getRegister();
       }
     }
   }
 
   DwarfFrameInfos.push_back(Frame);
 }
 
-void MCStreamer::EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame) {
-}
+void MCStreamer::EmitCFIStartProcImpl(MCDwarfFrameInfo &Frame,
+                                      MCSymbol *FuncSym) {}
 
 void MCStreamer::EmitCFIEndProc() {
   EnsureValidDwarfFrame();
   MCDwarfFrameInfo *CurFrame = getCurrentDwarfFrameInfo();
   EmitCFIEndProcImpl(*CurFrame);
 }
 
 void MCStreamer::EmitCFIEndProcImpl(MCDwarfFrameInfo &Frame) {

test/DebugInfo/AArch64/eh_frame.s

 // RUN: llvm-mc -triple aarch64-none-linux-gnu -filetype=obj %s -o %t
-// RUN: llvm-objdump -s %t | FileCheck %s
+// RUN: llvm-objdump -s %t | FileCheck %s --check-prefix=CHECK
+// RUN: llvm-readobj -r %t | FileCheck %s --check-prefix=RELOC
         .text
         .globl foo
         .type foo,@function
 foo:
         .cfi_startproc
         ret
         .cfi_endproc
 

 
 // Next the FDE:
 // -------------
 // 10000000: FDE length 0x10
 // 18000000: Uses CIE 0x18 backwards (only coincidentally same as above)
 // 00000000: PC begin for this FDE is at 00000000 (relocation is applied here)
 // 04000000: FDE applies up to PC begin+0x14
 // 00: Augmentation string length 0 for this FDE
+
+
+// Check the relocations applied to the .eh_frame section.
+// These must not contain section-relative relocations to a section which
+// is part of a group, as it could be removed.
+// RELOC: Section ({{[0-9]+}}) .rela.eh_frame {
+// RELOC-NEXT:   0x{{[0-9A-F]+}} R_AARCH64_PREL32 foo 0x0
+// RELOC-NEXT: }

test/DebugInfo/AArch64/eh_frame_personality.ll

 ; RUN: llc -verify-machineinstrs -mtriple=aarch64-none-linux-gnu %s -filetype=obj -o %t
-; RUN: llvm-objdump -s %t | FileCheck %s
+; RUN: llvm-objdump -s %t | FileCheck %s --check-prefix=CHECK
+; RUN: llvm-readobj -r %t | FileCheck %s --check-prefix=RELOC
 
 declare i32 @__gxx_personality_v0(...)
 
 declare void @bar()
 
 define i64 @foo(i64 %lhs, i64 %rhs) {
   invoke void @bar() to label %end unwind label %clean
 end:

 ; 78: Data alignment factor: -8
 ; 1e: Return address in x30
 ; 07: Augmentation data 0xb bytes (this is key!)
 ; 00: Personality encoding is DW_EH_PE_absptr
 ; 00 00 00 00 00 00 00 00: First part of aug (personality routine). Relocated, obviously
 ; 00: Second part of aug (language-specific data): absolute pointer format used
 ; 1b: pointer format: pc-relative signed 4-byte. Just like GNU.
 ; 0c 1f 00: Initial instructions ("DW_CFA_def_cfa x31 ofs 0" in this case)
+
+; Check the relocations applied to the .eh_frame section.
+; These must not contain section-relative relocations to a section which
+; is part of a group, as it could be removed.
+; RELOC: Section ({{[0-9]+}}) .rela.eh_frame {
+; RELOC-NEXT:   0x{{[0-9A-F]+}} R_AARCH64_ABS64 __gxx_personality_v0 0x0
+; RELOC-NEXT:   0x{{[0-9A-F]+}} R_AARCH64_PREL32 foo 0x0
+; RELOC-NEXT:   0x{{[0-9A-F]+}} R_AARCH64_ABS64 .gcc_except_table 0x0
+; RELOC-NEXT: }