[Analyzer] alpha.unix.cstring.OutOfBounds checker enable/disable fix

It was not possible to disable alpha.unix.cstring.OutOfBounds checker's reports
since unix.Malloc checker always implicitly enabled the filter. Moreover if the
checker was disabled from command line (-analyzer-disable-checker ..) the out
of bounds warnings were nevertheless emitted under different checker names such
as unix.cstring.NullArg, or unix.Malloc.

This patch fixes the case sot that Malloc checker only enables implicitly the
underlying modeling of strcpy, memcpy etc. but not the warning messages that
would have been emmitted by alpha.unix.cstring.OutOfBounds

Patch by: Dániel Krupp

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

llvm-svn: 337000

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

@@ -305,10 +305,10 @@ ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C,
   ProgramStateRef StOutBound = state->assumeInBound(Idx, Size, false);
   if (StOutBound && !StInBound) {
     // These checks are either enabled by the CString out-of-bounds checker
-    // explicitly or the "basic" CStringNullArg checker support that Malloc
-    // checker enables.
-    assert(Filter.CheckCStringOutOfBounds || Filter.CheckCStringNullArg);
-
+    // explicitly or implicitly by the Malloc checker.
+    // In the latter case we only do modeling but do not emit warning.
+    if (!Filter.CheckCStringOutOfBounds)
+      return nullptr;
     // Emit a bug report.
     if (warningMsg) {
       emitOutOfBoundsBug(C, StOutBound, S, warningMsg);
@@ -1039,7 +1039,7 @@ bool CStringChecker::memsetAux(const Expr *DstBuffer, const Expr *CharE,
     std::tie(StateWholeReg, StateNotWholeReg) =
         State->assume(svalBuilder.evalEQ(State, Extent, *SizeNL));
 
-    // With the semantic of 'memset()', we should convert the CharVal to 
+    // With the semantic of 'memset()', we should convert the CharVal to
     // unsigned char.
     CharVal = svalBuilder.evalCast(CharVal, Ctx.UnsignedCharTy, Ctx.IntTy);
 
@@ -2418,5 +2418,5 @@ void CStringChecker::checkDeadSymbols(SymbolReaper &SR,
 REGISTER_CHECKER(CStringNotNullTerm)
 
   void ento::registerCStringCheckerBasic(CheckerManager &Mgr) {
-    registerCStringNullArg(Mgr);
+    Mgr.registerChecker<CStringChecker>();
   }

clang/test/Analysis/cstring-plist.c

@@ -0,0 +1,22 @@
+// RUN: rm -f %t
+// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,unix.Malloc,unix.cstring.NullArg -analyzer-disable-checker=alpha.unix.cstring.OutOfBounds -analyzer-output=plist -analyzer-config path-diagnostics-alternate=false -o %t %s
+// RUN: FileCheck -input-file %t %s
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void free(void *);
+char *strncpy(char *restrict s1, const char *restrict s2, size_t n);
+
+
+
+void cstringchecker_bounds_nocrash() {
+  char *p = malloc(2);
+  strncpy(p, "AAA", sizeof("AAA")); // we don't expect warning as the checker is disabled
+  free(p);
+}
+
+// CHECK: <key>diagnostics</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </plist>

clang/test/Analysis/malloc.c

@@ -375,7 +375,7 @@ void CheckUseZeroReallocatedPathWarn(_Bool b) {
 // or inter-procedural analysis, this is a conservative answer.
 int *f3() {
   static int *p = 0;
-  p = malloc(12); 
+  p = malloc(12);
   return p; // no-warning
 }
 
@@ -384,7 +384,7 @@ int *f3() {
 // functions or inter-procedural analysis, this is a conservative answer.
 static int *p_f4 = 0;
 int *f4() {
-  p_f4 = malloc(12); 
+  p_f4 = malloc(12);
   return p_f4; // no-warning
 }
 
@@ -1232,7 +1232,7 @@ void radar10978247(int myValueSize) {
 
   if (myValueSize <= sizeof(stackBuffer))
     buffer = stackBuffer;
-  else 
+  else
     buffer = malloc(myValueSize);
 
   // do stuff with the buffer
@@ -1246,7 +1246,7 @@ void radar10978247_positive(int myValueSize) {
 
   if (myValueSize <= sizeof(stackBuffer))
     buffer = stackBuffer;
-  else 
+  else
     buffer = malloc(myValueSize);
 
   // do stuff with the buffer
@@ -1254,7 +1254,7 @@ void radar10978247_positive(int myValueSize) {
     return;
   else
     return; // expected-warning {{leak}}
-}
+}
 // <rdar://problem/11269741> Previously this triggered a false positive
 // because malloc() is known to return uninitialized memory and the binding
 // of 'o' to 'p->n' was not getting propertly handled.  Now we report a leak.
@@ -1698,7 +1698,7 @@ void testReallocEscaped(void **memory) {
 void *smallocNoWarn(size_t size) {
   if (size == 0) {
     return malloc(1); // this branch is never called
-  } 
+  }
   else {
     return malloc(size);
   }
@@ -1777,21 +1777,12 @@ void freeFunctionPtr() {
   free((void *)fnptr); // expected-warning {{Argument to free() is a function pointer}}
 }
 
-// Enabling the malloc checker enables some of the buffer-checking portions
-// of the C-string checker.
-void cstringchecker_bounds_nocrash() {
-  char *p = malloc(2);
-  strncpy(p, "AAA", sizeof("AAA")); // expected-warning {{Size argument is greater than the length of the destination buffer}}
-
-  free(p);
-}
-
 void allocateSomeMemory(void *offendingParameter, void **ptr) {
   *ptr = malloc(1);
 }
 
 void testNoCrashOnOffendingParameter() {
-  // "extern" is necessary to avoid unrelated warnings 
+  // "extern" is necessary to avoid unrelated warnings
   // on passing uninitialized value.
   extern void *offendingParameter;
   void* ptr;

clang/test/Analysis/string.c

@@ -31,6 +31,8 @@ typedef typeof(sizeof(int)) size_t;
 void clang_analyzer_eval(int);
 
 int scanf(const char *restrict format, ...);
+void *malloc(size_t);
+void free(void *);
 
 //===----------------------------------------------------------------------===
 // strlen()
@@ -110,7 +112,7 @@ void strlen_global() {
   if (a == 0) {
     clang_analyzer_eval(b == 0); // expected-warning{{TRUE}}
     // Make sure clang_analyzer_eval does not invalidate globals.
-    clang_analyzer_eval(strlen(global_str) == 0); // expected-warning{{TRUE}}    
+    clang_analyzer_eval(strlen(global_str) == 0); // expected-warning{{TRUE}}
   }
 
   // Call a function with unknown effects, which should invalidate globals.
@@ -309,11 +311,13 @@ void strcpy_effects(char *x, char *y) {
   clang_analyzer_eval(globalInt == 42); // expected-warning{{TRUE}}
 }
 
+#ifndef SUPPRESS_OUT_OF_BOUND
 void strcpy_overflow(char *y) {
   char x[4];
   if (strlen(y) == 4)
     strcpy(x, y); // expected-warning{{String copy function overflows destination buffer}}
 }
+#endif
 
 void strcpy_no_overflow(char *y) {
   char x[4];
@@ -348,11 +352,13 @@ void stpcpy_effect(char *x, char *y) {
   clang_analyzer_eval(a == x[0]); // expected-warning{{UNKNOWN}}
 }
 
+#ifndef SUPPRESS_OUT_OF_BOUND
 void stpcpy_overflow(char *y) {
   char x[4];
   if (strlen(y) == 4)
     stpcpy(x, y); // expected-warning{{String copy function overflows destination buffer}}
 }
+#endif
 
 void stpcpy_no_overflow(char *y) {
   char x[4];
@@ -403,6 +409,7 @@ void strcat_effects(char *y) {
   clang_analyzer_eval((int)strlen(x) == (orig_len + strlen(y))); // expected-warning{{TRUE}}
 }
 
+#ifndef SUPPRESS_OUT_OF_BOUND
 void strcat_overflow_0(char *y) {
   char x[4] = "12";
   if (strlen(y) == 4)
@@ -420,6 +427,7 @@ void strcat_overflow_2(char *y) {
   if (strlen(y) == 2)
     strcat(x, y); // expected-warning{{String copy function overflows destination buffer}}
 }
+#endif
 
 void strcat_no_overflow(char *y) {
   char x[5] = "12";
@@ -496,6 +504,15 @@ void strncpy_effects(char *x, char *y) {
   clang_analyzer_eval(a == x[0]); // expected-warning{{UNKNOWN}}
 }
 
+#ifndef SUPPRESS_OUT_OF_BOUND
+// Enabling the malloc checker enables some of the buffer-checking portions
+// of the C-string checker.
+void cstringchecker_bounds_nocrash() {
+  char *p = malloc(2);
+  strncpy(p, "AAA", sizeof("AAA")); // expected-warning {{Size argument is greater than the length of the destination buffer}}
+  free(p);
+}
+
 void strncpy_overflow(char *y) {
   char x[4];
   if (strlen(y) == 4)
@@ -516,6 +533,7 @@ void strncpy_no_overflow2(char *y, int n) {
   if (strlen(y) == 3)
     strncpy(x, y, n); // expected-warning{{Size argument is greater than the length of the destination buffer}}
 }
+#endif
 
 void strncpy_truncate(char *y) {
   char x[4];
@@ -592,6 +610,7 @@ void strncat_effects(char *y) {
   clang_analyzer_eval(strlen(x) == (orig_len + strlen(y))); // expected-warning{{TRUE}}
 }
 
+#ifndef SUPPRESS_OUT_OF_BOUND
 void strncat_overflow_0(char *y) {
   char x[4] = "12";
   if (strlen(y) == 4)
@@ -615,6 +634,8 @@ void strncat_overflow_3(char *y) {
   if (strlen(y) == 4)
     strncat(x, y, 2); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
 }
+#endif
+
 void strncat_no_overflow_1(char *y) {
   char x[5] = "12";
   if (strlen(y) == 2)
@@ -632,6 +653,7 @@ void strncat_symbolic_dst_length(char *dst) {
   clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{TRUE}}
 }
 
+#ifndef SUPPRESS_OUT_OF_BOUND
 void strncat_symbolic_src_length(char *src) {
   char dst[8] = "1234";
   strncat(dst, src, 3);
@@ -649,6 +671,7 @@ void strncat_unknown_src_length(char *src, int offset) {
   char dst2[8] = "1234";
   strncat(dst2, &src[offset], 4); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
 }
+#endif
 
 // There is no strncat_unknown_dst_length because if we can't get a symbolic
 // length for the "before" strlen, we won't be able to set one for "after".