[analyzer] Enable analysis of WebKit "unified sources".

Normally the analyzer begins path-sensitive analysis from functions within
the main file, even though the path is allowed to go through any functions
within the translation unit.

When a recent version of WebKit is compiled, the "unified sources" technique
is used, that assumes #including multiple code files into a single main file.
Such file would have no functions defined in it, so the analyzer wouldn't be
able to find any entry points for path-sensitive analysis.

This patch pattern-matches unified file names that are similar to those
used by WebKit and allows the analyzer to find entry points in the included
code files. A more aggressive/generic approach is being planned as well.

Differential Revision: https://reviews.llvm.org/D45839

llvm-svn: 330876

9 files changed, 99 insertions, 13 deletions

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
index 15b930bc3f1..02b335761e7 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
@@ -126,6 +126,36 @@ public:
   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")) {
+      // It might be great to reuse FrontendOptions::getInputKindForExtension()
+      // but for now it doesn't discriminate between code and header files.
+      return llvm::StringSwitch<bool>(SM.getFilename(SL).rsplit('.').second)
+          .Cases("c", "m", "mm", "C", "cc", "cp", true)
+          .Cases("cpp", "CPP", "c++", "cxx", "cppm", true)
+          .Default(false);
+    }
+
+    return false;
+  }
+
+  bool isInCodeFile(SourceLocation SL) {
+    const SourceManager &SM = getASTContext().getSourceManager();
+    return isInCodeFile(SL, SM);
+  }
 };
 
 } // enAnaCtxMgrspace
diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index 259386cc48d..20fa63ef66b 100644
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -939,15 +939,14 @@ const ObjCPropertyDecl *ObjCMethodCall::getAccessedProperty() const {
 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.
@@ -969,7 +968,7 @@ bool ObjCMethodCall::canBeOverridenInSubclass(ObjCInterfaceDecl *IDecl,
       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()) {
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 689e3e56fda..6fafadda0d2 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -806,8 +806,9 @@ static bool isCXXSharedPtrDtor(const FunctionDecl *FD) {
 /// 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 @@ static bool mayInlineDecl(AnalysisDeclContext *CalleeADC,
       // 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 @@ bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
   } 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);
diff --git a/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
index fafedbb32b6..dcb197c9ca4 100644
--- a/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ b/clang/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 @@ getFirstStackedCallToHeaderFile(PathDiagnosticCallPiece *CP,
   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;
diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 6c379ed6e76..eee93d74b23 100644
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -678,7 +678,7 @@ AnalysisConsumer::getModeForDecl(Decl *D, AnalysisMode Mode) {
   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;
diff --git a/clang/test/Analysis/unified-sources/UnifiedSource-1.cpp b/clang/test/Analysis/unified-sources/UnifiedSource-1.cpp
new file mode 100644
index 00000000000..02569f247ee
--- /dev/null
+++ b/clang/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"
diff --git a/clang/test/Analysis/unified-sources/container.h b/clang/test/Analysis/unified-sources/container.h
new file mode 100644
index 00000000000..d0bcd778899
--- /dev/null
+++ b/clang/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; }
+};
diff --git a/clang/test/Analysis/unified-sources/source1.cpp b/clang/test/Analysis/unified-sources/source1.cpp
new file mode 100644
index 00000000000..886afed8d41
--- /dev/null
+++ b/clang/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
+}
diff --git a/clang/test/Analysis/unified-sources/source2.cpp b/clang/test/Analysis/unified-sources/source2.cpp
new file mode 100644
index 00000000000..cd85e352234
--- /dev/null
+++ b/clang/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{{}}
+}