[analyzer] Be careful about implicitly-declared operator new/delete. (PR13090)

The implicit global allocation functions do not have valid source locations,
but we still want to treat them as being "system header" functions for the
purposes of how they affect program state.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@159160 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
index f01bad02c2033be0d735744e9feaecac0cb3cc63..8fc4050c7e0830e5ae05411afee79d7f9d526f90 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
@@ -199,10 +199,16 @@ public:
 
   /// Check if the callee is declared in the system header.
   bool isInSystemHeader() const {
-    if (const Decl *FD = getDecl()) {
+    if (const Decl *D = getDecl()) {
       const SourceManager &SM =
         State->getStateManager().getContext().getSourceManager();
-      return SM.isInSystemHeader(FD->getLocation());
+      SourceLocation Loc = D->getLocation();
+      // Be careful: the implicit declarations of operator new/delete have
+      // invalid source locations but should still count as system files.
+      if (Loc.isValid())
+        return SM.isInSystemHeader(D->getLocation());
+      else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
+        return FD->isOverloadedOperator() && FD->isImplicit() && FD->isGlobal();
     }
     return false;
   }

diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 50d4273e48c0a1dae4a33f3e297ee0090c130f22..1aa9b824e306e86c26695cd7c3a1a42c44ccb6f3 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1338,8 +1338,7 @@ bool MallocChecker::doesNotFreeMemory(const CallOrObjCMessage *Call,
   }
 
   // If it's not a system call, assume it frees memory.
-  SourceManager &SM = ASTC.getSourceManager();
-  if (!SM.isInSystemHeader(D->getLocation()))
+  if (!Call->isInSystemHeader())
     return false;
 
   // Process C/ObjC functions.

diff --git a/test/Analysis/new.cpp b/test/Analysis/new.cpp
index 8093eff3a1c2b33ab9a4fde3e7c3c34fecd0c42d..723d033bc794d63758bb28e6c93414ad56168ff4 100644 (file)
--- a/test/Analysis/new.cpp
+++ b/test/Analysis/new.cpp
@@ -5,6 +5,21 @@ void clang_analyzer_eval(bool);
 typedef typeof(sizeof(int)) size_t;
 extern "C" void *malloc(size_t);
 
+int someGlobal;
+void testImplicitlyDeclaredGlobalNew() {
+  if (someGlobal != 0)
+    return;
+
+  // This used to crash because the global operator new is being implicitly
+  // declared and it does not have a valid source location. (PR13090)
+  void *x = ::operator new(0);
+  ::operator delete(x);
+
+  // Check that the new/delete did not invalidate someGlobal;
+  clang_analyzer_eval(someGlobal == 0); // expected-warning{{TRUE}}
+}
+
+
 // This is the standard placement new.
 inline void* operator new(size_t, void* __p) throw()
 {