FWIW I don't like that we're using "last emitted label" to track this. It means that we could start failing if someone emitted a label between the function label and the cfi directive and we wouldn't know why it's failing.<br><div><br></div><div>I see you've reverted it for the time being, but before recommitting could you try to organize this a bit differently?</div><div><br></div><div>Thanks!</div><div><br></div><div>-eric</div><br><div class="gmail_quote">On Mon Nov 03 2014 at 4:18:30 AM Oliver Stannard <<a href="mailto:oliver.stannard@arm.com">oliver.stannard@arm.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: olista01<br>
Date: Mon Nov 3 06:02:51 2014<br>
New Revision: 221150<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=221150&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=221150&view=rev</a><br>
Log:<br>
Emit .eh_frame with relocations to functions, rather than sections<br>
<br>
When LLVM emits DWARF call frame information, it currently creates a local,<br>
section-relative symbol in the code section, which is pointed to by a<br>
relocation on the .eh_frame section. However, for C++ we emit some functions in<br>
section groups, and the SysV ABI has some rules to make it easier to remove<br>
these sections<br>
(<a href="http://www.sco.com/developers/gabi/latest/ch4.sheader.html#section_group_rules" target="_blank">http://www.sco.com/<u></u>developers/gabi/latest/ch4.<u></u>sheader.html#section_group_<u></u>rules</a>):<br>
<br>
A symbol table entry with STB_LOCAL binding that is defined relative to one<br>
of a group's sections, and that is contained in a symbol table section that is<br>
not part of the group, must be discarded if the group members are discarded.<br>
References to this symbol table entry from outside the group are not allowed.<br>
<br>
This means that we need to use the function symbol for the relocation, not a<br>
temporary symbol.<br>
<br>
There was a comment in the code claiming that the local symbol was used to<br>
avoid creating a relocation, but a relocation must be created anyway as the<br>
code and CFI are in different sections.<br>
<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/MC/<u></u>MCObjectStreamer.h<br>
llvm/trunk/include/llvm/MC/<u></u>MCStreamer.h<br>
llvm/trunk/lib/CodeGen/<u></u>AsmPrinter/ARMException.cpp<br>
llvm/trunk/lib/CodeGen/<u></u>AsmPrinter/DwarfCFIException.<u></u>cpp<br>
llvm/trunk/lib/MC/<u></u>MCAsmStreamer.cpp<br>
llvm/trunk/lib/MC/<u></u>MCObjectStreamer.cpp<br>
llvm/trunk/lib/MC/MCParser/<u></u>AsmParser.cpp<br>
llvm/trunk/lib/MC/MCStreamer.<u></u>cpp<br>
llvm/trunk/test/DebugInfo/<u></u>AArch64/eh_frame.s<br>
llvm/trunk/test/DebugInfo/<u></u>AArch64/eh_frame_personality.<u></u>ll<br>
<br>
Modified: llvm/trunk/include/llvm/MC/<u></u>MCObjectStreamer.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCObjectStreamer.h?rev=221150&r1=221149&r2=221150&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/include/<u></u>llvm/MC/MCObjectStreamer.h?<u></u>rev=221150&r1=221149&r2=<u></u>221150&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/include/llvm/MC/<u></u>MCObjectStreamer.h (original)<br>
+++ llvm/trunk/include/llvm/MC/<u></u>MCObjectStreamer.h Mon Nov 3 06:02:51 2014<br>
@@ -41,7 +41,8 @@ class MCObjectStreamer : public MCStream<br>
SmallVector<MCSymbolData *, 2> PendingLabels;<br>
<br>
virtual void EmitInstToData(const MCInst &Inst, const MCSubtargetInfo&) = 0;<br>
- void EmitCFIStartProcImpl(<u></u>MCDwarfFrameInfo &Frame) override;<br>
+ void EmitCFIStartProcImpl(<u></u>MCDwarfFrameInfo &Frame,<br>
+ MCSymbol *FuncSym) override;<br>
void EmitCFIEndProcImpl(<u></u>MCDwarfFrameInfo &Frame) override;<br>
<br>
// If any labels have been emitted but not assigned fragments, ensure that<br>
<br>
Modified: llvm/trunk/include/llvm/MC/<u></u>MCStreamer.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCStreamer.h?rev=221150&r1=221149&r2=221150&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/include/<u></u>llvm/MC/MCStreamer.h?rev=<u></u>221150&r1=221149&r2=221150&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/include/llvm/MC/<u></u>MCStreamer.h (original)<br>
+++ llvm/trunk/include/llvm/MC/<u></u>MCStreamer.h Mon Nov 3 06:02:51 2014<br>
@@ -198,7 +198,7 @@ class MCStreamer {<br>
protected:<br>
MCStreamer(MCContext &Ctx);<br>
<br>
- virtual void EmitCFIStartProcImpl(<u></u>MCDwarfFrameInfo &Frame);<br>
+ virtual void EmitCFIStartProcImpl(<u></u>MCDwarfFrameInfo &Frame, MCSymbol *FuncSym);<br>
virtual void EmitCFIEndProcImpl(<u></u>MCDwarfFrameInfo &CurFrame);<br>
<br>
WinEH::FrameInfo *getCurrentWinFrameInfo() {<br>
@@ -661,7 +661,7 @@ public:<br>
<br>
virtual MCSymbol *getDwarfLineTableSymbol(<u></u>unsigned CUID);<br>
virtual void EmitCFISections(bool EH, bool Debug);<br>
- void EmitCFIStartProc(bool IsSimple);<br>
+ void EmitCFIStartProc(bool IsSimple, MCSymbol *FuncSym);<br>
void EmitCFIEndProc();<br>
virtual void EmitCFIDefCfa(int64_t Register, int64_t Offset);<br>
virtual void EmitCFIDefCfaOffset(int64_t Offset);<br>
<br>
Modified: llvm/trunk/lib/CodeGen/<u></u>AsmPrinter/ARMException.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/ARMException.cpp?rev=221150&r1=221149&r2=221150&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/<u></u>CodeGen/AsmPrinter/<u></u>ARMException.cpp?rev=221150&<u></u>r1=221149&r2=221150&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/CodeGen/<u></u>AsmPrinter/ARMException.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/<u></u>AsmPrinter/ARMException.cpp Mon Nov 3 06:02:51 2014<br>
@@ -66,7 +66,7 @@ void ARMException::beginFunction(<u></u>const M<br>
"non-EH CFI not yet supported in prologue with EHABI lowering");<br>
if (MoveType == AsmPrinter::CFI_M_Debug) {<br>
shouldEmitCFI = true;<br>
- Asm->OutStreamer.<u></u>EmitCFIStartProc(false);<br>
+ Asm->OutStreamer.<u></u>EmitCFIStartProc(false, Asm->CurrentFnSym);<br>
}<br>
}<br>
<br>
<br>
Modified: llvm/trunk/lib/CodeGen/<u></u>AsmPrinter/DwarfCFIException.<u></u>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCFIException.cpp?rev=221150&r1=221149&r2=221150&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/<u></u>CodeGen/AsmPrinter/<u></u>DwarfCFIException.cpp?rev=<u></u>221150&r1=221149&r2=221150&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/CodeGen/<u></u>AsmPrinter/DwarfCFIException.<u></u>cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/<u></u>AsmPrinter/DwarfCFIException.<u></u>cpp Mon Nov 3 06:02:51 2014<br>
@@ -102,7 +102,7 @@ void DwarfCFIException::<u></u>beginFunction(co<br>
if (!shouldEmitPersonality && !shouldEmitMoves)<br>
return;<br>
<br>
- Asm->OutStreamer.<u></u>EmitCFIStartProc(/*IsSimple=*/<u></u>false);<br>
+ Asm->OutStreamer.<u></u>EmitCFIStartProc(/*IsSimple=*/<u></u>false, Asm->CurrentFnSym);<br>
<br>
// Indicate personality routine, if any.<br>
if (!shouldEmitPersonality)<br>
<br>
Modified: llvm/trunk/lib/MC/<u></u>MCAsmStreamer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAsmStreamer.cpp?rev=221150&r1=221149&r2=221150&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/MC/<u></u>MCAsmStreamer.cpp?rev=221150&<u></u>r1=221149&r2=221150&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/MC/<u></u>MCAsmStreamer.cpp (original)<br>
+++ llvm/trunk/lib/MC/<u></u>MCAsmStreamer.cpp Mon Nov 3 06:02:51 2014<br>
@@ -54,7 +54,8 @@ private:<br>
unsigned UseDwarfDirectory : 1;<br>
<br>
void EmitRegisterName(int64_t Register);<br>
- void EmitCFIStartProcImpl(<u></u>MCDwarfFrameInfo &Frame) override;<br>
+ void EmitCFIStartProcImpl(<u></u>MCDwarfFrameInfo &Frame,<br>
+ MCSymbol *FuncSym) override;<br>
void EmitCFIEndProcImpl(<u></u>MCDwarfFrameInfo &Frame) override;<br>
<br>
public:<br>
@@ -925,7 +926,8 @@ void MCAsmStreamer::<u></u>EmitCFISections(bool<br>
EmitEOL();<br>
}<br>
<br>
-void MCAsmStreamer::<u></u>EmitCFIStartProcImpl(<u></u>MCDwarfFrameInfo &Frame) {<br>
+void MCAsmStreamer::<u></u>EmitCFIStartProcImpl(<u></u>MCDwarfFrameInfo &Frame,<br>
+ MCSymbol *FuncSym) {<br>
OS << "\t.cfi_startproc";<br>
if (Frame.IsSimple)<br>
OS << " simple";<br>
<br>
Modified: llvm/trunk/lib/MC/<u></u>MCObjectStreamer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCObjectStreamer.cpp?rev=221150&r1=221149&r2=221150&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/MC/<u></u>MCObjectStreamer.cpp?rev=<u></u>221150&r1=221149&r2=221150&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/MC/<u></u>MCObjectStreamer.cpp (original)<br>
+++ llvm/trunk/lib/MC/<u></u>MCObjectStreamer.cpp Mon Nov 3 06:02:51 2014<br>
@@ -128,10 +128,13 @@ void MCObjectStreamer::<u></u>EmitValueImpl(con<br>
DF->getContents().resize(DF-><u></u>getContents().size() + Size, 0);<br>
}<br>
<br>
-void MCObjectStreamer::<u></u>EmitCFIStartProcImpl(<u></u>MCDwarfFrameInfo &Frame) {<br>
- // We need to create a local symbol to avoid relocations.<br>
- Frame.Begin = getContext().CreateTempSymbol(<u></u>);<br>
- EmitLabel(Frame.Begin);<br>
+void MCObjectStreamer::<u></u>EmitCFIStartProcImpl(<u></u>MCDwarfFrameInfo &Frame,<br>
+ MCSymbol *FuncSym) {<br>
+ if (!FuncSym) {<br>
+ FuncSym = getContext().CreateTempSymbol(<u></u>);<br>
+ EmitLabel(FuncSym);<br>
+ }<br>
+ Frame.Begin = FuncSym;<br>
}<br>
<br>
void MCObjectStreamer::<u></u>EmitCFIEndProcImpl(<u></u>MCDwarfFrameInfo &Frame) {<br>
<br>
Modified: llvm/trunk/lib/MC/MCParser/<u></u>AsmParser.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/AsmParser.cpp?rev=221150&r1=221149&r2=221150&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/MC/<u></u>MCParser/AsmParser.cpp?rev=<u></u>221150&r1=221149&r2=221150&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/MC/MCParser/<u></u>AsmParser.cpp (original)<br>
+++ llvm/trunk/lib/MC/MCParser/<u></u>AsmParser.cpp Mon Nov 3 06:02:51 2014<br>
@@ -172,6 +172,9 @@ private:<br>
/// \brief Are we parsing ms-style inline assembly?<br>
bool ParsingInlineAsm;<br>
<br>
+ /// \brief The last symbol we emitted, used for call frame information.<br>
+ MCSymbol *LastFuncSymbol;<br>
+<br>
public:<br>
AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,<br>
const MCAsmInfo &MAI);<br>
@@ -491,7 +494,8 @@ AsmParser::AsmParser(SourceMgr &_SM, MCC<br>
: Lexer(_MAI), Ctx(_Ctx), Out(_Out), MAI(_MAI), SrcMgr(_SM),<br>
PlatformParser(nullptr), CurBuffer(_SM.getMainFileID())<u></u>,<br>
MacrosEnabledFlag(true), HadError(false), CppHashLineNumber(0),<br>
- AssemblerDialect(~0U), IsDarwin(false), ParsingInlineAsm(false) {<br>
+ AssemblerDialect(~0U), IsDarwin(false), ParsingInlineAsm(false),<br>
+ LastFuncSymbol(nullptr) {<br>
// Save the old handler.<br>
SavedDiagHandler = SrcMgr.getDiagHandler();<br>
SavedDiagContext = SrcMgr.getDiagContext();<br>
@@ -1305,6 +1309,9 @@ bool AsmParser::parseStatement(<u></u>ParseStat<br>
if (!ParsingInlineAsm)<br>
Out.EmitLabel(Sym);<br>
<br>
+ // Record the symbol, so that it can be used for call frame information<br>
+ LastFuncSymbol = Sym;<br>
+<br>
// If we are generating dwarf for assembly source files then gather the<br>
// info to make a dwarf label entry for this label if needed.<br>
if (getContext().<u></u>getGenDwarfForAssembly())<br>
@@ -2961,7 +2968,7 @@ bool AsmParser::<u></u>parseDirectiveCFIStartPr<br>
if (parseIdentifier(Simple) || Simple != "simple")<br>
return TokError("unexpected token in .cfi_startproc directive");<br>
<br>
- getStreamer().<u></u>EmitCFIStartProc(!Simple.<u></u>empty());<br>
+ getStreamer().<u></u>EmitCFIStartProc(!Simple.<u></u>empty(), LastFuncSymbol);<br>
return false;<br>
}<br>
<br>
<br>
Modified: llvm/trunk/lib/MC/MCStreamer.<u></u>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCStreamer.cpp?rev=221150&r1=221149&r2=221150&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/lib/MC/<u></u>MCStreamer.cpp?rev=221150&r1=<u></u>221149&r2=221150&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/lib/MC/MCStreamer.<u></u>cpp (original)<br>
+++ llvm/trunk/lib/MC/MCStreamer.<u></u>cpp Mon Nov 3 06:02:51 2014<br>
@@ -211,14 +211,14 @@ void MCStreamer::EmitCFISections(<u></u>bool EH<br>
assert(EH || Debug);<br>
}<br>
<br>
-void MCStreamer::EmitCFIStartProc(<u></u>bool IsSimple) {<br>
+void MCStreamer::EmitCFIStartProc(<u></u>bool IsSimple, MCSymbol *FuncSym) {<br>
MCDwarfFrameInfo *CurFrame = getCurrentDwarfFrameInfo();<br>
if (CurFrame && !CurFrame->End)<br>
report_fatal_error("Starting a frame before finishing the previous one!");<br>
<br>
MCDwarfFrameInfo Frame;<br>
Frame.IsSimple = IsSimple;<br>
- EmitCFIStartProcImpl(Frame);<br>
+ EmitCFIStartProcImpl(Frame, FuncSym);<br>
<br>
const MCAsmInfo* MAI = Context.getAsmInfo();<br>
if (MAI) {<br>
@@ -233,8 +233,8 @@ void MCStreamer::EmitCFIStartProc(<u></u>bool I<br>
DwarfFrameInfos.push_back(<u></u>Frame);<br>
}<br>
<br>
-void MCStreamer::<u></u>EmitCFIStartProcImpl(<u></u>MCDwarfFrameInfo &Frame) {<br>
-}<br>
+void MCStreamer::<u></u>EmitCFIStartProcImpl(<u></u>MCDwarfFrameInfo &Frame,<br>
+ MCSymbol *FuncSym) {}<br>
<br>
void MCStreamer::EmitCFIEndProc() {<br>
EnsureValidDwarfFrame();<br>
<br>
Modified: llvm/trunk/test/DebugInfo/<u></u>AArch64/eh_frame.s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/AArch64/eh_frame.s?rev=221150&r1=221149&r2=221150&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/test/<u></u>DebugInfo/AArch64/eh_frame.s?<u></u>rev=221150&r1=221149&r2=<u></u>221150&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/test/DebugInfo/<u></u>AArch64/eh_frame.s (original)<br>
+++ llvm/trunk/test/DebugInfo/<u></u>AArch64/eh_frame.s Mon Nov 3 06:02:51 2014<br>
@@ -1,5 +1,6 @@<br>
// RUN: llvm-mc -triple aarch64-none-linux-gnu -filetype=obj %s -o %t<br>
-// RUN: llvm-objdump -s %t | FileCheck %s<br>
+// RUN: llvm-objdump -s %t | FileCheck %s --check-prefix=CHECK<br>
+// RUN: llvm-readobj -r %t | FileCheck %s --check-prefix=RELOC<br>
.text<br>
.globl foo<br>
.type foo,@function<br>
@@ -46,3 +47,11 @@ foo:<br>
// 00000000: PC begin for this FDE is at 00000000 (relocation is applied here)<br>
// 04000000: FDE applies up to PC begin+0x14<br>
// 00: Augmentation string length 0 for this FDE<br>
+<br>
+<br>
+// Check the relocations applied to the .eh_frame section.<br>
+// These must not contain section-relative relocations to a section which<br>
+// is part of a group, as it could be removed.<br>
+// RELOC: Section ({{[0-9]+}}) .rela.eh_frame {<br>
+// RELOC-NEXT: 0x{{[0-9A-F]+}} R_AARCH64_PREL32 foo 0x0<br>
+// RELOC-NEXT: }<br>
<br>
Modified: llvm/trunk/test/DebugInfo/<u></u>AArch64/eh_frame_personality.<u></u>ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/AArch64/eh_frame_personality.ll?rev=221150&r1=221149&r2=221150&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/llvm/trunk/test/<u></u>DebugInfo/AArch64/eh_frame_<u></u>personality.ll?rev=221150&r1=<u></u>221149&r2=221150&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- llvm/trunk/test/DebugInfo/<u></u>AArch64/eh_frame_personality.<u></u>ll (original)<br>
+++ llvm/trunk/test/DebugInfo/<u></u>AArch64/eh_frame_personality.<u></u>ll Mon Nov 3 06:02:51 2014<br>
@@ -1,5 +1,6 @@<br>
; RUN: llc -verify-machineinstrs -mtriple=aarch64-none-linux-<u></u>gnu %s -filetype=obj -o %t<br>
-; RUN: llvm-objdump -s %t | FileCheck %s<br>
+; RUN: llvm-objdump -s %t | FileCheck %s --check-prefix=CHECK<br>
+; RUN: llvm-readobj -r %t | FileCheck %s --check-prefix=RELOC<br>
<br>
declare i32 @__gxx_personality_v0(...)<br>
<br>
@@ -44,3 +45,12 @@ clean:<br>
; 00: Second part of aug (language-specific data): absolute pointer format used<br>
; 1b: pointer format: pc-relative signed 4-byte. Just like GNU.<br>
; 0c 1f 00: Initial instructions ("DW_CFA_def_cfa x31 ofs 0" in this case)<br>
+<br>
+; Check the relocations applied to the .eh_frame section.<br>
+; These must not contain section-relative relocations to a section which<br>
+; is part of a group, as it could be removed.<br>
+; RELOC: Section ({{[0-9]+}}) .rela.eh_frame {<br>
+; RELOC-NEXT: 0x{{[0-9A-F]+}} R_AARCH64_ABS64 __gxx_personality_v0 0x0<br>
+; RELOC-NEXT: 0x{{[0-9A-F]+}} R_AARCH64_PREL32 foo 0x0<br>
+; RELOC-NEXT: 0x{{[0-9A-F]+}} R_AARCH64_ABS64 .gcc_except_table 0x0<br>
+; RELOC-NEXT: }<br>
<br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</blockquote></div>