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

Reply via email to