[analyzer] Don't inline constructors for objects allocated with operator new.

Because the CXXNewExpr appears after the CXXConstructExpr in the CFG, we don't
actually have the correct region to construct into at the time we decide
whether or not to inline. The long-term fix (discussed in PR12014) might be to
introduce a new CFG node (CFGAllocator) that appears before the constructor.

Tracking the short-term fix in <rdar://problem/12180598>.

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

docs/analyzer/IPA.txt

@@ -103,7 +103,8 @@ some cases, however, where the analyzer chooses not to inline:
   See "C++ Caveats" below.
 
 - In C++, ExprEngine does not inline custom implementations of operator 'new'
-  or operator 'delete'. See "C++ Caveats" below.
+  or operator 'delete', nor does it inline the constructors and destructors
+  associated with these. See "C++ Caveats" below.
 
 - Calls resulting in "dynamic dispatch" are specially handled.  See more below.
 

lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

@@ -18,6 +18,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/ParentMap.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Support/SaveAndRestore.h"
@@ -350,6 +351,15 @@ bool ExprEngine::inlineCall(const CallEvent &Call, const Decl *D,
     if (Target && isa<ElementRegion>(Target))
       return false;
 
+    // FIXME: This is a hack. We don't use the correct region for a new
+    // expression, so if we inline the constructor its result will just be
+    // thrown away. This short-term hack is tracked in <rdar://problem/12180598>
+    // and the longer-term possible fix is discussed in PR12014.
+    const CXXConstructExpr *CtorExpr = Ctor.getOriginExpr();
+    if (const Stmt *Parent = CurLC->getParentMap().getParent(CtorExpr))
+      if (isa<CXXNewExpr>(Parent))
+        return false;
+
     // If the destructor is trivial, it's always safe to inline the constructor.
     if (Ctor.getDecl()->getParent()->hasTrivialDestructor())
       break;
@@ -363,7 +373,6 @@ bool ExprEngine::inlineCall(const CallEvent &Call, const Decl *D,
 
     // FIXME: This is a hack. We don't handle temporary destructors
     // right now, so we shouldn't inline their constructors.
-    const CXXConstructExpr *CtorExpr = Ctor.getOriginExpr();
     if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete)
       if (!Target || !isa<DeclRegion>(Target))
         return false;

test/Analysis/inline.cpp

@@ -1,8 +1,18 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-ipa=inlining -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-ipa=inlining -verify %s
 
 void clang_analyzer_eval(bool);
 void clang_analyzer_checkInlined(bool);
 
+typedef __typeof__(sizeof(int)) size_t;
+extern "C" void *malloc(size_t);
+
+// This is the standard placement new.
+inline void* operator new(size_t, void* __p) throw()
+{
+  return __p;
+}
+
+
 class A {
 public:
   int getZero() { return 0; }
@@ -227,3 +237,33 @@ namespace DefaultArgs {
     clang_analyzer_eval(obj.get() == 42); // expected-warning{{UNKNOWN}}
   }
 }
+
+namespace OperatorNew {
+  class IntWrapper {
+  public:
+    int value;
+
+    IntWrapper(int input) : value(input) {
+      // We don't want this constructor to be inlined unless we can actually
+      // use the proper region for operator new.
+      // See PR12014 and <rdar://problem/12180598>.
+      clang_analyzer_checkInlined(false); // no-warning
+    }
+  };
+
+  void test() {
+    IntWrapper *obj = new IntWrapper(42);
+    // should be TRUE
+    clang_analyzer_eval(obj->value == 42); // expected-warning{{UNKNOWN}}
+  }
+
+  void testPlacement() {
+    IntWrapper *obj = static_cast<IntWrapper *>(malloc(sizeof(IntWrapper)));
+    IntWrapper *alias = new (obj) IntWrapper(42);
+
+    clang_analyzer_eval(alias == obj); // expected-warning{{TRUE}}
+
+    // should be TRUE
+    clang_analyzer_eval(obj->value == 42); // expected-warning{{UNKNOWN}}
+  }
+}