Introduce factory methods for SpecialCaseList

Summary:
Doing work in constructors is bad: this change suggests to
call SpecialCaseList::create(Path, Error) instead of
"new SpecialCaseList(Path)". Currently the latter may crash with
report_fatal_error, which is undesirable - sometimes we want to report
the error to user gracefully - for example, if he provides an incorrect
file as an argument of Clang's -fsanitize-blacklist flag.

Reviewers: pcc

Reviewed By: pcc

CC: llvm-commits

Differential Revision: http://llvm-reviews.chandlerc.com/D1327

llvm-svn: 188156
This commit is contained in:
Alexey Samsonov 2013-08-12 07:49:36 +00:00
parent feb34713d5
commit 9e4fdd2656
3 changed files with 92 additions and 13 deletions

View File

@ -57,8 +57,18 @@ class StringRef;
class SpecialCaseList { class SpecialCaseList {
public: public:
// FIXME: Switch all users to factories and remove these constructors.
SpecialCaseList(const StringRef Path); SpecialCaseList(const StringRef Path);
SpecialCaseList(const MemoryBuffer *MB); SpecialCaseList(const MemoryBuffer *MB);
/// Parses the special case list from a file. If Path is empty, returns
/// an empty special case list. On failure, returns 0 and writes an error
/// message to string.
static SpecialCaseList *create(const StringRef Path, std::string &Error);
/// Parses the special case list from a memory buffer. On failure, returns
/// 0 and writes an error message to string.
static SpecialCaseList *create(const MemoryBuffer *MB, std::string &Error);
~SpecialCaseList(); ~SpecialCaseList();
/// Returns whether either this function or its source file are listed in the /// Returns whether either this function or its source file are listed in the
@ -89,10 +99,16 @@ class SpecialCaseList {
bool findCategory(const Module &M, StringRef &Category) const; bool findCategory(const Module &M, StringRef &Category) const;
private: private:
SpecialCaseList(SpecialCaseList const &) LLVM_DELETED_FUNCTION;
SpecialCaseList &operator=(SpecialCaseList const &) LLVM_DELETED_FUNCTION;
struct Entry; struct Entry;
StringMap<StringMap<Entry> > Entries; StringMap<StringMap<Entry> > Entries;
void init(const MemoryBuffer *MB); SpecialCaseList();
/// Parses just-constructed SpecialCaseList entries from a memory buffer.
bool parse(const MemoryBuffer *MB, std::string &Error);
bool findCategory(const StringRef Section, const StringRef Query, bool findCategory(const StringRef Section, const StringRef Query,
StringRef &Category) const; StringRef &Category) const;
bool inSectionCategory(const StringRef Section, const StringRef Query, bool inSectionCategory(const StringRef Section, const StringRef Query,

View File

@ -49,29 +49,58 @@ struct SpecialCaseList::Entry {
} }
}; };
SpecialCaseList::SpecialCaseList() : Entries() {}
SpecialCaseList::SpecialCaseList(const StringRef Path) { SpecialCaseList::SpecialCaseList(const StringRef Path) {
// Validate and open blacklist file. // Validate and open blacklist file.
if (Path.empty()) return; if (Path.empty()) return;
OwningPtr<MemoryBuffer> File; OwningPtr<MemoryBuffer> File;
if (error_code EC = MemoryBuffer::getFile(Path, File)) { if (error_code EC = MemoryBuffer::getFile(Path, File)) {
report_fatal_error("Can't open blacklist file: " + Path + ": " + report_fatal_error("Can't open file '" + Path + "': " +
EC.message()); EC.message());
} }
init(File.get()); std::string Error;
if (!parse(File.get(), Error))
report_fatal_error(Error);
} }
SpecialCaseList::SpecialCaseList(const MemoryBuffer *MB) { SpecialCaseList::SpecialCaseList(const MemoryBuffer *MB) {
init(MB); std::string Error;
if (!parse(MB, Error))
report_fatal_error(Error);
} }
void SpecialCaseList::init(const MemoryBuffer *MB) { SpecialCaseList *SpecialCaseList::create(
const StringRef Path, std::string &Error) {
if (Path.empty())
return new SpecialCaseList();
OwningPtr<MemoryBuffer> File;
if (error_code EC = MemoryBuffer::getFile(Path, File)) {
Error = (Twine("Can't open file '") + Path + "': " + EC.message()).str();
return 0;
}
return create(File.get(), Error);
}
SpecialCaseList *SpecialCaseList::create(
const MemoryBuffer *MB, std::string &Error) {
OwningPtr<SpecialCaseList> SCL(new SpecialCaseList());
if (!SCL->parse(MB, Error))
return 0;
return SCL.take();
}
bool SpecialCaseList::parse(const MemoryBuffer *MB, std::string &Error) {
// Iterate through each line in the blacklist file. // Iterate through each line in the blacklist file.
SmallVector<StringRef, 16> Lines; SmallVector<StringRef, 16> Lines;
SplitString(MB->getBuffer(), Lines, "\n\r"); SplitString(MB->getBuffer(), Lines, "\n\r");
StringMap<StringMap<std::string> > Regexps; StringMap<StringMap<std::string> > Regexps;
assert(Entries.empty() &&
"parse() should be called on an empty SpecialCaseList");
int LineNo = 1;
for (SmallVectorImpl<StringRef>::iterator I = Lines.begin(), E = Lines.end(); for (SmallVectorImpl<StringRef>::iterator I = Lines.begin(), E = Lines.end();
I != E; ++I) { I != E; ++I, ++LineNo) {
// Ignore empty lines and lines starting with "#" // Ignore empty lines and lines starting with "#"
if (I->empty() || I->startswith("#")) if (I->empty() || I->startswith("#"))
continue; continue;
@ -80,7 +109,9 @@ void SpecialCaseList::init(const MemoryBuffer *MB) {
StringRef Prefix = SplitLine.first; StringRef Prefix = SplitLine.first;
if (SplitLine.second.empty()) { if (SplitLine.second.empty()) {
// Missing ':' in the line. // Missing ':' in the line.
report_fatal_error("malformed blacklist line: " + SplitLine.first); Error = (Twine("Malformed line ") + Twine(LineNo) + ": '" +
SplitLine.first + "'").str();
return false;
} }
std::pair<StringRef, StringRef> SplitRegexp = SplitLine.second.split("="); std::pair<StringRef, StringRef> SplitRegexp = SplitLine.second.split("=");
@ -113,10 +144,11 @@ void SpecialCaseList::init(const MemoryBuffer *MB) {
// Check that the regexp is valid. // Check that the regexp is valid.
Regex CheckRE(Regexp); Regex CheckRE(Regexp);
std::string Error; std::string REError;
if (!CheckRE.isValid(Error)) { if (!CheckRE.isValid(REError)) {
report_fatal_error("malformed blacklist regex: " + SplitLine.second + Error = (Twine("Malformed regex in line ") + Twine(LineNo) + ": '" +
": " + Error); SplitLine.second + "': " + REError).str();
return false;
} }
// Add this regexp into the proper group by its prefix. // Add this regexp into the proper group by its prefix.
@ -135,6 +167,7 @@ void SpecialCaseList::init(const MemoryBuffer *MB) {
Entries[I->getKey()][II->getKey()].RegEx = new Regex(II->getValue()); Entries[I->getKey()][II->getKey()].RegEx = new Regex(II->getValue());
} }
} }
return true;
} }
SpecialCaseList::~SpecialCaseList() { SpecialCaseList::~SpecialCaseList() {

View File

@ -34,9 +34,17 @@ protected:
M, ST, false, GlobalValue::ExternalLinkage, 0, Name); M, ST, false, GlobalValue::ExternalLinkage, 0, Name);
} }
SpecialCaseList *makeSpecialCaseList(StringRef List) { SpecialCaseList *makeSpecialCaseList(StringRef List, std::string &Error) {
OwningPtr<MemoryBuffer> MB(MemoryBuffer::getMemBuffer(List)); OwningPtr<MemoryBuffer> MB(MemoryBuffer::getMemBuffer(List));
return new SpecialCaseList(MB.get()); return SpecialCaseList::create(MB.get(), Error);
}
SpecialCaseList *makeSpecialCaseList(StringRef List) {
std::string Error;
SpecialCaseList *SCL = makeSpecialCaseList(List, Error);
assert(SCL);
assert(Error == "");
return SCL;
} }
LLVMContext Ctx; LLVMContext Ctx;
@ -155,4 +163,26 @@ TEST_F(SpecialCaseListTest, Substring) {
EXPECT_TRUE(SCL->isIn(*F)); EXPECT_TRUE(SCL->isIn(*F));
} }
TEST_F(SpecialCaseListTest, InvalidSpecialCaseList) {
std::string Error;
EXPECT_EQ(0, makeSpecialCaseList("badline", Error));
EXPECT_EQ("Malformed line 1: 'badline'", Error);
EXPECT_EQ(0, makeSpecialCaseList("src:bad[a-", Error));
EXPECT_EQ("Malformed regex in line 1: 'bad[a-': invalid character range",
Error);
EXPECT_EQ(0, makeSpecialCaseList("src:a.c\n"
"fun:fun(a\n",
Error));
EXPECT_EQ("Malformed regex in line 2: 'fun(a': parentheses not balanced",
Error);
EXPECT_EQ(0, SpecialCaseList::create("unexisting", Error));
EXPECT_EQ("Can't open file 'unexisting': No such file or directory", Error);
}
TEST_F(SpecialCaseListTest, EmptySpecialCaseList) {
OwningPtr<SpecialCaseList> SCL(makeSpecialCaseList(""));
Module M("foo", Ctx);
EXPECT_FALSE(SCL->isIn(M));
}
} }