[MC] Reject CFI advance_loc separated by a non-private label for Mach-O

[MaskRay]

Summary

Due to Mach-O's .subsections_via_symbols mechanism, non-private labels cannot
appear between .cfi_startproc/.cfi_endproc. Compilers do not produce such
labels, but hand-written assembly may. Give an error. Unfortunately,
emitDwarfAdvanceFrameAddr generated MCExpr doesn't have location
informatin.

Note: evaluateKnownAbsolute is to force folding A-B to a constant even if A and
B are separate by a non-private label. The function is a workaround for some
Mach-O assembl issues and should generally be avoided.

[MaskRay]

Ping:)

[efriedma]

LGTM

We could consider adding an SMLoc field to MCDwarfCallFrameFragment; other fragments have a location field (for similar reasons, I assume). But that doesn't need to block this fix.

[MaskRay]

In D153167#4450248, @efriedma wrote:

LGTM

We could consider adding an SMLoc field to MCDwarfCallFrameFragment; other fragments have a location field (for similar reasons, I assume). But that doesn't need to block this fix.

Thank you!
To get SMLoc, I think we'll need to change functions like llvm/include/llvm/MC/MCDwarf.h:createAdjustCfaOffset and FrameEmitterImpl::emitCFIInstructions, quite a few changes.
I'll need to investigate this area more to prepare for --gsframe support...

[jroelofs]

re: https://reviews.llvm.org/D153167#4657202

oops. copied the wrong link into that commit message.

llvm/lib/MC/MCAssembler.cpp

 
 bool MCAssembler::relaxDwarfCallFrameFragment(MCAsmLayout &Layout,
                                               MCDwarfCallFrameFragment &DF) {
   bool WasRelaxed;
   if (getBackend().relaxDwarfCFA(DF, Layout, WasRelaxed))
     return WasRelaxed;
 
   MCContext &Context = Layout.getAssembler().getContext();
-  uint64_t OldSize = DF.getContents().size();
-  int64_t AddrDelta;
-  bool Abs = DF.getAddrDelta().evaluateKnownAbsolute(AddrDelta, Layout);
-  assert(Abs && "We created call frame with an invalid expression");
-  (void) Abs;
+  int64_t Value;
+  bool Abs = DF.getAddrDelta().evaluateAsAbsolute(Value, Layout);
+  if (!Abs) {
+    getContext().reportError(DF.getAddrDelta().getLoc(),
+                             "invalid CFI advance_loc expression");
+    return false;
+  }
+
   SmallVectorImpl<char> &Data = DF.getContents();
+  uint64_t OldSize = Data.size();
   Data.clear();
   DF.getFixups().clear();
 
-  MCDwarfFrameEmitter::encodeAdvanceLoc(Context, AddrDelta, Data);
+  MCDwarfFrameEmitter::encodeAdvanceLoc(Context, Value, Data);
   return OldSize != Data.size();
 }
 
 bool MCAssembler::relaxCVInlineLineTable(MCAsmLayout &Layout,
                                          MCCVInlineLineTableFragment &F) {
   unsigned OldSize = F.getContents().size();
   getContext().getCVContext().encodeInlineLineTable(Layout, F);
   return OldSize != F.getContents().size();

llvm/test/MC/MachO/cfi-advance-loc-err.s

+# RUN: not llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o /dev/null 2>&1 | FileCheck %s --implicit-check-not=error:
+
+# CHECK-COUNT-4: <unknown>:0: error: invalid CFI advance_loc expression
+
+.section __TEXT,__text
+.globl _foo
+_foo:
+  .cfi_startproc
+  subq $8, %rsp
+  .cfi_adjust_cfa_offset 8
+  subq $8, %rsp
+  .cfi_adjust_cfa_offset 8
+
+tmp0: # non-private label cannot appear here
+  addq $8, %rsp
+  .cfi_adjust_cfa_offset -8
+.tmp1: # non-private label cannot appear here
+  addq $8, %rsp
+  .cfi_adjust_cfa_offset -8
+  retq
+  .cfi_endproc