NoQ updated this revision to Diff 143396. NoQ marked 3 inline comments as done. NoQ added a comment.
Address most comments. https://reviews.llvm.org/D45839 Files: include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h lib/StaticAnalyzer/Core/CallEvent.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp lib/StaticAnalyzer/Core/PathDiagnostic.cpp lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp test/Analysis/unified-sources/UnifiedSource-1.cpp test/Analysis/unified-sources/container.h test/Analysis/unified-sources/source1.cpp test/Analysis/unified-sources/source2.cpp
Index: test/Analysis/unified-sources/source2.cpp =================================================================== --- /dev/null +++ test/Analysis/unified-sources/source2.cpp @@ -0,0 +1,25 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s + +// This test tests that the warning is here when it is included from +// the unified sources file. The run-line in this file is there +// only to suppress LIT warning for the complete lack of run-line. +int testNullDereference() { + int *x = 0; + return *x; // expected-warning{{}} +} + +// Let's see if the container inlining heuristic still works. +class ContainerInCodeFile { + class Iterator { + }; + +public: + Iterator begin() const; + Iterator end() const; + + int method() { return 0; } +}; + +int testContainerMethodInCodeFile(ContainerInCodeFile Cont) { + return 1 / Cont.method(); // expected-warning{{}} +} Index: test/Analysis/unified-sources/source1.cpp =================================================================== --- /dev/null +++ test/Analysis/unified-sources/source1.cpp @@ -0,0 +1,15 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s + +// This test tests that the warning is here when it is included from +// the unified sources file. The run-line in this file is there +// only to suppress LIT warning for the complete lack of run-line. +int foo(int x) { + if (x) {} + return 1 / x; // expected-warning{{}} +} + +// Let's see if the container inlining heuristic still works. +#include "container.h" +int testContainerMethodInHeaderFile(ContainerInHeaderFile Cont) { + return 1 / Cont.method(); // no-warning +} Index: test/Analysis/unified-sources/container.h =================================================================== --- /dev/null +++ test/Analysis/unified-sources/container.h @@ -0,0 +1,10 @@ +class ContainerInHeaderFile { + class Iterator { + }; + +public: + Iterator begin() const; + Iterator end() const; + + int method() { return 0; } +}; Index: test/Analysis/unified-sources/UnifiedSource-1.cpp =================================================================== --- /dev/null +++ test/Analysis/unified-sources/UnifiedSource-1.cpp @@ -0,0 +1,5 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s + +// There should still be diagnostics within included files. +#include "source1.cpp" +#include "source2.cpp" Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp =================================================================== --- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -678,7 +678,7 @@ SourceLocation SL = Body ? Body->getLocStart() : D->getLocation(); SL = SM.getExpansionLoc(SL); - if (!Opts->AnalyzeAll && !SM.isWrittenInMainFile(SL)) { + if (!Opts->AnalyzeAll && !Mgr->isInCodeFile(SL)) { if (SL.isInvalid() || SM.isInSystemHeader(SL)) return AM_None; return Mode & ~AM_Path; Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp =================================================================== --- lib/StaticAnalyzer/Core/PathDiagnostic.cpp +++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp @@ -29,6 +29,7 @@ #include "clang/Basic/LLVM.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" #include "llvm/ADT/ArrayRef.h" @@ -148,11 +149,11 @@ if (CallLoc.isMacroID()) return nullptr; - assert(SMgr.isInMainFile(CallLoc) && - "The call piece should be in the main file."); + assert(AnalysisManager::isInCodeFile(CallLoc, SMgr) && + "The call piece should not be in a header file."); // Check if CP represents a path through a function outside of the main file. - if (!SMgr.isInMainFile(CP->callEnterWithin.asLocation())) + if (!AnalysisManager::isInCodeFile(CP->callEnterWithin.asLocation(), SMgr)) return CP; const PathPieces &Path = CP->path; Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -806,8 +806,9 @@ /// This checks static properties of the function, such as its signature and /// CFG, to determine whether the analyzer should ever consider inlining it, /// in any context. -static bool mayInlineDecl(AnalysisDeclContext *CalleeADC, - AnalyzerOptions &Opts) { +static bool mayInlineDecl(AnalysisManager &AMgr, + AnalysisDeclContext *CalleeADC) { + AnalyzerOptions &Opts = AMgr.getAnalyzerOptions(); // FIXME: Do not inline variadic calls. if (CallEvent::isVariadic(CalleeADC->getDecl())) return false; @@ -830,7 +831,7 @@ // Conditionally control the inlining of methods on objects that look // like C++ containers. if (!Opts.mayInlineCXXContainerMethods()) - if (!Ctx.getSourceManager().isInMainFile(FD->getLocation())) + if (!AMgr.isInCodeFile(FD->getLocation())) if (isContainerMethod(Ctx, FD)) return false; @@ -891,7 +892,7 @@ } else { // We haven't actually checked the static properties of this function yet. // Do that now, and record our decision in the function summaries. - if (mayInlineDecl(CalleeADC, Opts)) { + if (mayInlineDecl(getAnalysisManager(), CalleeADC)) { Engine.FunctionSummaries->markMayInline(D); } else { Engine.FunctionSummaries->markShouldNotInline(D); Index: lib/StaticAnalyzer/Core/CallEvent.cpp =================================================================== --- lib/StaticAnalyzer/Core/CallEvent.cpp +++ lib/StaticAnalyzer/Core/CallEvent.cpp @@ -938,15 +938,14 @@ bool ObjCMethodCall::canBeOverridenInSubclass(ObjCInterfaceDecl *IDecl, Selector Sel) const { assert(IDecl); - const SourceManager &SM = - getState()->getStateManager().getContext().getSourceManager(); - + AnalysisManager &AMgr = + getState()->getStateManager().getOwningEngine()->getAnalysisManager(); // If the class interface is declared inside the main file, assume it is not // subcassed. // TODO: It could actually be subclassed if the subclass is private as well. // This is probably very rare. SourceLocation InterfLoc = IDecl->getEndOfDefinitionLoc(); - if (InterfLoc.isValid() && SM.isInMainFile(InterfLoc)) + if (InterfLoc.isValid() && AMgr.isInCodeFile(InterfLoc)) return false; // Assume that property accessors are not overridden. @@ -968,7 +967,7 @@ return false; // If outside the main file, - if (D->getLocation().isValid() && !SM.isInMainFile(D->getLocation())) + if (D->getLocation().isValid() && !AMgr.isInCodeFile(D->getLocation())) return true; if (D->isOverriding()) { Index: include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h @@ -126,6 +126,36 @@ AnalysisDeclContext *getAnalysisDeclContext(const Decl *D) { return AnaCtxMgr.getContext(D); } + + static bool isInCodeFile(SourceLocation SL, const SourceManager &SM) { + if (SM.isInMainFile(SL)) + return true; + + // Support the "unified sources" compilation method (eg. WebKit) that + // involves producing non-header files that include other non-header files. + // We should be included directly from a UnifiedSource* file + // and we shouldn't be a header - which is a very safe defensive check. + SourceLocation IL = SM.getIncludeLoc(SM.getFileID(SL)); + if (!IL.isValid() || !SM.isInMainFile(IL)) + return false; + // Should rather be "file name starts with", but the current .getFilename + // includes the full path. + if (SM.getFilename(IL).contains("UnifiedSource")) { + StringRef Name = SM.getFilename(SL); + if (Name.endswith_lower(".c") || Name.endswith_lower(".cpp") || + Name.endswith_lower(".cc") || Name.endswith_lower(".cxx") || + Name.endswith_lower(".m") || Name.endswith_lower(".mm")) { + return true; + } + } + + return false; + } + + bool isInCodeFile(SourceLocation SL) { + const SourceManager &SM = getASTContext().getSourceManager(); + return isInCodeFile(SL, SM); + } }; } // enAnaCtxMgrspace
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits