[PATCH] D28765: CStringChecker can crash when uninitialized checks are disabled

[Vlad Tsyrklevich via Phabricator via cfe-commits]

vlad.tsyrklevich created this revision.
vlad.tsyrklevich added reviewers: cfe-commits, zaks.anna, dcoughlin, NoQ.

CStringChecker assumes that SVals are not undefined at two points with comments stating that other checkers will check for that condition first; however, it can crash if a user chooses a particular configuration. I hit such an unlucky configuration while eliminating false positive heavy checks analyzing the Linux kernel and tracked down the crashes to this assumption.

Add UndefinedVal checks where appropriate.


https://reviews.llvm.org/D28765

Files:
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  test/Analysis/cstring-regressions.c


Index: test/Analysis/cstring-regressions.c
===================================================================
--- /dev/null
+++ test/Analysis/cstring-regressions.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.cstring,debug.ExprInspection -analyzer-disable-checker=core.UndefinedBinaryOperatorResult,core.CallAndMessage,core.NullDereference,core.uninitialized -analyzer-store=region -verify %s
+//
+// CStringChecker crashes found running analyzer over the Linux kernel source
+// with a very particular set of enabled/disabled checks.
+
+int strcmp(const char * s1, const char * s2);
+int memcmp(const void *s1, const void *s2, unsigned long n);
+void clang_analyzer_eval(int);
+
+void test1(void *ptr) {
+ int uninit;
+ clang_analyzer_eval(strcmp(ptr + uninit, "mcp_trace_meta") == 0); // expected-warning{{UNKNOWN}}
+}
+
+void test2(char *foo, void *ptr) {
+ int uninit;
+ clang_analyzer_eval(memcmp(foo, ptr + uninit, 48) == 0); // expected-warning{{UNKNOWN}}
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1179,15 +1179,17 @@
   if (stateNonZeroSize) {
     state = stateNonZeroSize;
     // If we know the two buffers are the same, we know the result is 0.
-    // First, get the two buffers' addresses. Another checker will have already
-    // made sure they're not undefined.
-    DefinedOrUnknownSVal LV =
-        state->getSVal(Left, LCtx).castAs<DefinedOrUnknownSVal>();
-    DefinedOrUnknownSVal RV =
-        state->getSVal(Right, LCtx).castAs<DefinedOrUnknownSVal>();
+    // First, get the two buffers' addresses.
+    Optional<DefinedOrUnknownSVal> LV =
+        state->getSVal(Left, LCtx).getAs<DefinedOrUnknownSVal>();
+    Optional<DefinedOrUnknownSVal> RV =
+        state->getSVal(Right, LCtx).getAs<DefinedOrUnknownSVal>();
+
+    if (!LV || !RV)
+      return;
 
     // See if they are the same.
-    DefinedOrUnknownSVal SameBuf = svalBuilder.evalEQ(state, LV, RV);
+    DefinedOrUnknownSVal SameBuf = svalBuilder.evalEQ(state, *LV, *RV);
     ProgramStateRef StSameBuf, StNotSameBuf;
     std::tie(StSameBuf, StNotSameBuf) = state->assume(SameBuf);
 
@@ -1781,13 +1783,19 @@
   // Check that the first string is non-null
   const Expr *s1 = CE->getArg(0);
   SVal s1Val = state->getSVal(s1, LCtx);
+  if (s1Val.isUndef())
+    return;
+
   state = checkNonNull(C, state, s1, s1Val);
   if (!state)
     return;
 
   // Check that the second string is non-null.
   const Expr *s2 = CE->getArg(1);
   SVal s2Val = state->getSVal(s2, LCtx);
+  if (s2Val.isUndef())
+    return;
+
   state = checkNonNull(C, state, s2, s2Val);
   if (!state)
     return;
@@ -1803,8 +1811,7 @@
     return;
 
   // If we know the two buffers are the same, we know the result is 0.
-  // First, get the two buffers' addresses. Another checker will have already
-  // made sure they're not undefined.
+  // First, get the two buffers' addresses.
   DefinedOrUnknownSVal LV = s1Val.castAs<DefinedOrUnknownSVal>();
   DefinedOrUnknownSVal RV = s2Val.castAs<DefinedOrUnknownSVal>();
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D28765.84534.patch
Type: text/x-patch
Size: 3200 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170116/c278041c/attachment.bin>