<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Oct 9, 2013, at 6:40 PM, Anna Zaks <<a href="mailto:ganna@apple.com">ganna@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Aemon, <div><br></div><div>Below are some high-level comments.</div><div><br></div><div>Thanks for working on this!</div><div>Anna.<br><div><div>On Oct 3, 2013, at 7:07 PM, Aemon Cannon <<a href="mailto:aemoncannon@gmail.com">aemoncannon@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div dir="ltr">I have a work-in-progress patch I'd like to share. Feedback appreciated.</div></blockquote><br>Generally, we send patches to cfe-dev.</div><div><br></div></div></div></blockquote><div><br></div>I ment to say "we send patches to cfe-commits mailing list (<a href="http://clang.llvm.org/hacking.html">http://clang.llvm.org/hacking.html</a>)".</div><div><br><blockquote type="cite" dir="auto"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div><div><blockquote type="cite" dir="auto"><div dir="ltr"><div><div>Goal: </div><div>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.</div></div><div><br></div><div>TODO:</div><div>* Silence warnings from code in blacklisted headers when that code is inlined into sources.</div><div>* Check case-sensitivity of the local filesystem and choose string Compare implementation accordingly.</div><div><br></div></div></blockquote><blockquote type="cite" dir="auto"><div dir="ltr"><div>Misc:</div><div>* I decided against the -isystem option because I wanted something more targeted that doesn't require modifying the build command.</div><div>* I'm not doing any path expansion or normalization, just a dumb string comparison. Is this sufficient?</div><div><br></div><div><br></div><div>Thanks!</div><div><br></div><div><div>diff --git a/include/clang/Driver/CC1Options.td b/include/clang/Driver/CC1Options.td</div><div>index 9acbd48..cc64c34 100644</div><div>--- a/include/clang/Driver/CC1Options.td</div><div>+++ b/include/clang/Driver/CC1Options.td</div><div>@@ -111,6 +111,9 @@ def analyzer_checker_help : Flag<["-"], "analyzer-checker-help">,</div><div> def analyzer_config : Separate<["-"], "analyzer-config">,</div><div> HelpText<"Choose analyzer options to enable">;</div><div> </div><div>+def analyzer_ignore_headers : Separate<["-"], "analyzer-ignore-headers">,</div><div>+ HelpText<"Ignore headers by prefix.">;</div><div>+</div></div></div></blockquote><div><br></div>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?</div><div><br><blockquote type="cite" dir="auto"><div dir="ltr"><div> //===----------------------------------------------------------------------===//</div><div> // Migrator Options</div><div> //===----------------------------------------------------------------------===//</div><div>diff --git a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h</div><div>index 618782e..45e75d1 100644</div><div>--- a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h</div><div>+++ b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h</div><div>@@ -122,6 +122,7 @@ public:</div><div> </div><div> /// \brief Pair of checker name and enable/disable.</div><div> std::vector<std::pair<std::string, bool> > CheckersControlList;</div><div>+ std::vector<std::string> HeaderBlacklist;</div><div> </div><div> /// \brief A key-value table of use-specified configuration values.</div><div> ConfigTable Config;</div><div>@@ -267,6 +268,8 @@ public:</div><div> /// \sa CXXMemberInliningMode</div><div> bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K);</div><div> </div><div>+ bool headerIsBlacklisted(StringRef IncludePath);</div><div>+</div><div> /// Returns true if ObjectiveC inlining is enabled, false otherwise.</div><div> bool mayInlineObjCMethod();</div><div> </div><div>diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp</div><div>index b0d7a0a..6ed2abc 100644</div><div>--- a/lib/Frontend/CompilerInvocation.cpp</div><div>+++ b/lib/Frontend/CompilerInvocation.cpp</div><div>@@ -280,6 +280,28 @@ static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args,</div><div> }</div><div> }</div><div> </div><div>+ // Go through the analyzer header blacklist.</div><div>+ for (arg_iterator it = Args.filtered_begin(OPT_analyzer_ignore_headers),</div><div>+ ie = Args.filtered_end(); it != ie; ++it) {</div><div>+ const Arg *A = *it;</div><div>+ A->claim();</div><div>+ StringRef headerList = A->getValue();</div><div>+ SmallVector<StringRef, 4> headerVals;</div><div>+ headerList.split(headerVals, ",");</div><div>+ for (unsigned i = 0, e = headerVals.size(); i != e; ++i) {</div><div>+ StringRef val = headerVals[i];</div><div>+ if (val.empty()) {</div><div>+ // TODO(aemon): properly report problem</div><div>+ // Diags.Report(SourceLocation(),</div><div>+ // diag::err_analyzer_header_no_value) << headerVals[i];</div><div>+ Success = false;</div><div>+ break;</div><div>+ }</div><div>+ Opts.HeaderBlacklist.push_back(val.str());</div><div>+ }</div><div>+ }</div><div>+ std::sort(Opts.HeaderBlacklist.begin(), Opts.HeaderBlacklist.end());</div><div>+</div><div> return Success;</div><div> }</div><div> </div><div>diff --git a/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt</div><div>index 392995e..123e84f 100644</div><div>--- a/lib/StaticAnalyzer/Checkers/CMakeLists.txt</div><div>+++ b/lib/StaticAnalyzer/Checkers/CMakeLists.txt</div><div>@@ -50,6 +50,8 @@ add_clang_library(clangStaticAnalyzerCheckers</div><div> ObjCMissingSuperCallChecker.cpp</div><div> ObjCSelfInitChecker.cpp</div><div> ObjCUnusedIVarsChecker.cpp</div><div>+ OwnershipChecker.cpp</div><div>+ TracingChecker.cpp</div><div> PointerArithChecker.cpp</div><div> PointerSubChecker.cpp</div><div> PthreadLockChecker.cpp</div><div>diff --git a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp</div><div>index 9dcf58b..3cfde27 100644</div><div>--- a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp</div><div>+++ b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp</div><div>@@ -96,6 +96,16 @@ AnalyzerOptions::mayInlineCXXMemberFunction(CXXInlineableMemberKind K) {</div><div> return CXXMemberInliningMode >= K;</div><div> }</div><div> </div><div>+bool AnalyzerOptions::headerIsBlacklisted(StringRef IncludePath) {</div><div>+ if (HeaderBlacklist.size() == 0) return false;</div><div>+ std::vector<std::string>::const_iterator SI =</div><div>+ lower_bound(HeaderBlacklist.begin(), HeaderBlacklist.end(),</div><div>+ IncludePath);</div><div>+ if (SI != HeaderBlacklist.end() && *SI == IncludePath) return true;</div><div>+ if (SI == HeaderBlacklist.begin()) return false;</div><div>+ return IncludePath.startswith(*(--SI));</div><div>+}</div><div>+</div><div> static StringRef toString(bool b) { return b ? "true" : "false"; }</div><div> </div><div> bool AnalyzerOptions::getBooleanOption(StringRef Name, bool DefaultVal) {</div><div>diff --git a/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp</div><div>index 9efe997..0a13906 100644</div><div>--- a/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp</div><div>+++ b/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp</div><div>@@ -414,13 +414,23 @@ void AnalysisConsumer::HandleTopLevelDeclInObjCContainer(DeclGroupRef DG) {</div><div> }</div><div> </div><div> void AnalysisConsumer::storeTopLevelDecls(DeclGroupRef DG) {</div><div>+ SourceManager &SM = Ctx->getSourceManager();</div><div> for (DeclGroupRef::iterator I = DG.begin(), E = DG.end(); I != E; ++I) {</div><div>-</div><div> // Skip ObjCMethodDecl, wait for the objc container to avoid</div><div> // analyzing twice.</div><div> if (isa<ObjCMethodDecl>(*I))</div><div> continue;</div><div> </div></div></blockquote><div><br></div><div>I'd add this to AnalysisConsumer::getModeForDecl(), where the analysis mode/policy is defined, instead of AnalysisConsumer::storeTopLevelDecls().</div></div><div><br><blockquote type="cite" dir="auto"><div dir="ltr"><div>+ SourceLocation SL = (*I)->getLocation();</div><div>+ if (!SM.isInMainFile(SL) && SL.isFileID()) {</div><div>+ if (const char* IncludePath = SM.getBufferName(SL)) {</div><div>+ StringRef S(IncludePath);</div><div>+ if (Opts->headerIsBlacklisted(S)) {</div><div>+ continue;</div><div>+ }</div><div>+ }</div><div>+ }</div><div>+</div><div> LocalTUDecls.push_back(*I);</div><div> }</div><div> }</div></div></blockquote><div><br></div>There is no test for the command line case.</div><div><br><blockquote type="cite" dir="auto"><div dir="ltr"><div><div>diff --git a/unittests/Frontend/AnalyzerOptionsTest.cpp b/unittests/Frontend/AnalyzerOptionsTest.cpp</div><div>new file mode 100644</div><div>index 0000000..489772c</div><div>--- /dev/null</div><div>+++ b/unittests/Frontend/AnalyzerOptionsTest.cpp</div><div>@@ -0,0 +1,57 @@</div><div>+//===- unittests/Frontend/AnalyzerOptionsTest.cpp - AnalyzerOptions tests ---===//</div><div>+//</div><div>+// The LLVM Compiler Infrastructure</div><div>+//</div><div>+// This file is distributed under the University of Illinois Open Source</div><div>+// License. See LICENSE.TXT for details.</div><div>+//</div><div>+//===----------------------------------------------------------------------===//</div><div>+</div><div>+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"</div><div>+#include "gtest/gtest.h"</div><div>+</div><div>+using namespace llvm;</div><div>+using namespace clang;</div><div>+</div><div>+namespace {</div><div>+</div><div>+TEST(AnalyzerOptionsTest, EmptyBlacklist) {</div><div>+ AnalyzerOptions options;</div><div>+ ASSERT_FALSE(options.headerIsBlacklisted("C:\\abc\\hello.h"));</div><div>+ ASSERT_FALSE(options.headerIsBlacklisted("bla.h"));</div><div>+}</div><div>+</div><div>+TEST(AnalyzerOptionsTest, Simple) {</div><div>+ AnalyzerOptions options;</div><div>+ options.HeaderBlacklist.push_back("C:\\abc\\");</div><div>+ options.HeaderBlacklist.push_back("/system/include");</div><div>+ std::sort(options.HeaderBlacklist.begin(), options.HeaderBlacklist.end());</div><div>+ ASSERT_TRUE(options.headerIsBlacklisted("C:\\abc\\hello.h"));</div><div>+ ASSERT_FALSE(options.headerIsBlacklisted("/system/hello.h"));</div><div>+}</div><div>+</div><div>+TEST(AnalyzerOptionsTest, LongerList) {</div><div>+ AnalyzerOptions options;</div><div>+ options.HeaderBlacklist.push_back("D:\\abc\\");</div><div>+ options.HeaderBlacklist.push_back("C:\\abc\\");</div><div>+ options.HeaderBlacklist.push_back("/var/log/system");</div><div>+ options.HeaderBlacklist.push_back("/system/include");</div><div>+ options.HeaderBlacklist.push_back("/home/tmp/");</div><div>+ options.HeaderBlacklist.push_back("../things/");</div><div>+ std::sort(options.HeaderBlacklist.begin(), options.HeaderBlacklist.end());</div><div>+ ASSERT_TRUE(options.headerIsBlacklisted("C:\\abc\\hello.h"));</div><div>+ ASSERT_TRUE(options.headerIsBlacklisted("/var/log/system/../hello"));</div><div>+ ASSERT_TRUE(options.headerIsBlacklisted("../things/foo.c"));</div><div>+ ASSERT_FALSE(options.headerIsBlacklisted("D:\\ab\\"));</div><div>+}</div><div>+</div><div>+TEST(AnalyzerOptionsTest, CaseSensitive) {</div><div>+ AnalyzerOptions options;</div><div>+ options.HeaderBlacklist.push_back("C:\\abc\\");</div><div>+ options.HeaderBlacklist.push_back("/system/include");</div><div>+ std::sort(options.HeaderBlacklist.begin(), options.HeaderBlacklist.end());</div><div>+ ASSERT_FALSE(options.headerIsBlacklisted("c:\\abc\\hello.h"));</div><div>+ ASSERT_FALSE(options.headerIsBlacklisted("/system/Include/hello.h"));</div><div>+}</div><div>+</div><div>+} // anonymous namespace</div><div>diff --git a/unittests/Frontend/CMakeLists.txt b/unittests/Frontend/CMakeLists.txt</div><div>index c65a163..39b4064 100644</div><div>--- a/unittests/Frontend/CMakeLists.txt</div><div>+++ b/unittests/Frontend/CMakeLists.txt</div><div>@@ -8,6 +8,7 @@ set(LLVM_LINK_COMPONENTS</div><div> </div><div> add_clang_unittest(FrontendTests</div><div> FrontendActionTest.cpp</div><div>+ AnalyzerOptionsTest.cpp</div><div> )</div><div> target_link_libraries(FrontendTests</div><div> clangFrontend</div></div><div><br></div><div><br></div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Sep 17, 2013 at 12:14 PM, Jordan Rose<span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;"><div class="im"><br><div><div>On Sep 16, 2013, at 22:34 , Ben Pope <<a href="mailto:benpope81@gmail.com" target="_blank">benpope81@gmail.com</a>> wrote:</div><br><blockquote type="cite">On 17/09/13 01:16, Anna Zaks wrote:<br><blockquote type="cite">We don't have a list of known third-party headers anywhere, but having this seems generally useful.<br></blockquote><br>Determining them based on -I vs -isystem would seem to be a sensible<br>distinction.<br><br>At least, that's what I use to include third party libraries when I want<br>to ignore warnings they produce. It's entirely possible I'm abusing<br>that feature.<br></blockquote><br></div></div><div>It's a bit of an abuse but likely not a harmful one. Huh, we don't actually have that mode now. Regular mode runs path-sensitive checks on the main source file and non-path-sensitive checks on the main source file + non-system headers. -analyzer-opt-analyze-headers mode runs path-sensitive checks everywhere. Patches welcome?</div><div><br></div><div>As for an explicit blacklist, we've gotten that request before, but only for main source files: <a href="http://llvm.org/bugs/show_bug.cgi?id=2706" target="_blank">PR2706</a> and <<a>rdar://problem/9063762</a>>. A header file filter would be slightly different.</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>Jordan</div></font></span></div><br>_______________________________________________<br>cfe-dev mailing list<br><a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br><br></blockquote></div><br></div>_______________________________________________<br>cfe-dev mailing list<br><a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br></blockquote></div><br></div>_______________________________________________<br>cfe-dev mailing list<br><a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a></div></blockquote></div><br></body></html>