Hi Aemon,
Below are some high-level comments.
Thanks for working on this!
Anna.
I have a work-in-progress patch I’d like to share. Feedback appreciated.
Generally, we send patches to cfe-dev.
Goal:
I want to be able to analyze the code in my headers while ignoring code from third-party headers that are included in my sources. Adds a new analyzer option, analyzer-ignore-headers, for explicitly blacklisting headers by prefix.
TODO:
- Silence warnings from code in blacklisted headers when that code is inlined into sources.
- Check case-sensitivity of the local filesystem and choose string Compare implementation accordingly.
Misc:
- I decided against the -isystem option because I wanted something more targeted that doesn’t require modifying the build command.
- I’m not doing any path expansion or normalization, just a dumb string comparison. Is this sufficient?
Thanks!
diff --git a/include/clang/Driver/CC1Options.td b/include/clang/Driver/CC1Options.td
index 9acbd48…cc64c34 100644
— a/include/clang/Driver/CC1Options.td
+++ b/include/clang/Driver/CC1Options.td
@@ -111,6 +111,9 @@ def analyzer_checker_help : Flag<["-"], “analyzer-checker-help”>,
def analyzer_config : Separate<["-"], “analyzer-config”>,
HelpText<“Choose analyzer options to enable”>;
+def analyzer_ignore_headers : Separate<["-"], “analyzer-ignore-headers”>,
- HelpText<“Ignore headers by prefix.”>;
-
We are moving toward not exposing/implementing analyzer options support in the clang Driver code, but rather keeping it within the analyzer. You can see that some of the options are implemented that way (for example, see AnalyzerOptions::mayInlineCXXMemberFunction). Would it be possible to implement “analyzer-ignore-headers” in the same way?
//===----------------------------------------------------------------------===//
// Migrator Options
//===----------------------------------------------------------------------===//
diff --git a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 618782e…45e75d1 100644
— a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -122,6 +122,7 @@ public:
/// \brief Pair of checker name and enable/disable.
std::vector<std::pair<std::string, bool> > CheckersControlList;
/// \brief A key-value table of use-specified configuration values.
ConfigTable Config;
@@ -267,6 +268,8 @@ public:
/// \sa CXXMemberInliningMode
bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K);
- bool headerIsBlacklisted(StringRef IncludePath);
-
/// Returns true if ObjectiveC inlining is enabled, false otherwise.
bool mayInlineObjCMethod();
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
index b0d7a0a…6ed2abc 100644
— a/lib/Frontend/CompilerInvocation.cpp
+++ b/lib/Frontend/CompilerInvocation.cpp
@@ -280,6 +280,28 @@ static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args,
}
}
- // Go through the analyzer header blacklist.
- for (arg_iterator it = Args.filtered_begin(OPT_analyzer_ignore_headers),
- ie = Args.filtered_end(); it != ie; ++it) {
- const Arg *A = *it;
- A->claim();
- StringRef headerList = A->getValue();
- SmallVector<StringRef, 4> headerVals;
- headerList.split(headerVals, “,”);
- for (unsigned i = 0, e = headerVals.size(); i != e; ++i) {
- StringRef val = headerVals[i];
- if (val.empty()) {
- // TODO(aemon): properly report problem
- // Diags.Report(SourceLocation(),
- // diag::err_analyzer_header_no_value) << headerVals[i];
- Success = false;
- break;
- }
- Opts.HeaderBlacklist.push_back(val.str());
- }
- }
- std::sort(Opts.HeaderBlacklist.begin(), Opts.HeaderBlacklist.end());
-
return Success;
}
diff --git a/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 392995e…123e84f 100644
— a/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -50,6 +50,8 @@ add_clang_library(clangStaticAnalyzerCheckers
ObjCMissingSuperCallChecker.cpp
ObjCSelfInitChecker.cpp
ObjCUnusedIVarsChecker.cpp
- OwnershipChecker.cpp
- TracingChecker.cpp
PointerArithChecker.cpp
PointerSubChecker.cpp
PthreadLockChecker.cpp
diff --git a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
index 9dcf58b…3cfde27 100644
— a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -96,6 +96,16 @@ AnalyzerOptions::mayInlineCXXMemberFunction(CXXInlineableMemberKind K) {
return CXXMemberInliningMode >= K;
}
+bool AnalyzerOptions::headerIsBlacklisted(StringRef IncludePath) {
- if (HeaderBlacklist.size() == 0) return false;
- std::vectorstd::string::const_iterator SI =
- lower_bound(HeaderBlacklist.begin(), HeaderBlacklist.end(),
- IncludePath);
- if (SI != HeaderBlacklist.end() && *SI == IncludePath) return true;
- if (SI == HeaderBlacklist.begin()) return false;
- return IncludePath.startswith(*(–SI));
+}
-
static StringRef toString(bool b) { return b ? “true” : “false”; }
bool AnalyzerOptions::getBooleanOption(StringRef Name, bool DefaultVal) {
diff --git a/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 9efe997…0a13906 100644
— a/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -414,13 +414,23 @@ void AnalysisConsumer::HandleTopLevelDeclInObjCContainer(DeclGroupRef DG) {
}
void AnalysisConsumer::storeTopLevelDecls(DeclGroupRef DG) {
- SourceManager &SM = Ctx->getSourceManager();
for (DeclGroupRef::iterator I = DG.begin(), E = DG.end(); I != E; ++I) {
// Skip ObjCMethodDecl, wait for the objc container to avoid
// analyzing twice.
if (isa(*I))
continue;
I’d add this to AnalysisConsumer::getModeForDecl(), where the analysis mode/policy is defined, instead of AnalysisConsumer::storeTopLevelDecls().
- SourceLocation SL = (*I)->getLocation();
- if (!SM.isInMainFile(SL) && SL.isFileID()) {
- if (const char* IncludePath = SM.getBufferName(SL)) {
- StringRef S(IncludePath);
- if (Opts->headerIsBlacklisted(S)) {
- continue;
- }
- }
- }
-
LocalTUDecls.push_back(*I);
}
}
There is no test for the command line case.