[PATCH] Fix .thumb_func directive in ARMAsmParser

[Gabor Ballabas]

Hi rengolin,

This patch is intended to solve the following FIXME in ARMAsmParser::parseDirectiveThumbFunc:

  // FIXME: assuming function name will be the line following .thumb_func
  // We really should be checking the next symbol definition even if there's
  // stuff in between.

I didn't add any new test just modified the existing one by moving the .type directive between the .thumb_func directive
and the actual symbol (you can check it in the diff). This is how GCC places the directives (at least it did it for me when
I compiled  a simple "Hello world!" C program to assembly code).

http://llvm-reviews.chandlerc.com/D2023

Files:
  include/llvm/MC/MCTargetAsmParser.h
  lib/MC/MCParser/AsmParser.cpp
  lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  test/MC/ARM/elf-thumbfunc.s

Index: include/llvm/MC/MCTargetAsmParser.h
===================================================================
--- include/llvm/MC/MCTargetAsmParser.h
+++ include/llvm/MC/MCTargetAsmParser.h
@@ -181,6 +181,8 @@
                                             MCContext &Ctx) {
     return 0;
   }
+
+  virtual void onLabelParsed(MCSymbol *Symbol) { };
 };
 
 } // End llvm namespace
Index: lib/MC/MCParser/AsmParser.cpp
===================================================================
--- lib/MC/MCParser/AsmParser.cpp
+++ lib/MC/MCParser/AsmParser.cpp
@@ -1262,6 +1262,8 @@
       MCGenDwarfLabelEntry::Make(Sym, &getStreamer(), getSourceManager(),
                                  IDLoc);
 
+    getTargetParser().onLabelParsed(Sym);
+
     // Consume any end of statement token, if present, to avoid spurious
     // AddBlankLine calls().
     if (Lexer.is(AsmToken::EndOfStatement)) {
Index: lib/Target/ARM/AsmParser/ARMAsmParser.cpp
===================================================================
--- lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -74,6 +74,8 @@
   // Map of register aliases registers via the .req directive.
   StringMap<unsigned> RegisterReqs;
 
+  bool nextSymbolIsThumb;
+
   struct {
     ARMCC::CondCodes Cond;    // Condition for IT block.
     unsigned Mask:4;          // Condition mask for instructions.
@@ -268,6 +270,8 @@
 
     // Not in an ITBlock to start with.
     ITState.CurPosition = ~0U;
+
+    nextSymbolIsThumb = false;
   }
 
   // Implementation of the MCTargetAsmParser interface:
@@ -284,6 +288,8 @@
                                SmallVectorImpl<MCParsedAsmOperand*> &Operands,
                                MCStreamer &Out, unsigned &ErrorInfo,
                                bool MatchingInlineAsm);
+  void onLabelParsed(MCSymbol *Symbol);
+
 };
 } // end anonymous namespace
 
@@ -7837,44 +7843,39 @@
   return false;
 }
 
+void ARMAsmParser::onLabelParsed(MCSymbol *Symbol) {
+  if (nextSymbolIsThumb) {
+    getParser().getStreamer().EmitThumbFunc(Symbol);
+    nextSymbolIsThumb = false;
+  }
+}
+
 /// parseDirectiveThumbFunc
 ///  ::= .thumbfunc symbol_name
 bool ARMAsmParser::parseDirectiveThumbFunc(SMLoc L) {
   const MCAsmInfo *MAI = getParser().getStreamer().getContext().getAsmInfo();
   bool isMachO = MAI->hasSubsectionsViaSymbols();
-  StringRef Name;
-  bool needFuncName = true;
 
   // Darwin asm has (optionally) function name after .thumb_func direction
   // ELF doesn't
   if (isMachO) {
     const AsmToken &Tok = Parser.getTok();
     if (Tok.isNot(AsmToken::EndOfStatement)) {
       if (Tok.isNot(AsmToken::Identifier) && Tok.isNot(AsmToken::String))
         return Error(L, "unexpected token in .thumb_func directive");
-      Name = Tok.getIdentifier();
+      MCSymbol *Func =
+          getParser().getContext().GetOrCreateSymbol(Tok.getIdentifier());
+      getParser().getStreamer().EmitThumbFunc(Func);
       Parser.Lex(); // Consume the identifier token.
-      needFuncName = false;
+      return false;
     }
   }
 
   if (getLexer().isNot(AsmToken::EndOfStatement))
     return Error(L, "unexpected token in directive");
 
-  // Eat the end of statement and any blank lines that follow.
-  while (getLexer().is(AsmToken::EndOfStatement))
-    Parser.Lex();
-
-  // FIXME: assuming function name will be the line following .thumb_func
-  // We really should be checking the next symbol definition even if there's
-  // stuff in between.
-  if (needFuncName) {
-    Name = Parser.getTok().getIdentifier();
-  }
+  nextSymbolIsThumb = true;
 
-  // Mark symbol as a thumb symbol.
-  MCSymbol *Func = getParser().getContext().GetOrCreateSymbol(Name);
-  getParser().getStreamer().EmitThumbFunc(Func);
   return false;
 }
 
Index: test/MC/ARM/elf-thumbfunc.s
===================================================================
--- test/MC/ARM/elf-thumbfunc.s
+++ test/MC/ARM/elf-thumbfunc.s
@@ -5,9 +5,9 @@
 	.text
 	.globl	foo
 	.align	2
-	.type	foo,%function
 	.code	16
 	.thumb_func
+	.type	foo,%function
 foo:
 	bx	lr
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2023.1.patch
Type: text/x-patch
Size: 4067 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131025/44275728/attachment.bin>