[clang] f450dd6 - [analyzer] CStringChecker: Fix a crash on unknown value passed to strlcat.

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2019-12-13T18:00:24-08:00
New Revision: f450dd63a14d6cb16418f6a6f4de26916502c13f

URL: https://github.com/llvm/llvm-project/commit/f450dd63a14d6cb16418f6a6f4de26916502c13f
DIFF: https://github.com/llvm/llvm-project/commit/f450dd63a14d6cb16418f6a6f4de26916502c13f.diff

LOG: [analyzer] CStringChecker: Fix a crash on unknown value passed to strlcat.

Checkers should always account for unknown values.

Also use a slightly more high-level API that naturally avoids the problem.

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
    clang/test/Analysis/bsd-string.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 4203f790e211..0cf7056a0783 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1706,13 +1706,12 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE,
           } else {
             if (appendK == ConcatFnKind::none) {
               // strlcpy returns strlen(src)
-              StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, *strLengthNL);
-            } else if (dstStrLengthNL) {
+              StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, strLength);
+            } else {
               // strlcat returns strlen(src) + strlen(dst)
-              SVal retSize = svalBuilder.evalBinOpNN(
-                  state, BO_Add, *strLengthNL, *dstStrLengthNL, sizeTy);
-              StateZeroSize =
-                  StateZeroSize->BindExpr(CE, LCtx, *(retSize.getAs<NonLoc>()));
+              SVal retSize = svalBuilder.evalBinOp(
+                  state, BO_Add, strLength, dstStrLength, sizeTy);
+              StateZeroSize = StateZeroSize->BindExpr(CE, LCtx, retSize);
             }
           }
           C.addTransition(StateZeroSize);

diff  --git a/clang/test/Analysis/bsd-string.c b/clang/test/Analysis/bsd-string.c
index 3778664a8ef5..adb8721c3fa2 100644
--- a/clang/test/Analysis/bsd-string.c
+++ b/clang/test/Analysis/bsd-string.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: %clang_analyze_cc1 -w -verify %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=unix.cstring.NullArg \
 // RUN:   -analyzer-checker=alpha.unix.cstring \
@@ -131,3 +131,9 @@ void f11() {
   strlcpy(b, "hello ", sizeof(b));
   strlcat(b, a, sizeof(b)); // no-warning
 }
+
+int a, b;
+void unknown_val_crash() {
+  // We're unable to evaluate the integer-to-pointer cast.
+  strlcat(&b, a, 0); // no-crash
+}