[analyzer] Improve suppression for inlined defensive checks before operator &.

Null dereferences are suppressed if the lvalue was constrained to 0 for the
first time inside a sub-function that was inlined during analysis, because
such constraint is a valid defensive check that does not, by itself,
indicate that null pointer case is anyhow special for the caller.

If further operations on the lvalue are performed, the symbolic lvalue is
collapsed to concrete null pointer, and we need to track where does the null
pointer come from.

Improve such tracking for lvalue operations involving operator &.

rdar://problem/27876009

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

llvm-svn: 301224

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

@@ -961,6 +961,26 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N,
   const Expr *Inner = nullptr;
   if (const Expr *Ex = dyn_cast<Expr>(S)) {
     Ex = Ex->IgnoreParenCasts();
+
+    // Performing operator `&' on an lvalue expression is essentially a no-op.
+    // Then, if we are taking addresses of fields or elements, these are also
+    // unlikely to matter.
+    // FIXME: There's a hack in our Store implementation that always computes
+    // field offsets around null pointers as if they are always equal to 0.
+    // The idea here is to report accesses to fields as null dereferences
+    // even though the pointer value that's being dereferenced is actually
+    // the offset of the field rather than exactly 0.
+    // See the FIXME in StoreManager's getLValueFieldOrIvar() method.
+    // This code interacts heavily with this hack; otherwise the value
+    // would not be null at all for most fields, so we'd be unable to track it.
+    if (const auto *Op = dyn_cast<UnaryOperator>(Ex))
+      if (Op->getOpcode() == UO_AddrOf && Op->getSubExpr()->isLValue()) {
+        Ex = Op->getSubExpr()->IgnoreParenCasts();
+        while (const auto *ME = dyn_cast<MemberExpr>(Ex)) {
+          Ex = ME->getBase()->IgnoreParenCasts();
+        }
+      }
+
     if (ExplodedGraph::isInterestingLValueExpr(Ex) || CallEvent::isCallStmt(Ex))
       Inner = Ex;
   }

clang/lib/StaticAnalyzer/Core/Store.cpp

@@ -404,9 +404,15 @@ SVal StoreManager::getLValueFieldOrIvar(const Decl *D, SVal Base) {
 
   case loc::ConcreteIntKind:
     // While these seem funny, this can happen through casts.
-    // FIXME: What we should return is the field offset.  For example,
-    //  add the field offset to the integer value.  That way funny things
+    // FIXME: What we should return is the field offset, not base. For example,
+    //  add the field offset to the integer value.  That way things
     //  like this work properly:  &(((struct foo *) 0xa)->f)
+    //  However, that's not easy to fix without reducing our abilities
+    //  to catch null pointer dereference. Eg., ((struct foo *)0x0)->f = 7
+    //  is a null dereference even though we're dereferencing offset of f
+    //  rather than null. Coming up with an approach that computes offsets
+    //  over null pointers properly while still being able to catch null
+    //  dereferences might be worth it.
     return Base;
 
   default:
@@ -431,7 +437,7 @@ SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset,
   // If the base is an unknown or undefined value, just return it back.
   // FIXME: For absolute pointer addresses, we just return that value back as
   //  well, although in reality we should return the offset added to that
-  //  value.
+  //  value. See also the similar FIXME in getLValueFieldOrIvar().
   if (Base.isUnknownOrUndef() || Base.getAs<loc::ConcreteInt>())
     return Base;
 

clang/test/Analysis/inlining/inline-defensive-checks.c

@@ -1,7 +1,7 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config suppress-inlined-defensive-checks=true -verify %s
 
 // Perform inline defensive checks.
-void idc(int *p) {
+void idc(void *p) {
 	if (p)
 		;
 }
@@ -139,3 +139,42 @@ void idcTrackZeroThroughDoubleAssignemnt(int x) {
   int z = y;
   idcTriggerZeroValueThroughCall(z);
 }
+
+struct S {
+  int f1;
+  int f2;
+};
+
+void idcTrackZeroValueThroughUnaryPointerOperators(struct S *s) {
+  idc(s);
+  *(&(s->f1)) = 7; // no-warning
+}
+
+void idcTrackZeroValueThroughUnaryPointerOperatorsWithOffset1(struct S *s) {
+  idc(s);
+  int *x = &(s->f2);
+  *x = 7; // no-warning
+}
+
+void idcTrackZeroValueThroughUnaryPointerOperatorsWithOffset2(struct S *s) {
+  idc(s);
+  int *x = &(s->f2) - 1;
+  // FIXME: Should not warn.
+  *x = 7; // expected-warning{{Dereference of null pointer}}
+}
+
+void idcTrackZeroValueThroughUnaryPointerOperatorsWithAssignment(struct S *s) {
+  idc(s);
+  int *x = &(s->f1);
+  *x = 7; // no-warning
+}
+
+
+struct S2 {
+  int a[1];
+};
+
+void idcTrackZeroValueThroughUnaryPointerOperatorsWithArrayField(struct S2 *s) {
+  idc(s);
+  *(&(s->a[0])) = 7; // no-warning
+}

clang/test/Analysis/inlining/inline-defensive-checks.cpp

@@ -71,3 +71,16 @@ void test(int *p1, int *p2) {
   idc(p1);
 	Foo f(p1);
 }
+
+struct Bar {
+  int x;
+};
+void idcBar(Bar *b) {
+  if (b)
+    ;
+}
+void testRefToField(Bar *b) {
+  idcBar(b);
+  int &x = b->x; // no-warning
+  x = 5;
+}

clang/test/Analysis/null-deref-offsets.c

@@ -0,0 +1,34 @@
+// RUN: %clang_analyze_cc1 -w -triple i386-apple-darwin10 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+struct S {
+  int x, y;
+  int z[2];
+};
+
+void testOffsets(struct S *s) {
+  if (s != 0)
+    return;
+
+  // FIXME: Here we are testing the hack that computes offsets to null pointers
+  // as 0 in order to find null dereferences of not-exactly-null pointers,
+  // such as &(s->y) below, which is equal to 4 rather than 0 in run-time.
+
+  // These are indeed null.
+  clang_analyzer_eval(s == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(&(s->x) == 0); // expected-warning{{TRUE}}
+
+  // FIXME: These should ideally be true.
+  clang_analyzer_eval(&(s->y) == 4); // expected-warning{{FALSE}}
+  clang_analyzer_eval(&(s->z[0]) == 8); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(&(s->z[1]) == 12); // expected-warning{{UNKNOWN}}
+
+  // FIXME: These should ideally be false.
+  clang_analyzer_eval(&(s->y) == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(&(s->z[0]) == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(&(s->z[1]) == 0); // expected-warning{{UNKNOWN}}
+
+  // But this should still be a null dereference.
+  s->y = 5; // expected-warning{{Access to field 'y' results in a dereference of a null pointer (loaded from variable 's')}}
+}

clang/test/Analysis/uninit-const.cpp

@@ -122,7 +122,7 @@ void f1(void) {
 }
 
 void f_uninit(void) {
-      int x;
+      int x;               // expected-note {{'x' declared without an initial value}}
       doStuff_uninit(&x);  // expected-warning {{1st function call argument is a pointer to uninitialized value}}
                            // expected-note@-1 {{1st function call argument is a pointer to uninitialized value}}
 }