diff options
author | Nirav Dave <niravd@google.com> | 2016-09-13 13:55:06 +0000 |
---|---|---|
committer | Nirav Dave <niravd@google.com> | 2016-09-13 13:55:06 +0000 |
commit | 9fa8af21807e91ffbdf96ac3d9dde50cffeb7457 (patch) | |
tree | 2dac47e674dd60caf7bd765d907c19cd2caee339 /llvm/lib/MC/MCParser/AsmParser.cpp | |
parent | 7ea0d3947a070aed7c75ee253ae3addd252fe9f2 (diff) | |
download | bcm5719-llvm-9fa8af21807e91ffbdf96ac3d9dde50cffeb7457.tar.gz bcm5719-llvm-9fa8af21807e91ffbdf96ac3d9dde50cffeb7457.zip |
Defer asm errors to post-statement failure
Recommitting after fixing AsmParser Initialization.
Allow errors to be deferred and emitted as part of clean up to simplify
and shorten Assembly parser code. This will allow error messages to be
emitted in helper functions and be modified by the caller which has
better context.
As part of this many minor cleanups to the Parser:
* Unify parser cleanup on error
* Add Workaround for incorrect return values in ParseDirective instances
* Tighten checks on error-signifying return values for parser functions
and fix in-tree TargetParsers to be more consistent with the changes.
* Fix AArch64 test cases checking for spurious error messages that are
now fixed.
These changes should be backwards compatible with current Target Parsers
so long as the error status are correctly returned in appropriate
functions.
Reviewers: rnk, majnemer
Subscribers: aemerson, jyknight, llvm-commits
Differential Revision: https://reviews.llvm.org/D24047
llvm-svn: 281336
Diffstat (limited to 'llvm/lib/MC/MCParser/AsmParser.cpp')
-rw-r--r-- | llvm/lib/MC/MCParser/AsmParser.cpp | 254 |
1 files changed, 130 insertions, 124 deletions
diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp index a75818e99cd..b7ebf32991f 100644 --- a/llvm/lib/MC/MCParser/AsmParser.cpp +++ b/llvm/lib/MC/MCParser/AsmParser.cpp @@ -178,9 +178,6 @@ private: /// \brief Keeps track of how many .macro's have been instantiated. unsigned NumOfMacroInstantiations; - /// Flag tracking whether any errors have been encountered. - unsigned HadError : 1; - /// The values from the last parsed cpp hash file line comment if any. struct CppHashInfoTy { StringRef Filename; @@ -247,12 +244,9 @@ public: AssemblerDialect = i; } - void Note(SMLoc L, const Twine &Msg, - ArrayRef<SMRange> Ranges = None) override; - bool Warning(SMLoc L, const Twine &Msg, - ArrayRef<SMRange> Ranges = None) override; - bool Error(SMLoc L, const Twine &Msg, - ArrayRef<SMRange> Ranges = None) override; + void Note(SMLoc L, const Twine &Msg, SMRange Range = None) override; + bool Warning(SMLoc L, const Twine &Msg, SMRange Range = None) override; + bool printError(SMLoc L, const Twine &Msg, SMRange Range = None) override; const AsmToken &Lex() override; @@ -337,7 +331,8 @@ private: void printMacroInstantiations(); void printMessage(SMLoc Loc, SourceMgr::DiagKind Kind, const Twine &Msg, - ArrayRef<SMRange> Ranges = None) const { + SMRange Range = None) const { + ArrayRef<SMRange> Ranges(Range); SrcMgr.PrintMessage(Loc, Kind, Msg, Ranges); } static void DiagHandler(const SMDiagnostic &Diag, void *Context); @@ -565,8 +560,9 @@ AsmParser::AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out, const MCAsmInfo &MAI) : Lexer(MAI), Ctx(Ctx), Out(Out), MAI(MAI), SrcMgr(SM), PlatformParser(nullptr), CurBuffer(SM.getMainFileID()), - MacrosEnabledFlag(true), HadError(false), CppHashInfo(), - AssemblerDialect(~0U), IsDarwin(false), ParsingInlineAsm(false) { + MacrosEnabledFlag(true), CppHashInfo(), AssemblerDialect(~0U), + IsDarwin(false), ParsingInlineAsm(false) { + HadError = false; // Save the old handler. SavedDiagHandler = SrcMgr.getDiagHandler(); SavedDiagContext = SrcMgr.getDiagContext(); @@ -609,24 +605,25 @@ void AsmParser::printMacroInstantiations() { "while in macro instantiation"); } -void AsmParser::Note(SMLoc L, const Twine &Msg, ArrayRef<SMRange> Ranges) { - printMessage(L, SourceMgr::DK_Note, Msg, Ranges); +void AsmParser::Note(SMLoc L, const Twine &Msg, SMRange Range) { + printPendingErrors(); + printMessage(L, SourceMgr::DK_Note, Msg, Range); printMacroInstantiations(); } -bool AsmParser::Warning(SMLoc L, const Twine &Msg, ArrayRef<SMRange> Ranges) { +bool AsmParser::Warning(SMLoc L, const Twine &Msg, SMRange Range) { if(getTargetParser().getTargetOptions().MCNoWarn) return false; if (getTargetParser().getTargetOptions().MCFatalWarnings) - return Error(L, Msg, Ranges); - printMessage(L, SourceMgr::DK_Warning, Msg, Ranges); + return Error(L, Msg, Range); + printMessage(L, SourceMgr::DK_Warning, Msg, Range); printMacroInstantiations(); return false; } -bool AsmParser::Error(SMLoc L, const Twine &Msg, ArrayRef<SMRange> Ranges) { +bool AsmParser::printError(SMLoc L, const Twine &Msg, SMRange Range) { HadError = true; - printMessage(L, SourceMgr::DK_Error, Msg, Ranges); + printMessage(L, SourceMgr::DK_Error, Msg, Range); printMacroInstantiations(); return true; } @@ -731,32 +728,38 @@ bool AsmParser::Run(bool NoInitialTextSection, bool NoFinalize) { if (!parseStatement(Info, nullptr)) continue; - // If we've failed, but on a Error Token, but did not consume it in - // favor of a better message, emit it now. - if (Lexer.getTok().is(AsmToken::Error)) { + // If we have a Lexer Error we are on an Error Token. Load in Lexer Error + // for printing ErrMsg via Lex() only if no (presumably better) parser error + // exists. + if (!hasPendingError() && Lexer.getTok().is(AsmToken::Error)) { Lex(); } - // We had an error, validate that one was emitted and recover by skipping to - // the next line. - assert(HadError && "Parse statement returned an error, but none emitted!"); - eatToEndOfStatement(); + // parseStatement returned true so may need to emit an error. + printPendingErrors(); + + // Skipping to the next line if needed. + if (!getLexer().isAtStartOfStatement()) + eatToEndOfStatement(); } + // All errors should have been emitted. + assert(!hasPendingError() && "unexpected error from parseStatement"); + getTargetParser().flushPendingInstructions(getStreamer()); if (TheCondState.TheCond != StartingCondState.TheCond || TheCondState.Ignore != StartingCondState.Ignore) - return TokError("unmatched .ifs or .elses"); - + printError(getTok().getLoc(), "unmatched .ifs or .elses"); // Check to see there are no empty DwarfFile slots. const auto &LineTables = getContext().getMCDwarfLineTables(); if (!LineTables.empty()) { unsigned Index = 0; for (const auto &File : LineTables.begin()->second.getMCDwarfFiles()) { if (File.Name.empty() && Index != 0) - TokError("unassigned file number: " + Twine(Index) + - " for .file directives"); + printError(getTok().getLoc(), "unassigned file number: " + + Twine(Index) + + " for .file directives"); ++Index; } } @@ -776,9 +779,8 @@ bool AsmParser::Run(bool NoInitialTextSection, bool NoFinalize) { // FIXME: We would really like to refer back to where the symbol was // first referenced for a source location. We need to add something // to track that. Currently, we just point to the end of the file. - HadError |= - Error(getTok().getLoc(), "assembler local symbol '" + - Sym->getName() + "' not defined"); + printError(getTok().getLoc(), "assembler local symbol '" + + Sym->getName() + "' not defined"); } } @@ -789,7 +791,7 @@ bool AsmParser::Run(bool NoInitialTextSection, bool NoFinalize) { // Reset the state of any "# line file" directives we've seen to the // context as it was at the diagnostic site. CppHashInfo = std::get<1>(LocSym); - HadError |= Error(std::get<0>(LocSym), "directional label undefined"); + printError(std::get<0>(LocSym), "directional label undefined"); } } } @@ -804,7 +806,8 @@ bool AsmParser::Run(bool NoInitialTextSection, bool NoFinalize) { void AsmParser::checkForValidSection() { if (!ParsingInlineAsm && !getStreamer().getCurrentSection().first) { - TokError("expected section directive before assembly directive"); + printError(getTok().getLoc(), + "expected section directive before assembly directive"); Out.InitSections(false); } } @@ -1435,6 +1438,7 @@ bool AsmParser::parseBinOpRHS(unsigned Precedence, const MCExpr *&Res, /// ::= Label* Identifier OperandList* EndOfStatement bool AsmParser::parseStatement(ParseStatementInfo &Info, MCAsmParserSemaCallback *SI) { + assert(!hasPendingError() && "parseStatement started with pending error"); // Eat initial spaces and comments while (Lexer.is(AsmToken::Space)) Lex(); @@ -1467,15 +1471,19 @@ bool AsmParser::parseStatement(ParseStatementInfo &Info, if (Lexer.is(AsmToken::Integer)) { LocalLabelVal = getTok().getIntVal(); if (LocalLabelVal < 0) { - if (!TheCondState.Ignore) - return TokError("unexpected token at start of statement"); + if (!TheCondState.Ignore) { + Lex(); // always eat a token + return Error(IDLoc, "unexpected token at start of statement"); + } IDVal = ""; } else { IDVal = getTok().getString(); Lex(); // Consume the integer token to be used as an identifier token. if (Lexer.getKind() != AsmToken::Colon) { - if (!TheCondState.Ignore) - return TokError("unexpected token at start of statement"); + if (!TheCondState.Ignore) { + Lex(); // always eat a token + return Error(IDLoc, "unexpected token at start of statement"); + } } } } else if (Lexer.is(AsmToken::Dot)) { @@ -1492,8 +1500,10 @@ bool AsmParser::parseStatement(ParseStatementInfo &Info, Lex(); IDVal = "}"; } else if (parseIdentifier(IDVal)) { - if (!TheCondState.Ignore) - return TokError("unexpected token at start of statement"); + if (!TheCondState.Ignore) { + Lex(); // always eat a token + return Error(IDLoc, "unexpected token at start of statement"); + } IDVal = ""; } @@ -1655,9 +1665,20 @@ bool AsmParser::parseStatement(ParseStatementInfo &Info, getTargetParser().flushPendingInstructions(getStreamer()); - // First query the target-specific parser. It will return 'true' if it - // isn't interested in this directive. - if (!getTargetParser().ParseDirective(ID)) + SMLoc StartTokLoc = getTok().getLoc(); + bool TPDirectiveReturn = getTargetParser().ParseDirective(ID); + + if (hasPendingError()) + return true; + // Currently the return value should be true if we are + // uninterested but as this is at odds with the standard parsing + // convention (return true = error) we have instances of a parsed + // directive that fails returning true as an error. Catch these + // cases as best as possible errors here. + if (TPDirectiveReturn && StartTokLoc != getTok().getLoc()) + return true; + // Return if we did some parsing or believe we succeeded. + if (!TPDirectiveReturn || StartTokLoc != getTok().getLoc()) return false; // Next, check the extension directive map to see if any extension has @@ -1912,9 +1933,9 @@ bool AsmParser::parseStatement(ParseStatementInfo &Info, // Canonicalize the opcode to lower case. std::string OpcodeStr = IDVal.lower(); ParseInstructionInfo IInfo(Info.AsmRewrites); - bool HadError = getTargetParser().ParseInstruction(IInfo, OpcodeStr, ID, - Info.ParsedOperands); - Info.ParseError = HadError; + bool ParseHadError = getTargetParser().ParseInstruction(IInfo, OpcodeStr, ID, + Info.ParsedOperands); + Info.ParseError = ParseHadError; // Dump the parsed representation, if requested. if (getShowParsedOperands()) { @@ -1931,9 +1952,13 @@ bool AsmParser::parseStatement(ParseStatementInfo &Info, printMessage(IDLoc, SourceMgr::DK_Note, OS.str()); } + // Fail even if ParseInstruction erroneously returns false. + if (hasPendingError() || ParseHadError) + return true; + // If we are generating dwarf for the current section then generate a .loc // directive for the instruction. - if (!HadError && getContext().getGenDwarfForAssembly() && + if (!ParseHadError && getContext().getGenDwarfForAssembly() && getContext().getGenDwarfSectionSyms().count( getStreamer().getCurrentSection().first)) { unsigned Line; @@ -1975,15 +2000,13 @@ bool AsmParser::parseStatement(ParseStatementInfo &Info, } // If parsing succeeded, match the instruction. - if (!HadError) { + if (!ParseHadError) { uint64_t ErrorInfo; - getTargetParser().MatchAndEmitInstruction(IDLoc, Info.Opcode, - Info.ParsedOperands, Out, - ErrorInfo, ParsingInlineAsm); + if (getTargetParser().MatchAndEmitInstruction(IDLoc, Info.Opcode, + Info.ParsedOperands, Out, + ErrorInfo, ParsingInlineAsm)) + return true; } - - // Don't skip the rest of the line, the instruction parser is responsible for - // that. return false; } @@ -2019,6 +2042,7 @@ bool AsmParser::parseCppHashLineFilenameComment(SMLoc L) { "Lexing Cpp line comment: Expected String"); StringRef Filename = getTok().getString(); Lex(); + // Get rid of the enclosing quotes. Filename = Filename.substr(1, Filename.size() - 2); @@ -2353,27 +2377,19 @@ bool AsmParser::parseMacroArguments(const MCAsmMacro *M, MCAsmMacroParameter FA; if (Lexer.is(AsmToken::Identifier) && Lexer.peekTok().is(AsmToken::Equal)) { - if (parseIdentifier(FA.Name)) { - Error(IDLoc, "invalid argument identifier for formal argument"); - eatToEndOfStatement(); - return true; - } + if (parseIdentifier(FA.Name)) + return Error(IDLoc, "invalid argument identifier for formal argument"); + + if (Lexer.isNot(AsmToken::Equal)) + return TokError("expected '=' after formal parameter identifier"); - if (Lexer.isNot(AsmToken::Equal)) { - TokError("expected '=' after formal parameter identifier"); - eatToEndOfStatement(); - return true; - } Lex(); NamedParametersFound = true; } - if (NamedParametersFound && FA.Name.empty()) { - Error(IDLoc, "cannot mix positional and keyword arguments"); - eatToEndOfStatement(); - return true; - } + if (NamedParametersFound && FA.Name.empty()) + return Error(IDLoc, "cannot mix positional and keyword arguments"); bool Vararg = HasVararg && Parameter == (NParameters - 1); if (parseMacroArgument(FA.Value, Vararg)) @@ -2388,10 +2404,8 @@ bool AsmParser::parseMacroArguments(const MCAsmMacro *M, if (FAI >= NParameters) { assert(M && "expected macro to be defined"); - Error(IDLoc, - "parameter named '" + FA.Name + "' does not exist for macro '" + - M->Name + "'"); - return true; + return Error(IDLoc, "parameter named '" + FA.Name + + "' does not exist for macro '" + M->Name + "'"); } PI = FAI; } @@ -2992,11 +3006,14 @@ bool AsmParser::parseDirectiveAlign(bool IsPow2, unsigned ValueSize) { if (!HasFillExpr) FillExpr = 0; + // Always emit an alignment here even if we thrown an error. + bool ReturnVal = false; + // Compute alignment in bytes. if (IsPow2) { // FIXME: Diagnose overflow. if (Alignment >= 32) { - Error(AlignmentLoc, "invalid alignment value"); + ReturnVal |= Error(AlignmentLoc, "invalid alignment value"); Alignment = 31; } @@ -3008,13 +3025,14 @@ bool AsmParser::parseDirectiveAlign(bool IsPow2, unsigned ValueSize) { if (Alignment == 0) Alignment = 1; if (!isPowerOf2_64(Alignment)) - Error(AlignmentLoc, "alignment must be a power of 2"); + ReturnVal |= Error(AlignmentLoc, "alignment must be a power of 2"); } // Diagnose non-sensical max bytes to align. if (MaxBytesLoc.isValid()) { if (MaxBytesToFill < 1) { - Error(MaxBytesLoc, "alignment directive can never be satisfied in this " + ReturnVal |= Error(MaxBytesLoc, + "alignment directive can never be satisfied in this " "many bytes, ignoring maximum bytes expression"); MaxBytesToFill = 0; } @@ -3040,7 +3058,7 @@ bool AsmParser::parseDirectiveAlign(bool IsPow2, unsigned ValueSize) { MaxBytesToFill); } - return false; + return ReturnVal; } /// parseDirectiveFile @@ -3094,7 +3112,7 @@ bool AsmParser::parseDirectiveFile(SMLoc DirectiveLoc) { getContext().setGenDwarfForAssembly(false); else if (getStreamer().EmitDwarfFileDirective(FileNumber, Directory, Filename) == 0) - Error(FileNumberLoc, "file number already allocated"); + return Error(FileNumberLoc, "file number already allocated"); } return false; @@ -3346,7 +3364,7 @@ bool AsmParser::parseDirectiveCVInlineSiteId() { if (!getStreamer().EmitCVInlineSiteIdDirective(FunctionId, IAFunc, IAFile, IALine, IACol, FunctionIdLoc)) - Error(FunctionIdLoc, "function id already allocated"); + return Error(FunctionIdLoc, "function id already allocated"); return false; } @@ -4340,9 +4358,9 @@ bool AsmParser::parseDirectiveAbort() { return true; if (Str.empty()) - Error(Loc, ".abort detected. Assembly stopping."); + return Error(Loc, ".abort detected. Assembly stopping."); else - Error(Loc, ".abort '" + Str + "' detected. Assembly stopping."); + return Error(Loc, ".abort '" + Str + "' detected. Assembly stopping."); // FIXME: Actually abort assembly here. return false; @@ -4487,11 +4505,8 @@ bool AsmParser::parseDirectiveIfc(SMLoc DirectiveLoc, bool ExpectEqual) { bool AsmParser::parseDirectiveIfeqs(SMLoc DirectiveLoc, bool ExpectEqual) { if (Lexer.isNot(AsmToken::String)) { if (ExpectEqual) - TokError("expected string parameter for '.ifeqs' directive"); - else - TokError("expected string parameter for '.ifnes' directive"); - eatToEndOfStatement(); - return true; + return TokError("expected string parameter for '.ifeqs' directive"); + return TokError("expected string parameter for '.ifnes' directive"); } StringRef String1 = getTok().getStringContents(); @@ -4499,22 +4514,17 @@ bool AsmParser::parseDirectiveIfeqs(SMLoc DirectiveLoc, bool ExpectEqual) { if (Lexer.isNot(AsmToken::Comma)) { if (ExpectEqual) - TokError("expected comma after first string for '.ifeqs' directive"); - else - TokError("expected comma after first string for '.ifnes' directive"); - eatToEndOfStatement(); - return true; + return TokError( + "expected comma after first string for '.ifeqs' directive"); + return TokError("expected comma after first string for '.ifnes' directive"); } Lex(); if (Lexer.isNot(AsmToken::String)) { if (ExpectEqual) - TokError("expected string parameter for '.ifeqs' directive"); - else - TokError("expected string parameter for '.ifnes' directive"); - eatToEndOfStatement(); - return true; + return TokError("expected string parameter for '.ifeqs' directive"); + return TokError("expected string parameter for '.ifnes' directive"); } StringRef String2 = getTok().getStringContents(); @@ -4559,8 +4569,8 @@ bool AsmParser::parseDirectiveIfdef(SMLoc DirectiveLoc, bool expect_defined) { bool AsmParser::parseDirectiveElseIf(SMLoc DirectiveLoc) { if (TheCondState.TheCond != AsmCond::IfCond && TheCondState.TheCond != AsmCond::ElseIfCond) - Error(DirectiveLoc, "Encountered a .elseif that doesn't follow a .if or " - " an .elseif"); + return Error(DirectiveLoc, "Encountered a .elseif that doesn't follow an" + " .if or an .elseif"); TheCondState.TheCond = AsmCond::ElseIfCond; bool LastIgnoreState = false; @@ -4594,8 +4604,8 @@ bool AsmParser::parseDirectiveElse(SMLoc DirectiveLoc) { if (TheCondState.TheCond != AsmCond::IfCond && TheCondState.TheCond != AsmCond::ElseIfCond) - Error(DirectiveLoc, "Encountered a .else that doesn't follow a .if or an " - ".elseif"); + return Error(DirectiveLoc, "Encountered a .else that doesn't follow " + " an .if or an .elseif"); TheCondState.TheCond = AsmCond::ElseCond; bool LastIgnoreState = false; if (!TheCondStack.empty()) @@ -4637,18 +4647,14 @@ bool AsmParser::parseDirectiveError(SMLoc L, bool WithMessage) { StringRef Message = ".error directive invoked in source file"; if (Lexer.isNot(AsmToken::EndOfStatement)) { - if (Lexer.isNot(AsmToken::String)) { - TokError(".error argument must be a string"); - eatToEndOfStatement(); - return true; - } + if (Lexer.isNot(AsmToken::String)) + return TokError(".error argument must be a string"); Message = getTok().getStringContents(); Lex(); } - Error(L, Message); - return true; + return Error(L, Message); } /// parseDirectiveWarning @@ -4663,18 +4669,14 @@ bool AsmParser::parseDirectiveWarning(SMLoc L) { StringRef Message = ".warning directive invoked in source file"; if (Lexer.isNot(AsmToken::EndOfStatement)) { - if (Lexer.isNot(AsmToken::String)) { - TokError(".warning argument must be a string"); - eatToEndOfStatement(); - return true; - } + if (Lexer.isNot(AsmToken::String)) + return TokError(".warning argument must be a string"); Message = getTok().getStringContents(); Lex(); } - Warning(L, Message); - return false; + return Warning(L, Message); } /// parseDirectiveEndIf @@ -4685,8 +4687,8 @@ bool AsmParser::parseDirectiveEndIf(SMLoc DirectiveLoc) { return true; if ((TheCondState.TheCond == AsmCond::NoCond) || TheCondStack.empty()) - Error(DirectiveLoc, "Encountered a .endif that doesn't follow a .if or " - ".else"); + return Error(DirectiveLoc, "Encountered a .endif that doesn't follow " + "an .if or .else"); if (!TheCondStack.empty()) { TheCondState = TheCondStack.back(); TheCondStack.pop_back(); @@ -4838,7 +4840,7 @@ MCAsmMacro *AsmParser::parseMacroLikeBody(SMLoc DirectiveLoc) { while (true) { // Check whether we have reached the end of the file. if (getLexer().is(AsmToken::Eof)) { - Error(DirectiveLoc, "no matching '.endr' in definition"); + printError(DirectiveLoc, "no matching '.endr' in definition"); return nullptr; } @@ -4855,7 +4857,8 @@ MCAsmMacro *AsmParser::parseMacroLikeBody(SMLoc DirectiveLoc) { EndToken = getTok(); Lex(); if (Lexer.isNot(AsmToken::EndOfStatement)) { - TokError("unexpected token in '.endr' directive"); + printError(getTok().getLoc(), + "unexpected token in '.endr' directive"); return nullptr; } break; @@ -4905,7 +4908,6 @@ bool AsmParser::parseDirectiveRept(SMLoc DirectiveLoc, StringRef Dir) { int64_t Count; if (!CountExpr->evaluateAsAbsolute(Count)) { - eatToEndOfStatement(); return Error(CountLoc, "unexpected token in '" + Dir + "' directive"); } @@ -5108,11 +5110,16 @@ bool AsmParser::parseMSInlineAsm( continue; ParseStatementInfo Info(&AsmStrRewrites); - if (parseStatement(Info, &SI)) - return true; + bool StatementErr = parseStatement(Info, &SI); - if (Info.ParseError) + if (StatementErr || Info.ParseError) { + // Emit pending errors if any exist. + printPendingErrors(); return true; + } + + // No pending error should exist here. + assert(!hasPendingError() && "unexpected error from parseStatement"); if (Info.Opcode == ~0U) continue; @@ -5339,7 +5346,6 @@ bool parseAssignmentExpression(StringRef Name, bool allow_redef, if (Parser.parseExpression(Value)) { Parser.TokError("missing expression"); - Parser.eatToEndOfStatement(); return true; } |