[analyzer] Track null or undef values through pointer arithmetic.

[NoQ]

Summary

Pointer arithmetic on null or undefined pointers results in null or undefined pointers. This is obvious for undefined pointers; for null pointers it follows from our incorrect-but-somehow-working approach that declares that 0 (Loc) doesn't necessarily represent a pointer of numeric address value 0, but instead it represents any pointer that will cause a valid "null pointer dereference" issue when dereferenced.

For now we've been seeing through pointer arithmetic at the original dereference expression, i.e. in bugreporter::getDerefExpr(), but not during further investigation of the value's origins in bugreporter::trackNullOrUndefValue(). The patch fixes it.

Reproducers are reduced from WebKit's custom string implementations. CStringChecker doesn't call getDerefExpr() at all (this is probably because passing a pointer to memcpy is not an immediate null dereference - in fact i've no idea how this decision was made or how this code was actually supposed to operate). But the second test case demonstrates that simply adding getDerefExpr() to CStringChecker is insufficient. Also an older FIXME test gets fixed this way.

[george.karpenkov]

LGTM with a nit.
Also I don't quite understand why being additive is important? Isn't pointer subtraction basically the same?

lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

78

static.
+1 for using functions.

[NoQ]

Substraction is an additive operation. Added tests for that. Added even more tests.

[NoQ]

lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

78

Whoops.

lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

 //===----------------------------------------------------------------------===//
 
 bool bugreporter::isDeclRefExprToReference(const Expr *E) {
   if (const auto *DRE = dyn_cast<DeclRefExpr>(E))
     return DRE->getDecl()->getType()->isReferenceType();
   return false;
 }
 
+static const Expr *peelOffPointerArithmetic(const BinaryOperator *B) {
+  if (B->isAdditiveOp() && B->getType()->isPointerType()) {
+    if (B->getLHS()->getType()->isPointerType()) {
+      return B->getLHS();
+    } else if (B->getRHS()->getType()->isPointerType()) {
+      return B->getRHS();
+    }
+  }
+  return nullptr;
+}
+
 /// Given that expression S represents a pointer that would be dereferenced,
 /// try to find a sub-expression from which the pointer came from.
 /// This is used for tracking down origins of a null or undefined value:
 /// "this is null because that is null because that is null" etc.
 /// We wipe away field and element offsets because they merely add offsets.
 /// We also wipe away all casts except lvalue-to-rvalue casts, because the
 /// latter represent an actual pointer dereference; however, we remove
 /// the final lvalue-to-rvalue cast before returning from this function
while (true) {
     if (const auto *CE = dyn_cast<CastExpr>(E)) {
       if (CE->getCastKind() == CK_LValueToRValue) {
         // This cast represents the load we're looking for.
         break;
       }
       E = CE->getSubExpr();
     } else if (const auto *B = dyn_cast<BinaryOperator>(E)) {
       // Pointer arithmetic: '*(x + 2)' -> 'x') etc.
-      if (B->getType()->isPointerType()) {
-        if (B->getLHS()->getType()->isPointerType()) {
-          E = B->getLHS();
-        } else if (B->getRHS()->getType()->isPointerType()) {
-          E = B->getRHS();
-        } else {
-          break;
-        }
+      if (const Expr *Inner = peelOffPointerArithmetic(B)) {
+        E = Inner;
       } else {
         // Probably more arithmetic can be pattern-matched here,
         // but for now give up.
         break;
       }
     } else if (const auto *U = dyn_cast<UnaryOperator>(E)) {
       if (U->getOpcode() == UO_Deref || U->getOpcode() == UO_AddrOf ||
           (U->isIncrementDecrementOp() && U->getType()->isPointerType())) {
do {
             else
               return peelOffOuterExpr(CO->getFalseExpr(), N);
           }
         }
       }
       NI = NI->getFirstPred();
     } while (NI);
   }
+
+  if (auto *BO = dyn_cast<BinaryOperator>(Ex))
+    if (const Expr *SubEx = peelOffPointerArithmetic(BO))
+      return peelOffOuterExpr(SubEx, N);
+
   return Ex;
 }
 
 /// Walk through nodes until we get one that matches the statement exactly.
 /// Alternately, if we hit a known lvalue for the statement, we know we've
 /// gone too far (though we can likely track the lvalue better anyway).
 static const ExplodedNode* findNodeForStatement(const ExplodedNode *N,
                                                 const Stmt *S,

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

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}}
+  *x = 7; // no-warning
 }
 
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithAssignment(struct S *s) {
   idc(s);
   int *x = &(s->f1);
   *x = 7; // no-warning
 }
 

test/Analysis/null-deref-path-notes.c

-// RUN: %clang_analyze_cc1 -w -x c -analyzer-checker=core -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -w -x c -analyzer-checker=core,unix -analyzer-output=text -verify %s
 
 // Avoid the crash when finding the expression for tracking the origins
 // of the null pointer for path notes.
 void pr34373() {
   int *a = 0; // expected-note{{'a' initialized to a null pointer value}}
   (a + 0)[0]; // expected-warning{{Array access results in a null pointer dereference}}
               // expected-note@-1{{Array access results in a null pointer dereference}}
 }
+
+typedef __typeof(sizeof(int)) size_t;
+void *memcpy(void *dest, const void *src, unsigned long count);
+
+void f1(char *source) {
+  char *destination = 0; // expected-note{{'destination' initialized to a null pointer value}}
+  memcpy(destination + 0, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+                                       // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f2(char *source) {
+  char *destination = 0; // expected-note{{'destination' initialized to a null pointer value}}
+  memcpy(destination - 0, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+                                       // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f3(char *source) {
+  char *destination = 0; // FIXME: There should be a note here as well.
+  destination = destination + 0; // expected-note{{Null pointer value stored to 'destination'}}
+  memcpy(destination, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+                                   // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f4(char *source) {
+  char *destination = 0; // FIXME: There should be a note here as well.
+  destination = destination - 0; // expected-note{{Null pointer value stored to 'destination'}}
+  memcpy(destination, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+                                   // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f5(char *source) {
+  char *destination1 = 0; // expected-note{{'destination1' initialized to a null pointer value}}
+  char *destination2 = destination1 + 0; // expected-note{{'destination2' initialized to a null pointer value}}
+  memcpy(destination2, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+                                    // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}
+
+void f6(char *source) {
+  char *destination1 = 0; // expected-note{{'destination1' initialized to a null pointer value}}
+  char *destination2 = destination1 - 0; // expected-note{{'destination2' initialized to a null pointer value}}
+  memcpy(destination2, source, 10); // expected-warning{{Null pointer argument in call to memory copy function}}
+                                    // expected-note@-1{{Null pointer argument in call to memory copy function}}
+}