[analyzer] ReturnVisitor: Bypass everything to see inlined calls

Summary:
When we traversed backwards on ExplodedNodes to see where processed the
given statement we `break` too early. With the current approach we do not
miss the CallExitEnd ProgramPoint which stands for an inlined call.

Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

Reviewed By: NoQ

Subscribers: szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy,
             dkrupp, cfe-commits

Tags: #clang

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

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

4 files changed, 199 insertions, 9 deletions

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 9aaf050d47..469dd1d3c0 100644
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -841,16 +841,38 @@ public:
       return;
 
     // First, find when we processed the statement.
+    // If we work with a 'CXXNewExpr' that is going to be purged away before
+    // its call take place. We would catch that purge in the last condition
+    // as a 'StmtPoint' so we have to bypass it.
+    const bool BypassCXXNewExprEval = isa<CXXNewExpr>(S);
+
+    // This is moving forward when we enter into another context.
+    const StackFrameContext *CurrentSFC = Node->getStackFrame();
+
     do {
-      if (auto CEE = Node->getLocationAs<CallExitEnd>())
+      // If that is satisfied we found our statement as an inlined call.
+      if (Optional<CallExitEnd> CEE = Node->getLocationAs<CallExitEnd>())
         if (CEE->getCalleeContext()->getCallSite() == S)
           break;
-      if (auto SP = Node->getLocationAs<StmtPoint>())
-        if (SP->getStmt() == S)
-          break;
 
+      // Try to move forward to the end of the call-chain.
       Node = Node->getFirstPred();
-    } while (Node);
+      if (!Node)
+        break;
+
+      const StackFrameContext *PredSFC = Node->getStackFrame();
+
+      // If that is satisfied we found our statement.
+      // FIXME: This code currently bypasses the call site for the
+      //        conservatively evaluated allocator.
+      if (!BypassCXXNewExprEval)
+        if (Optional<StmtPoint> SP = Node->getLocationAs<StmtPoint>())
+          // See if we do not enter into another context.
+          if (SP->getStmt() == S && CurrentSFC == PredSFC)
+            break;
+
+      CurrentSFC = PredSFC;
+    } while (Node->getStackFrame() == CurrentSFC);
 
     // Next, step over any post-statement checks.
     while (Node && Node->getLocation().getAs<PostStmt>())
diff --git a/test/Analysis/inlining/placement-new-fp-suppression.cpp b/test/Analysis/inlining/placement-new-fp-suppression.cpp
new file mode 100644
index 0000000000..b051ad1b04
--- /dev/null
+++ b/test/Analysis/inlining/placement-new-fp-suppression.cpp
@@ -0,0 +1,137 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core.CallAndMessage \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// RUN:  -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core.CallAndMessage \
+// RUN:  -DSUPPRESSED \
+// RUN:  -verify %s
+
+#ifdef SUPPRESSED
+// expected-no-diagnostics
+#endif
+
+#include "../Inputs/system-header-simulator-cxx.h"
+
+typedef unsigned long uintptr_t;
+
+void error();
+void *malloc(size_t);
+
+
+// From llvm/include/llvm/Support/MathExtras.h
+inline uintptr_t alignAddr(const void *Addr, size_t Alignment) {
+  return (((uintptr_t)Addr + Alignment - 1) & ~(uintptr_t)(Alignment - 1));
+}
+
+inline size_t alignmentAdjustment(const void *Ptr, size_t Alignment) {
+  return alignAddr(Ptr, Alignment) - (uintptr_t)Ptr;
+}
+
+
+// From llvm/include/llvm/Support/MemAlloc.h
+inline void *safe_malloc(size_t Sz) {
+  void *Result = malloc(Sz);
+  if (Result == nullptr)
+    error();
+
+  return Result;
+}
+
+
+// From llvm/include/llvm/Support/Allocator.h
+class MallocAllocator {
+public:
+  void *Allocate(size_t Size, size_t /*Alignment*/) {
+    return safe_malloc(Size);
+  }
+};
+
+class BumpPtrAllocator {
+public:
+  void *Allocate(size_t Size, size_t Alignment) {
+    BytesAllocated += Size;
+    size_t Adjustment = alignmentAdjustment(CurPtr, Alignment);
+    size_t SizeToAllocate = Size;
+
+    size_t PaddedSize = SizeToAllocate + Alignment - 1;
+    uintptr_t AlignedAddr = alignAddr(Allocator.Allocate(PaddedSize, 0),
+                                      Alignment);
+    char *AlignedPtr = (char*)AlignedAddr;
+
+    return AlignedPtr;
+  }
+
+private:
+  char *CurPtr = nullptr;
+  size_t BytesAllocated = 0;
+  MallocAllocator Allocator;
+};
+
+
+// From clang/include/clang/AST/ASTContextAllocate.h
+class ASTContext;
+
+void *operator new(size_t Bytes, const ASTContext &C, size_t Alignment = 8);
+void *operator new[](size_t Bytes, const ASTContext &C, size_t Alignment = 8);
+
+
+// From clang/include/clang/AST/ASTContext.h
+class ASTContext {
+public:
+  void *Allocate(size_t Size, unsigned Align = 8) const {
+    return BumpAlloc.Allocate(Size, Align);
+  }
+
+  template <typename T>
+  T *Allocate(size_t Num = 1) const {
+    return static_cast<T *>(Allocate(Num * sizeof(T), alignof(T)));
+  }
+
+private:
+  mutable BumpPtrAllocator BumpAlloc;
+};
+
+
+// From clang/include/clang/AST/ASTContext.h
+inline void *operator new(size_t Bytes, const ASTContext &C,
+                          size_t Alignment /* = 8 */) {
+  return C.Allocate(Bytes, Alignment);
+}
+
+inline void *operator new[](size_t Bytes, const ASTContext &C,
+                            size_t Alignment /* = 8 */) {
+  return C.Allocate(Bytes, Alignment);
+}
+
+
+// From clang/include/clang/AST/Attr.h
+void *operator new(size_t Bytes, ASTContext &C,
+                   size_t Alignment = 8) noexcept {
+  return ::operator new(Bytes, C, Alignment);
+}
+
+
+class A {
+public:
+  void setValue(int value) { Value = value; }
+private:
+  int Value;
+};
+
+void f(const ASTContext &C) {
+  A *a = new (C) A;
+  a->setValue(13);
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Called C++ object pointer is null}}
+#endif
+}	
+
+void g(const ASTContext &C) {
+  A *a = new (C) A[1];
+  a[0].setValue(13);
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Called C++ object pointer is null}}
+#endif
+}
+
diff --git a/test/Analysis/new-ctor-null-throw.cpp b/test/Analysis/new-ctor-null-throw.cpp
index cdaf01d8de..dfa7cba763 100644
--- a/test/Analysis/new-ctor-null-throw.cpp
+++ b/test/Analysis/new-ctor-null-throw.cpp
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// RUN:  -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -DSUPPRESSED \
+// RUN:  -verify %s
 
 void clang_analyzer_eval(bool);
 
@@ -9,18 +14,41 @@ typedef __typeof__(sizeof(int)) size_t;
 // operator new.
 void *operator new(size_t size) {
   return nullptr;
+  // expected-warning@-1 {{'operator new' should not return a null pointer unless it is declared 'throw()' or 'noexcept'}}
 }
 void *operator new[](size_t size) {
   return nullptr;
+  // expected-warning@-1 {{'operator new[]' should not return a null pointer unless it is declared 'throw()' or 'noexcept'}}
 }
 
 struct S {
   int x;
   S() : x(1) {}
   ~S() {}
+  int getX() const { return x; }
 };
 
 void testArrays() {
   S *s = new S[10]; // no-crash
-  s[0].x = 2; // expected-warning{{Dereference of null pointer}}
+  s[0].x = 2;
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Dereference of null pointer}}
+#endif
 }
+
+void testCtor() {
+  S *s = new S();
+  s->x = 13;
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Access to field 'x' results in a dereference of a null pointer (loaded from variable 's')}}
+#endif
+}
+
+void testMethod() {
+  S *s = new S();
+  const int X = s->getX();
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Called C++ object pointer is null}}
+#endif
+}
+
diff --git a/test/Analysis/new-ctor-null.cpp b/test/Analysis/new-ctor-null.cpp
index ac2a39a028..32f2f9500c 100644
--- a/test/Analysis/new-ctor-null.cpp
+++ b/test/Analysis/new-ctor-null.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,debug.ExprInspection \
+// RUN:  -verify %s
 
 void clang_analyzer_eval(bool);
 void clang_analyzer_warnIfReached();
@@ -24,7 +26,8 @@ struct S {
 
 void testArrays() {
   S *s = new S[10]; // no-crash
-  s[0].x = 2; // expected-warning{{Dereference of null pointer}}
+  s[0].x = 2;
+  // no-warning: 'Dereference of null pointer' suppressed by ReturnVisitor.
 }
 
 int global;