[PATCH][MC] Emit an error if cfi_startproc is used before a symbol is defined

[Eric Christopher]

gas appears to create a symbol for the .cfi_startproc and makes the
relocation relative to it:

.text
.space 1000
.cfi_startproc
.space 1000
.globl _someFunction
_someFunction :
  .cfi_def_cfa_offset 16
  .cfi_offset %rbp,-16
  .cfi_def_cfa_register rbp
  ret
  .cfi_endproc

RELOCATION RECORDS FOR [.eh_frame]:
OFFSET           TYPE              VALUE
0000000000000020 R_X86_64_PC32     .text+0x00000000000003e8

which holds up if you do:

.text
.space 1000
.globl _someFunction
_someFunction :
  .space 1000
  .cfi_startproc
  .cfi_def_cfa_offset 16
  .cfi_offset %rbp,-16
  .cfi_def_cfa_register rbp
  ret
  .cfi_endproc

RELOCATION RECORDS FOR [.eh_frame]:
OFFSET           TYPE              VALUE
0000000000000024 R_X86_64_PC32     .text+0x00000000000007d0

which seems to say that to be compatible we'll need to do the
relocation for the .cfi_startproc we need to emit it at the location
of the directive and not at the last known symbol.

What possible use this has? I have no idea. The last one appears that
it could be useful in some ways. Define a chunk of space that isn't
part of your function at the top and so you don't want it to be
considered part of the call frame that you then patch in on start up
perhaps?

*shrug*

Thoughts from the peanut gallery?

-eric

On Wed, Apr 9, 2014 at 4:06 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> Hi Eric and Jim,
>
> @Eric, I do not know if you are the right person to review this, but compact
> frame information looks related enough to dwarf emission that I assume you
> know :).
>
> The attached patch fixes an assertion failure[1] in the assembler when a
> .cfi_startproc is used before a function (or more generally a symbol) is
> defined.
>
> Thanks for your reviews/feedbacks.
>
> ** Context **
>
> Currently the assembler crashes when emitting the compact unwind information
> if the input assembly file starts with a .cfi_startproc directive before any
> symbol has been defined.
>
> I am not a CFI expert, but that does not look right to me to use a
> .cfi_startproc directive without having started the related function!
>
>
> ** Proposed Solution **
>
> Issue a backend fatal error if a frame is started whereas no symbol is
> defined. Although this diagnostic method is not pretty, this is how all cfi
> related errors are handled (e.g., looks for No open frame).
>
> The patch also modifies a few test cases to define a function before
> .cfi_startproc is used.
> I.e., test cases like this:
> .cfi_startproc
>
> Looks like this now:
> _proc:
> .cfi_startproc
>
> If that does not make sense, please let me know!
>
> <rdar://problem/15939159>
>
> [1]
> Assertion failed: (Symbol), function MCSymbolRefExpr, file
> include/llvm/MC/MCExpr.h, line 273.
> 0  llvm-mc                  0x0000000109d85e3e
> llvm::sys::PrintStackTrace(__sFILE*) + 46
> 1  llvm-mc                  0x0000000109d8614b
> PrintStackTraceSignalHandler(void*) + 27
> 2  llvm-mc                  0x0000000109d864c8 SignalHandler(int) + 408
> 3  libsystem_platform.dylib 0x00007fff8f8895aa _sigtramp + 26
> 4  libsystem_platform.dylib 0x00007f9452c05338 _sigtramp + 3275210152
> 5  llvm-mc                  0x0000000109d8617b raise + 27
> 6  llvm-mc                  0x0000000109d86232 abort + 18
> 7  llvm-mc                  0x0000000109d86211 __assert_rtn + 129
> 8  llvm-mc                  0x0000000109d019bf
> llvm::MCSymbolRefExpr::MCSymbolRefExpr(llvm::MCSymbol const*,
> llvm::MCSymbolRefExpr::VariantKind, llvm::MCAsmInfo const*) + 143
> 9  llvm-mc                  0x0000000109d0081b
> llvm::MCSymbolRefExpr::MCSymbolRefExpr(llvm::MCSymbol const*,
> llvm::MCSymbolRefExpr::VariantKind, llvm::MCAsmInfo const*) + 43
> 10 llvm-mc                  0x0000000109cfda56
> llvm::MCSymbolRefExpr::Create(llvm::MCSymbol const*,
> llvm::MCSymbolRefExpr::VariantKind, llvm::MCContext&) + 134
> 11 llvm-mc                  0x0000000109b79df4
> llvm::MCSymbolRefExpr::Create(llvm::MCSymbol const*, llvm::MCContext&) + 36
> 12 llvm-mc                  0x0000000109d16f85
> llvm::MCStreamer::EmitSymbolValue(llvm::MCSymbol const*, unsigned int) + 69
> 13 llvm-mc                  0x0000000109ceb7ca (anonymous
> namespace)::FrameEmitterImpl::EmitCompactUnwind(llvm::MCStreamer&,
> llvm::MCDwarfFrameInfo const&) + 314
> 14 llvm-mc                  0x0000000109ceb2d6
> llvm::MCDwarfFrameEmitter::Emit(llvm::MCStreamer&, llvm::MCAsmBackend*,
> bool, bool) + 470
> 15 llvm-mc                  0x0000000109d1b1cf
> llvm::MCStreamer::EmitFrames(llvm::MCAsmBackend*, bool) + 95
> 16 llvm-mc                  0x0000000109d04152 (anonymous
> namespace)::MCMachOStreamer::FinishImpl() + 66
> 17 llvm-mc                  0x0000000109d1b2df llvm::MCStreamer::Finish() +
> 159
> 18 llvm-mc                  0x0000000109c30582 (anonymous
> namespace)::AsmParser::Run(bool, bool) + 2546
> 19 llvm-mc                  0x000000010995457c AssembleInput(char const*,
> llvm::Target const*, llvm::SourceMgr&, llvm::MCContext&, llvm::MCStreamer&,
> llvm::MCAsmInfo&, llvm::MCSubtargetInfo&, llvm::MCInstrInfo&) + 940
> 20 llvm-mc                  0x0000000109952461 main + 9457
> 21 libdyld.dylib            0x00007fff959e65fd start + 1
>
> Cheers,
> -Quentin
>
>