[analyzer] Don't crash when copying an unknown number of bytes with memcpy(). Also handle all memcpy-family return values in evalCopyCommon(), rather than having some outside and some inside.

llvm-svn: 132617

2 files changed, 31 insertions, 11 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index d9e49e4c477..2e3a1f17659 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -748,19 +748,33 @@ void CStringChecker::evalCopyCommon(CheckerContext &C,
       // bind the expr.
       if (IsMempcpy) {
         loc::MemRegionVal *destRegVal = dyn_cast<loc::MemRegionVal>(&destVal);
+        assert(destRegVal && "Destination should be a known MemRegionVal here");
         
         // Get the length to copy.
-        SVal lenVal = state->getSVal(Size);
-        NonLoc *lenValNonLoc = dyn_cast<NonLoc>(&lenVal);
+        NonLoc *lenValNonLoc = dyn_cast<NonLoc>(&sizeVal);
         
-        // Get the byte after the last byte copied.
-        SVal lastElement = C.getSValBuilder().evalBinOpLN(state, BO_Add, 
-                                                          *destRegVal,
-                                                          *lenValNonLoc, 
-                                                          Dest->getType());
+        if (lenValNonLoc) {
+          // Get the byte after the last byte copied.
+          SVal lastElement = C.getSValBuilder().evalBinOpLN(state, BO_Add, 
+                                                            *destRegVal,
+                                                            *lenValNonLoc, 
+                                                            Dest->getType());
         
-        // The byte after the last byte copied is the return value.
-        state = state->BindExpr(CE, lastElement);
+          // The byte after the last byte copied is the return value.
+          state = state->BindExpr(CE, lastElement);
+        } else {
+          // If we don't know how much we copied, we can at least
+          // conjure a return value for later.
+          unsigned Count = C.getNodeBuilder().getCurrentBlockCount();
+          SVal result =
+            C.getSValBuilder().getConjuredSymbolVal(NULL, CE, Count);
+          state = state->BindExpr(CE, result);
+        }
+
+      } else {
+        // All other copies return the destination buffer.
+        // (Well, bcopy() has a void return type, but this won't hurt.)
+        state = state->BindExpr(CE, destVal);
       }
 
       // Invalidate the destination.
@@ -780,7 +794,7 @@ void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE) const {
   // The return value is the address of the destination buffer.
   const Expr *Dest = CE->getArg(0);
   const GRState *state = C.getState();
-  state = state->BindExpr(CE, state->getSVal(Dest));
+
   evalCopyCommon(C, CE, state, CE->getArg(2), Dest, CE->getArg(1), true);
 }
 
@@ -798,7 +812,7 @@ void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE) const {
   // The return value is the address of the destination buffer.
   const Expr *Dest = CE->getArg(0);
   const GRState *state = C.getState();
-  state = state->BindExpr(CE, state->getSVal(Dest));
+
   evalCopyCommon(C, CE, state, CE->getArg(2), Dest, CE->getArg(1));
 }
 
diff --git a/clang/test/Analysis/bstring.c b/clang/test/Analysis/bstring.c
index de88e9ae6fc..68bbb1a5b2f 100644
--- a/clang/test/Analysis/bstring.c
+++ b/clang/test/Analysis/bstring.c
@@ -264,6 +264,12 @@ void mempcpy_unknown_size_warn (size_t n) {
     (void)*(char*)0; // no-warning
 }
 
+void mempcpy_unknownable_size (char *src, float n) {
+  char a[4];
+  // This used to crash because we don't model floats.
+  mempcpy(a, src, (size_t)n);
+}
+
 //===----------------------------------------------------------------------===
 // memmove()
 //===----------------------------------------------------------------------===