From 7f660857309a14c036a80ef90b40bf8f68fda9da Mon Sep 17 00:00:00 2001
From: Jordan Rose <jordan_rose@apple.com>
Date: Wed, 15 Aug 2012 21:56:23 +0000
Subject: [PATCH] [analyzer] If we call a C++ method on an object, assume it's
 non-null.

This is analogous to our handling of pointer dereferences: if we
dereference a pointer that may or may not be null, we assume it's non-null
from then on.

While some implementations of C++ (including ours) allow you to call a
non-virtual method through a null pointer of object type, it is technically
disallowed by the C++ standard, and should not prune out any real paths in
practice.

  [class.mfct.non-static]p1: A non-static member function may be called
    for an object of its class type, or for an object of a class derived
    from its class type...
  (a null pointer value does not refer to an object)

We can also make the same assumption about function pointers.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@161992 91177308-0d34-0410-b5e6-96231b3b80d8
---
 .../Checkers/CallAndMessageChecker.cpp        | 12 ++++++++---
 test/Analysis/func.c                          | 14 ++++++++++++-
 test/Analysis/method-call.cpp                 | 20 +++++++++++++++----
 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
index 30f45c7685..5edcf09f11 100644
--- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -235,17 +235,20 @@ void CallAndMessageChecker::checkPreStmt(const CallExpr *CE,
   ProgramStateRef StNonNull, StNull;
   llvm::tie(StNonNull, StNull) = State->assume(cast<DefinedOrUnknownSVal>(L));
 
-  // FIXME: Do we want to record the non-null assumption here?
   if (StNull && !StNonNull) {
     if (!BT_call_null)
       BT_call_null.reset(
         new BuiltinBug("Called function pointer is null (null dereference)"));
     emitBadCall(BT_call_null.get(), C, Callee);
   }
+
+  C.addTransition(StNonNull);
 }
 
 void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
                                          CheckerContext &C) const {
+  ProgramStateRef State = C.getState();
+
   // If this is a call to a C++ method, check if the callee is null or
   // undefined.
   if (const CXXInstanceCall *CC = dyn_cast<CXXInstanceCall>(&Call)) {
@@ -258,11 +261,9 @@ void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
       return;
     }
 
-    ProgramStateRef State = C.getState();
     ProgramStateRef StNonNull, StNull;
     llvm::tie(StNonNull, StNull) = State->assume(cast<DefinedOrUnknownSVal>(V));
 
-    // FIXME: Do we want to record the non-null assumption here?
     if (StNull && !StNonNull) {
       if (!BT_cxx_call_null)
         BT_cxx_call_null.reset(new BuiltinBug("Called C++ object pointer "
@@ -270,6 +271,8 @@ void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
       emitBadCall(BT_cxx_call_null.get(), C, CC->getCXXThisExpr());
       return;
     }
+
+    State = StNonNull;
   }
 
   // Don't check for uninitialized field values in arguments if the
@@ -291,6 +294,9 @@ void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
                            Call.getArgExpr(i), /*IsFirstArgument=*/i == 0,
                            checkUninitFields, Call, *BT))
       return;
+
+  // If we make it here, record our assumptions about the callee.
+  C.addTransition(State);
 }
 
 void CallAndMessageChecker::checkPreObjCMessage(const ObjCMethodCall &msg,
diff --git a/test/Analysis/func.c b/test/Analysis/func.c
index b6cebde810..709ebf7793 100644
--- a/test/Analysis/func.c
+++ b/test/Analysis/func.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.core -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.core,debug.ExprInspection -analyzer-store=region -verify %s
+
+void clang_analyzer_eval(int);
 
 void f(void) {
   void (*p)(void);
@@ -13,3 +15,13 @@ void g(void (*fp)(void));
 void f2() {
   g(f);
 }
+
+void f3(void (*f)(void), void (*g)(void)) {
+  clang_analyzer_eval(!f); // expected-warning{{UNKNOWN}}
+  f();
+  clang_analyzer_eval(!f); // expected-warning{{FALSE}}
+
+  clang_analyzer_eval(!g); // expected-warning{{UNKNOWN}}
+  (*g)();
+  clang_analyzer_eval(!g); // expected-warning{{FALSE}}
+}
diff --git a/test/Analysis/method-call.cpp b/test/Analysis/method-call.cpp
index 91da532456..912062739c 100644
--- a/test/Analysis/method-call.cpp
+++ b/test/Analysis/method-call.cpp
@@ -1,25 +1,37 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-ipa=inlining -analyzer-store region -verify %s
-// XFAIL: *
 
 void clang_analyzer_eval(bool);
 
+
 struct A {
   int x;
   A(int a) { x = a; }
   int getx() const { return x; }
 };
 
+void testNullObject(A *a) {
+  clang_analyzer_eval(a); // expected-warning{{UNKNOWN}}
+  (void)a->getx(); // assume we know what we're doing
+  clang_analyzer_eval(a); // expected-warning{{TRUE}}
+}
+
+
+// FIXME: These require constructor inlining to be enabled.
+
 void f1() {
   A x(3);
-  clang_analyzer_eval(x.getx() == 3); // expected-warning{{TRUE}}
+  // should be TRUE
+  clang_analyzer_eval(x.getx() == 3); // expected-warning{{UNKNOWN}}
 }
 
 void f2() {
   const A &x = A(3);
-  clang_analyzer_eval(x.getx() == 3); // expected-warning{{TRUE}}
+  // should be TRUE
+  clang_analyzer_eval(x.getx() == 3); // expected-warning{{UNKNOWN}}
 }
 
 void f3() {
   const A &x = (A)3;
-  clang_analyzer_eval(x.getx() == 3); // expected-warning{{TRUE}}
+  // should be TRUE
+  clang_analyzer_eval(x.getx() == 3); // expected-warning{{UNKNOWN}}
 }
-- 
2.40.0