diff --git a/llvm/include/llvm/Support/FileCheck.h b/llvm/include/llvm/Support/FileCheck.h --- a/llvm/include/llvm/Support/FileCheck.h +++ b/llvm/include/llvm/Support/FileCheck.h @@ -102,8 +102,10 @@ llvm::Optional getVarValue(StringRef VarName); /// Define variables from definitions given on the command line passed as a - /// vector of VAR=VAL strings in \p CmdlineDefines. - void defineCmdlineVariables(std::vector &CmdlineDefines); + /// vector of VAR=VAL strings in \p CmdlineDefines. Report any error to \p SM + /// and return whether an error occured. + bool defineCmdlineVariables(std::vector &CmdlineDefines, + SourceMgr &SM); /// Undefine local variables (variables whose name does not start with a '$' /// sign), i.e. remove them from GlobalVariableTable. @@ -153,6 +155,13 @@ /// Returns the pointer to the global state for all patterns in this /// FileCheck instance. FileCheckPatternContext *getContext() const { return Context; } + /// Return whether \p is a valid first character for a variable name. + static bool isValidVarNameStart(char C); + /// Verify that the string at the start of \p Str is a well formed variable. + /// Return false if it is and set \p IsPseudo to indicate if it is a pseudo + /// variable and \p TrailIdx to the position of the last character that is + /// part of the variable name. Otherwise, only return true. + static bool parseVariable(StringRef Str, bool &IsPseudo, unsigned &TrailIdx); bool ParsePattern(StringRef PatternStr, StringRef Prefix, SourceMgr &SM, unsigned LineNumber, const FileCheckRequest &Req); size_t match(StringRef Buffer, size_t &MatchLen) const; diff --git a/llvm/lib/Support/FileCheck.cpp b/llvm/lib/Support/FileCheck.cpp --- a/llvm/lib/Support/FileCheck.cpp +++ b/llvm/lib/Support/FileCheck.cpp @@ -24,6 +24,38 @@ using namespace llvm; +bool FileCheckPattern::isValidVarNameStart(char C) { + return C == '_' || isalpha(C); +} + +bool FileCheckPattern::parseVariable(StringRef Str, bool &IsPseudo, + unsigned &TrailIdx) { + bool ParsedOneChar = false; + unsigned I = 0; + IsPseudo = false; + for (unsigned E = Str.size(); I != E; ++I) { + if (I == 0) { + // Global vars start with '$'. + if (Str[I] == '$') + continue; + else if (Str[I] == '@') { + IsPseudo = true; + continue; + } + } + if (!ParsedOneChar && !isValidVarNameStart(Str[I])) + return true; + + // Variable names are composed of alphanumeric characters and underscores. + if (Str[I] != '_' && !isalnum(Str[I])) + break; + ParsedOneChar = true; + } + + TrailIdx = I; + return false; +} + /// Parses the given string into the Pattern. /// /// \p Prefix provides which prefix is being matched, \p SM provides the @@ -117,9 +149,10 @@ // itself must be of the form "[a-zA-Z_][0-9a-zA-Z_]*", otherwise we reject // it. This is to catch some common errors. if (PatternStr.startswith("[[")) { + StringRef MatchStr = PatternStr.substr(2); // Find the closing bracket pair ending the match. End is going to be an // offset relative to the beginning of the match string. - size_t End = FindRegexVarEnd(PatternStr.substr(2), SM); + size_t End = FindRegexVarEnd(MatchStr, SM); if (End == StringRef::npos) { SM.PrintMessage(SMLoc::getFromPointer(PatternStr.data()), @@ -128,55 +161,44 @@ return true; } - StringRef MatchStr = PatternStr.substr(2, End); + MatchStr = MatchStr.substr(0, End); PatternStr = PatternStr.substr(End + 4); - // Get the regex name (e.g. "foo"). - size_t NameEnd = MatchStr.find(':'); - StringRef Name = MatchStr.substr(0, NameEnd); + // Get the regex name (e.g. "foo") and verify it is well formed. + bool IsPseudo; + unsigned TrailIdx; + if (parseVariable(MatchStr, IsPseudo, TrailIdx)) { + SM.PrintMessage(SMLoc::getFromPointer(MatchStr.data()), + SourceMgr::DK_Error, "invalid name in named regex"); + return true; + } + + StringRef Name = MatchStr.substr(0, TrailIdx); + StringRef Trailer = MatchStr.substr(TrailIdx); + bool IsVarDef = (Trailer.find(":") != StringRef::npos); - if (Name.empty()) { - SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error, - "invalid name in named regex: empty name"); + if (IsVarDef && (IsPseudo || !Trailer.consume_front(":"))) { + SM.PrintMessage(SMLoc::getFromPointer(MatchStr.data()), + SourceMgr::DK_Error, + "invalid name in named regex definition"); return true; } // Verify that the name/expression is well formed. FileCheck currently // supports @LINE, @LINE+number, @LINE-number expressions. The check here - // is relaxed, more strict check is performed in \c EvaluateExpression. - bool IsExpression = false; - for (unsigned i = 0, e = Name.size(); i != e; ++i) { - if (i == 0) { - if (Name[i] == '$') // Global vars start with '$' - continue; - if (Name[i] == '@') { - if (NameEnd != StringRef::npos) { - SM.PrintMessage(SMLoc::getFromPointer(Name.data()), - SourceMgr::DK_Error, - "invalid name in named regex definition"); - return true; - } - IsExpression = true; - continue; + // is relaxed. Stricter check is performed in \c EvaluateExpression. + if (IsPseudo) { + for (unsigned I = 0, E = Trailer.size(); I != E; ++I) { + if (!isalnum(Trailer[I]) && Trailer[I] != '+' && Trailer[I] != '-') { + SM.PrintMessage(SMLoc::getFromPointer(Name.data() + I), + SourceMgr::DK_Error, "invalid name in named regex"); + return true; } } - if (Name[i] != '_' && !isalnum(Name[i]) && - (!IsExpression || (Name[i] != '+' && Name[i] != '-'))) { - SM.PrintMessage(SMLoc::getFromPointer(Name.data() + i), - SourceMgr::DK_Error, "invalid name in named regex"); - return true; - } - } - - // Name can't start with a digit. - if (isdigit(static_cast(Name[0]))) { - SM.PrintMessage(SMLoc::getFromPointer(Name.data()), SourceMgr::DK_Error, - "invalid name in named regex"); - return true; } // Handle [[foo]]. - if (NameEnd == StringRef::npos) { + if (!IsVarDef) { // Handle variables that were defined earlier on the same line by // emitting a backreference. if (VariableDefs.find(Name) != VariableDefs.end()) { @@ -189,7 +211,7 @@ } AddBackrefToRegEx(VarParenNum); } else { - VariableUses.push_back(std::make_pair(Name, RegExStr.size())); + VariableUses.push_back(std::make_pair(MatchStr, RegExStr.size())); } continue; } @@ -199,7 +221,7 @@ RegExStr += '('; ++CurParen; - if (AddRegExToRegEx(MatchStr.substr(NameEnd + 1), CurParen, SM)) + if (AddRegExToRegEx(Trailer, CurParen, SM)) return true; RegExStr += ')'; @@ -755,7 +777,8 @@ bool llvm::FileCheck::ReadCheckFile( SourceMgr &SM, StringRef Buffer, Regex &PrefixRE, std::vector &CheckStrings) { - PatternContext.defineCmdlineVariables(Req.GlobalDefines); + if (PatternContext.defineCmdlineVariables(Req.GlobalDefines, SM)) + return true; std::vector ImplicitNegativeChecks; for (const auto &PatternString : Req.ImplicitCheckNot) { @@ -1374,12 +1397,36 @@ return Regex(PrefixRegexStr); } -void FileCheckPatternContext::defineCmdlineVariables( - std::vector &CmdlineDefines) { +bool FileCheckPatternContext::defineCmdlineVariables( + std::vector &CmdlineDefines, SourceMgr &SM) { assert(GlobalVariableTable.empty() && "Overriding defined variable with command-line variable definitions"); - for (StringRef CmdlineDef : CmdlineDefines) - GlobalVariableTable.insert(CmdlineDef.split('=')); + + bool ErrorFound = false; + for (const auto &CmdlineDef : CmdlineDefines) { + std::string Prefix = "-D"; + std::string DiagCmdline = Prefix + CmdlineDef; + std::unique_ptr CmdLine = + MemoryBuffer::getMemBufferCopy(DiagCmdline, "command line"); + StringRef CmdlineDefRef = CmdLine->getBuffer().substr(Prefix.size()); + SM.AddNewSourceBuffer(std::move(CmdLine), SMLoc()); + + bool IsPseudo; + unsigned TrailIdx; + std::pair CmdlineNameVal = CmdlineDefRef.split('='); + StringRef Name = CmdlineNameVal.first; + if (FileCheckPattern::parseVariable(Name, IsPseudo, TrailIdx) || IsPseudo || + TrailIdx != Name.size()) { + SM.PrintMessage(SMLoc::getFromPointer(CmdlineDefRef.data()), + SourceMgr::DK_Error, + "Invalid name for variable definition '" + Name + "'"); + ErrorFound = true; + continue; + } + GlobalVariableTable.insert(CmdlineNameVal); + } + + return ErrorFound; } void FileCheckPatternContext::clearLocalVars() { diff --git a/llvm/test/FileCheck/defines.txt b/llvm/test/FileCheck/defines.txt --- a/llvm/test/FileCheck/defines.txt +++ b/llvm/test/FileCheck/defines.txt @@ -1,6 +1,5 @@ ; RUN: FileCheck -DVALUE=10 -input-file %s %s ; RUN: not FileCheck -DVALUE=20 -input-file %s %s 2>&1 | FileCheck %s -check-prefix ERRMSG -; ; RUN: not FileCheck -DVALUE=10 -check-prefix NOT -input-file %s %s 2>&1 | FileCheck %s -check-prefix NOT-ERRMSG ; RUN: FileCheck -DVALUE=20 -check-prefix NOT -input-file %s %s ; RUN: not FileCheck -DVALUE10 -input-file %s %s 2>&1 | FileCheck %s -check-prefix ERRCLIEQ1 @@ -9,6 +8,9 @@ ; RUN: not FileCheck -D= -input-file %s %s 2>&1 | FileCheck %s -check-prefix ERRCLIVAR2 ; RUN: FileCheck -DVALUE= -check-prefix EMPTY -input-file %s %s 2>&1 +; RUN: not FileCheck -D10VALUE=10 -input-file %s %s 2>&1 | FileCheck %s -check-prefix ERRCLIFMT +; RUN: not FileCheck -D@VALUE=10 -input-file %s %s 2>&1 | FileCheck %s -check-prefix ERRCLIPSEUDO +; RUN: not FileCheck -D'VALUE + 2=10' -input-file %s %s 2>&1 | FileCheck %s -check-prefix ERRCLITRAIL Value = 10 ; CHECK: Value = [[VALUE]] ; NOT-NOT: Value = [[VALUE]] @@ -32,3 +34,15 @@ Empty value = @@ ; EMPTY: Empty value = @[[VALUE]]@ + +; ERRCLIFMT: command line:1:3: error: Invalid name for variable definition '10VALUE' +; ERRCLIFMT-NEXT: -D10VALUE=10 +; ERRCLIFMT-NEXT: {{^ \^$}} + +; ERRCLIPSEUDO: command line:1:3: error: Invalid name for variable definition '@VALUE' +; ERRCLIPSEUDO-NEXT: -D@VALUE=10 +; ERRCLIPSEUDO-NEXT: {{^ \^$}} + +; ERRCLITRAIL: command line:1:3: error: Invalid name for variable definition 'VALUE + 2' +; ERRCLITRAIL-NEXT: -DVALUE + 2=10 +; ERRCLITRAIL-NEXT: {{^ \^$}} diff --git a/llvm/unittests/Support/FileCheckTest.cpp b/llvm/unittests/Support/FileCheckTest.cpp --- a/llvm/unittests/Support/FileCheckTest.cpp +++ b/llvm/unittests/Support/FileCheckTest.cpp @@ -17,10 +17,11 @@ TEST_F(FileCheckTest, FileCheckContext) { FileCheckPatternContext Cxt; std::vector GlobalDefines; + SourceMgr SM; // Define local and global variables from command-line. GlobalDefines.emplace_back(std::string("LocalVar=FOO")); - Cxt.defineCmdlineVariables(GlobalDefines); + Cxt.defineCmdlineVariables(GlobalDefines, SM); // Check defined variables are present and undefined is absent. StringRef LocalVarStr = "LocalVar"; @@ -38,7 +39,7 @@ // Redefine global variables and check variables are defined again. GlobalDefines.emplace_back(std::string("$GlobalVar=BAR")); - Cxt.defineCmdlineVariables(GlobalDefines); + Cxt.defineCmdlineVariables(GlobalDefines, SM); StringRef GlobalVarStr = "$GlobalVar"; llvm::Optional GlobalVar = Cxt.getVarValue(GlobalVarStr); EXPECT_TRUE(GlobalVar); @@ -49,4 +50,80 @@ GlobalVar = Cxt.getVarValue(GlobalVarStr); EXPECT_TRUE(GlobalVar); } + +TEST_F(FileCheckTest, ValidVarNameStart) { + EXPECT_TRUE(FileCheckPattern::isValidVarNameStart('a')); + EXPECT_TRUE(FileCheckPattern::isValidVarNameStart('G')); + EXPECT_TRUE(FileCheckPattern::isValidVarNameStart('_')); + EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('2')); + EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('$')); + EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('@')); + EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('+')); + EXPECT_FALSE(FileCheckPattern::isValidVarNameStart('-')); + EXPECT_FALSE(FileCheckPattern::isValidVarNameStart(':')); +} + +TEST_F(FileCheckTest, ParseVar) { + StringRef VarName = "GoodVar42"; + bool IsPseudo = true; + unsigned TrailIdx = 0; + EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx)); + EXPECT_FALSE(IsPseudo); + EXPECT_EQ(TrailIdx, VarName.size()); + + VarName = "$GoodGlobalVar"; + IsPseudo = true; + TrailIdx = 0; + EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx)); + EXPECT_FALSE(IsPseudo); + EXPECT_EQ(TrailIdx, VarName.size()); + + VarName = "@GoodPseudoVar"; + IsPseudo = true; + TrailIdx = 0; + EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx)); + EXPECT_TRUE(IsPseudo); + EXPECT_EQ(TrailIdx, VarName.size()); + + VarName = "42BadVar"; + EXPECT_TRUE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx)); + + VarName = "$@"; + EXPECT_TRUE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx)); + + VarName = "B@dVar"; + IsPseudo = true; + TrailIdx = 0; + EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx)); + EXPECT_FALSE(IsPseudo); + EXPECT_EQ(TrailIdx, 1U); + + VarName = "B$dVar"; + IsPseudo = true; + TrailIdx = 0; + EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx)); + EXPECT_FALSE(IsPseudo); + EXPECT_EQ(TrailIdx, 1U); + + VarName = "BadVar+"; + IsPseudo = true; + TrailIdx = 0; + EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx)); + EXPECT_FALSE(IsPseudo); + EXPECT_EQ(TrailIdx, VarName.size() - 1); + + VarName = "BadVar-"; + IsPseudo = true; + TrailIdx = 0; + EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx)); + EXPECT_FALSE(IsPseudo); + EXPECT_EQ(TrailIdx, VarName.size() - 1); + + VarName = "BadVar:"; + IsPseudo = true; + TrailIdx = 0; + EXPECT_FALSE(FileCheckPattern::parseVariable(VarName, IsPseudo, TrailIdx)); + EXPECT_FALSE(IsPseudo); + EXPECT_EQ(TrailIdx, VarName.size() - 1); +} } // namespace