[MC] Consume EndOfStatement in .cfi_{sections,endproc}

[scott.linder]

Summary

Previously these directives were always interpreted as having an extra
blank line after them.

[MaskRay]

Previously these directives were always interpreted as having an extra blank line after them.

EndOfStatement was not consumed and the next AsmParser::parseStatement would consume EndOfStatement and add a blank line.

llvm/test/MC/AsmParser/directive-parse-err.s

1

Please merge this into a .cfi_* parsing test. You can leverage -defsym ERR=1 to check errors. See MC/ELF/reloc-directive.s for an example

6

[[@LINE+1]] is deprecated. Use [[#@LINE+1]]

llvm/test/MC/AsmParser/round-trip.s

16

One pair .cfi_startproc of should be sufficient.

[scott.linder]

llvm/test/MC/AsmParser/directive-parse-err.s

1

To clarify, is there an "omnibus" cfi parsing test under MC/AsmParser? I see a few specific examples, but not just cfi.s. Did you have a particular test in mind?

6

Sorry, I keep forgetting this when I look at existing tests as templates. I'll update this in the next patch!

llvm/test/MC/AsmParser/round-trip.s

16

Agreed, I'm not sure why I added the repeated pairs, I'll update this in the next patch.

[MaskRay]

llvm/test/MC/AsmParser/directive-parse-err.s

1

They don't have to be under MC/AsmParser. You may use MC/ELF where there are a few cfi*.s

[scott.linder]

Merge new test into existing cfi.s test, remove gratuitous directives in the
round trip test. Replace use of deprecated FileCheck syntax.

[scott.linder]

llvm/test/MC/AsmParser/directive-parse-err.s

1

Ok, it just seemed like tests under ELF/ were for the binary result, not necessarily for the behavior of the parser directly.

[MaskRay]

Thanks!

llvm/test/MC/ELF/cfi.s

439 ↗

(On Diff #309601)

I think it is more common to place RUN lines above all the CHECK lines.

MC/ELF does have some parser tests (-filetype=asm). I agree that the organization of tests is a bit unclear but I think MC/ELF for these .cfi_* is fine because they are indeed specific to certain binary formats.

llvm/lib/MC/MCParser/AsmParser.cpp

if (parseIdentifier(Name))
       return TokError("Expected an identifier");
 
     if (Name == ".eh_frame")
       EH = true;
     else if (Name == ".debug_frame")
       Debug = true;
   }
 
+  if (parseToken(AsmToken::EndOfStatement))
+    return addErrorSuffix(" in '.cfi_sections' directive");
+
   getStreamer().emitCFISections(EH, Debug);
   return false;
 }
 
 /// parseDirectiveCFIStartProc
 /// ::= .cfi_startproc [simple]
 bool AsmParser::parseDirectiveCFIStartProc() {
   StringRef Simple;
bool AsmParser::parseDirectiveCFIStartProc() {
   // correct context information.
   getStreamer().emitCFIStartProc(!Simple.empty(), Lexer.getLoc());
   return false;
 }
 
 /// parseDirectiveCFIEndProc
 /// ::= .cfi_endproc
 bool AsmParser::parseDirectiveCFIEndProc() {
+  if (parseToken(AsmToken::EndOfStatement))
+    return addErrorSuffix(" in '.cfi_endproc' directive");
   getStreamer().emitCFIEndProc();
   return false;
 }
 
 /// parse register name or number.
 bool AsmParser::parseRegisterOrRegisterNumber(int64_t &Register,
                                               SMLoc DirectiveLoc) {
   unsigned RegNo;

llvm/test/MC/AsmParser/directive-parse-err.s

+// RUN: not llvm-mc -triple i386-unknown-unknown %s 2>&1 | FileCheck %s
+// RUN: not llvm-mc -triple i386-unknown-unknown %s 2>&1 | grep "error:" | count 6
+
+// Test that we correctly diagnose parse errors.
+
+// CHECK: [[@LINE+1]]:15: error: Expected an identifier
+.cfi_sections $
+// CHECK: [[@LINE+1]]:28: error: unexpected token in '.cfi_sections' directive
+.cfi_sections .debug_frame $
+// CHECK: [[@LINE+1]]:39: error: unexpected token in '.cfi_sections' directive
+.cfi_sections .debug_frame, .eh_frame $
+
+// CHECK: [[@LINE+1]]:16: error: unexpected token in '.cfi_startproc' directive
+.cfi_startproc $
+// CHECK: [[@LINE+1]]:23: error: unexpected token in '.cfi_startproc' directive
+.cfi_startproc simple $
+
+// CHECK: [[@LINE+1]]:14: error: unexpected token in '.cfi_endproc' directive
+.cfi_endproc $
+
+// CHECK-NOT: error:

llvm/test/MC/AsmParser/round-trip.s

+# RUN: llvm-mc -preserve-comments -triple i386-unknown-unknown %s >%t-1.s
+# RUN: llvm-mc -preserve-comments -triple i386-unknown-unknown %t-1.s >%t-2.s
+# RUN: diff %t-1.s %t-2.s
+
+# Test that various assembly round-trips when run through MC; the first
+# transition from hand-written to "canonical" output may introduce some small
+# differences, so we don't include the initial input in the comparison.
+
+.text
+
+# Some of these CFI instructions didn't consume the end of statement
+# consistently, which led to extra empty lines in the output.
+.cfi_sections .debug_frame
+.cfi_startproc
+.cfi_endproc
+.cfi_startproc
+.cfi_endproc