From 80d21cb40dadc9b976a88143d50e0d900cd472a9 Mon Sep 17 00:00:00 2001 From: Pete Cooper Date: Mon, 22 Jun 2015 19:35:57 +0000 Subject: [PATCH] Change .thumb_set to have the same error checks as .set. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit According to the documentation, .thumb_set is 'the equivalent of a .set directive'. We didn't have equivalent behaviour in terms of all the errors we could throw, for example, when a symbol is redefined. This change refactors parseAssignment so that it can be used by .set and .thumb_set and implements tests for .thumb_set for all the errors thrown by that method. Reviewed by Rafael Espíndola. llvm-svn: 240318 --- llvm/include/llvm/MC/MCParser/MCAsmParserUtils.h | 27 ++++ llvm/include/llvm/MC/MCStreamer.h | 2 +- llvm/lib/MC/MCParser/AsmParser.cpp | 176 ++++++++++++++--------- llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp | 20 +-- llvm/test/MC/ARM/thumb_set-diagnostics.s | 28 ++++ llvm/test/MC/ARM/thumb_set.s | 2 - 6 files changed, 168 insertions(+), 87 deletions(-) create mode 100644 llvm/include/llvm/MC/MCParser/MCAsmParserUtils.h diff --git a/llvm/include/llvm/MC/MCParser/MCAsmParserUtils.h b/llvm/include/llvm/MC/MCParser/MCAsmParserUtils.h new file mode 100644 index 0000000..cb3d769 --- /dev/null +++ b/llvm/include/llvm/MC/MCParser/MCAsmParserUtils.h @@ -0,0 +1,27 @@ +//===------ llvm/MC/MCAsmParserUtils.h - Asm Parser Utilities ---*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_MC_MCPARSER_MCASMPARSERUTILS_H +#define LLVM_MC_MCPARSER_MCASMPARSERUTILS_H + +namespace llvm { +namespace MCParserUtils { + +/// Parse a value expression and return whether it can be assigned to a symbol +/// with the given name. +/// +/// On success, returns false and sets the Symbol and Value output parameters. +bool parseAssignmentExpression(StringRef Name, bool allow_redef, + MCAsmParser &Parser, MCSymbol *&Symbol, + const MCExpr *&Value); + +} // namespace MCParserUtils +} // namespace llvm + +#endif diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h index 50d8d31..6b9b8a1 100644 --- a/llvm/include/llvm/MC/MCStreamer.h +++ b/llvm/include/llvm/MC/MCStreamer.h @@ -78,7 +78,7 @@ public: MCTargetStreamer(MCStreamer &S); virtual ~MCTargetStreamer(); - const MCStreamer &getStreamer() { return Streamer; } + MCStreamer &getStreamer() { return Streamer; } // Allow a target to add behavior to the EmitLabel of MCStreamer. virtual void emitLabel(MCSymbol *Symbol); diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp index 9c1062f..02a4086 100644 --- a/llvm/lib/MC/MCParser/AsmParser.cpp +++ b/llvm/lib/MC/MCParser/AsmParser.cpp @@ -26,6 +26,7 @@ #include "llvm/MC/MCParser/AsmCond.h" #include "llvm/MC/MCParser/AsmLexer.h" #include "llvm/MC/MCParser/MCAsmParser.h" +#include "llvm/MC/MCParser/MCAsmParserUtils.h" #include "llvm/MC/MCParser/MCParsedAsmOperand.h" #include "llvm/MC/MCRegisterInfo.h" #include "llvm/MC/MCSectionMachO.h" @@ -2178,82 +2179,20 @@ void AsmParser::handleMacroExit() { ActiveMacros.pop_back(); } -static bool isUsedIn(const MCSymbol *Sym, const MCExpr *Value) { - switch (Value->getKind()) { - case MCExpr::Binary: { - const MCBinaryExpr *BE = static_cast(Value); - return isUsedIn(Sym, BE->getLHS()) || isUsedIn(Sym, BE->getRHS()); - } - case MCExpr::Target: - case MCExpr::Constant: - return false; - case MCExpr::SymbolRef: { - const MCSymbol &S = - static_cast(Value)->getSymbol(); - if (S.isVariable()) - return isUsedIn(Sym, S.getVariableValue()); - return &S == Sym; - } - case MCExpr::Unary: - return isUsedIn(Sym, static_cast(Value)->getSubExpr()); - } - - llvm_unreachable("Unknown expr kind!"); -} - bool AsmParser::parseAssignment(StringRef Name, bool allow_redef, bool NoDeadStrip) { - // FIXME: Use better location, we should use proper tokens. - SMLoc EqualLoc = Lexer.getLoc(); - + MCSymbol *Sym; const MCExpr *Value; - if (parseExpression(Value)) + if (MCParserUtils::parseAssignmentExpression(Name, allow_redef, *this, Sym, + Value)) return true; - // Note: we don't count b as used in "a = b". This is to allow - // a = b - // b = c - - if (Lexer.isNot(AsmToken::EndOfStatement)) - return TokError("unexpected token in assignment"); - - // Eat the end of statement marker. - Lex(); - - // Validate that the LHS is allowed to be a variable (either it has not been - // used as a symbol, or it is an absolute symbol). - MCSymbol *Sym = getContext().lookupSymbol(Name); - if (Sym) { - // Diagnose assignment to a label. - // - // FIXME: Diagnostics. Note the location of the definition as a label. - // FIXME: Diagnose assignment to protected identifier (e.g., register name). - if (isUsedIn(Sym, Value)) - return Error(EqualLoc, "Recursive use of '" + Name + "'"); - else if (Sym->isUndefined() && !Sym->isUsed() && !Sym->isVariable()) - ; // Allow redefinitions of undefined symbols only used in directives. - else if (Sym->isVariable() && !Sym->isUsed() && allow_redef) - ; // Allow redefinitions of variables that haven't yet been used. - else if (!Sym->isUndefined() && (!Sym->isVariable() || !allow_redef)) - return Error(EqualLoc, "redefinition of '" + Name + "'"); - else if (!Sym->isVariable()) - return Error(EqualLoc, "invalid assignment to '" + Name + "'"); - else if (!isa(Sym->getVariableValue())) - return Error(EqualLoc, "invalid reassignment of non-absolute variable '" + - Name + "'"); - - // Don't count these checks as uses. - Sym->setUsed(false); - } else if (Name == ".") { - if (Out.EmitValueToOffset(Value, 0)) { - Error(EqualLoc, "expected absolute expression"); - eatToEndOfStatement(); - } + if (!Sym) { + // In the case where we parse an expression starting with a '.', we will + // not generate an error, nor will we create a symbol. In this case we + // should just return out. return false; - } else - Sym = getContext().getOrCreateSymbol(Name); - - Sym->setRedefinable(allow_redef); + } // Do the assignment. Out.EmitAssignment(Sym, Value); @@ -4777,6 +4716,103 @@ bool AsmParser::parseMSInlineAsm( return false; } +namespace llvm { +namespace MCParserUtils { + +/// Returns whether the given symbol is used anywhere in the given expression, +/// or subexpressions. +static bool isSymbolUsedInExpression(const MCSymbol *Sym, const MCExpr *Value) { + switch (Value->getKind()) { + case MCExpr::Binary: { + const MCBinaryExpr *BE = static_cast(Value); + return isSymbolUsedInExpression(Sym, BE->getLHS()) || + isSymbolUsedInExpression(Sym, BE->getRHS()); + } + case MCExpr::Target: + case MCExpr::Constant: + return false; + case MCExpr::SymbolRef: { + const MCSymbol &S = + static_cast(Value)->getSymbol(); + if (S.isVariable()) + return isSymbolUsedInExpression(Sym, S.getVariableValue()); + return &S == Sym; + } + case MCExpr::Unary: + return isSymbolUsedInExpression( + Sym, static_cast(Value)->getSubExpr()); + } + + llvm_unreachable("Unknown expr kind!"); +} + +bool parseAssignmentExpression(StringRef Name, bool allow_redef, + MCAsmParser &Parser, MCSymbol *&Sym, + const MCExpr *&Value) { + MCAsmLexer &Lexer = Parser.getLexer(); + + // FIXME: Use better location, we should use proper tokens. + SMLoc EqualLoc = Lexer.getLoc(); + + if (Parser.parseExpression(Value)) { + Parser.TokError("missing expression"); + Parser.eatToEndOfStatement(); + return true; + } + + // Note: we don't count b as used in "a = b". This is to allow + // a = b + // b = c + + if (Lexer.isNot(AsmToken::EndOfStatement)) + return Parser.TokError("unexpected token in assignment"); + + // Eat the end of statement marker. + Parser.Lex(); + + // Validate that the LHS is allowed to be a variable (either it has not been + // used as a symbol, or it is an absolute symbol). + Sym = Parser.getContext().lookupSymbol(Name); + if (Sym) { + // Diagnose assignment to a label. + // + // FIXME: Diagnostics. Note the location of the definition as a label. + // FIXME: Diagnose assignment to protected identifier (e.g., register name). + if (isSymbolUsedInExpression(Sym, Value)) + return Parser.Error(EqualLoc, "Recursive use of '" + Name + "'"); + else if (Sym->isUndefined() && !Sym->isUsed() && !Sym->isVariable()) + ; // Allow redefinitions of undefined symbols only used in directives. + else if (Sym->isVariable() && !Sym->isUsed() && allow_redef) + ; // Allow redefinitions of variables that haven't yet been used. + else if (!Sym->isUndefined() && (!Sym->isVariable() || !allow_redef)) + return Parser.Error(EqualLoc, "redefinition of '" + Name + "'"); + else if (!Sym->isVariable()) + return Parser.Error(EqualLoc, "invalid assignment to '" + Name + "'"); + else if (!isa(Sym->getVariableValue())) + return Parser.Error(EqualLoc, + "invalid reassignment of non-absolute variable '" + + Name + "'"); + + // Don't count these checks as uses. + Sym->setUsed(false); + } else if (Name == ".") { + if (Parser.getStreamer().EmitValueToOffset(Value, 0)) { + Parser.Error(EqualLoc, "expected absolute expression"); + Parser.eatToEndOfStatement(); + return true; + } + return false; + } else + Sym = Parser.getContext().getOrCreateSymbol(Name); + + Sym->setRedefinable(allow_redef); + + return false; +} + +} // namespace MCParserUtils +} // namespace llvm + /// \brief Create an MCAsmParser instance. MCAsmParser *llvm::createMCAsmParser(SourceMgr &SM, MCContext &C, MCStreamer &Out, const MCAsmInfo &MAI) { diff --git a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp index 35387d3..c2db746 100644 --- a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp +++ b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp @@ -28,6 +28,7 @@ #include "llvm/MC/MCObjectFileInfo.h" #include "llvm/MC/MCParser/MCAsmLexer.h" #include "llvm/MC/MCParser/MCAsmParser.h" +#include "llvm/MC/MCParser/MCAsmParserUtils.h" #include "llvm/MC/MCParser/MCParsedAsmOperand.h" #include "llvm/MC/MCRegisterInfo.h" #include "llvm/MC/MCSection.h" @@ -9887,22 +9888,13 @@ bool ARMAsmParser::parseDirectiveThumbSet(SMLoc L) { } Lex(); + MCSymbol *Sym; const MCExpr *Value; - if (Parser.parseExpression(Value)) { - TokError("missing expression"); - Parser.eatToEndOfStatement(); - return false; - } - - if (getLexer().isNot(AsmToken::EndOfStatement)) { - TokError("unexpected token"); - Parser.eatToEndOfStatement(); - return false; - } - Lex(); + if (MCParserUtils::parseAssignmentExpression(Name, /* allow_redef */ true, + Parser, Sym, Value)) + return true; - MCSymbol *Alias = getContext().getOrCreateSymbol(Name); - getTargetStreamer().emitThumbSet(Alias, Value); + getTargetStreamer().emitThumbSet(Sym, Value); return false; } diff --git a/llvm/test/MC/ARM/thumb_set-diagnostics.s b/llvm/test/MC/ARM/thumb_set-diagnostics.s index 5f1844d..86f1ee5 100644 --- a/llvm/test/MC/ARM/thumb_set-diagnostics.s +++ b/llvm/test/MC/ARM/thumb_set-diagnostics.s @@ -41,3 +41,31 @@ @ CHECK: .thumb_set trailer_trash, 0x11fe1e55, @ CHECK: ^ + .type alpha,%function +alpha: + nop + + .type beta,%function +beta: + bkpt + + .thumb_set beta, alpha + +@ CHECK: error: redefinition of 'beta' +@ CHECK: .thumb_set beta, alpha +@ CHECK: ^ + + .type recursive_use,%function + .thumb_set recursive_use, recursive_use + 1 + +@ CHECK: error: Recursive use of 'recursive_use' +@ CHECK: .thumb_set recursive_use, recursive_use + 1 +@ CHECK: ^ + + variable_result = alpha + 1 + .long variable_result + .thumb_set variable_result, 1 + +@ CHECK: error: invalid reassignment of non-absolute variable 'variable_result' +@ CHECK: .thumb_set variable_result, 1 +@ CHECK: ^ \ No newline at end of file diff --git a/llvm/test/MC/ARM/thumb_set.s b/llvm/test/MC/ARM/thumb_set.s index d2a0dc0..00b3e53 100644 --- a/llvm/test/MC/ARM/thumb_set.s +++ b/llvm/test/MC/ARM/thumb_set.s @@ -54,8 +54,6 @@ alpha: nop .type beta,%function -beta: - bkpt .thumb_set beta, alpha -- 2.7.4