[clang] b7586af - [analyzer][StdLibraryFunctionsChecker] Remove strcasecmp

[Gabor Marton via cfe-commits]

Author: Gabor Marton
Date: 2020-09-10T12:29:39+02:00
New Revision: b7586afc4dcddd1abc70724585c3eb3857e27f43

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

LOG: [analyzer][StdLibraryFunctionsChecker] Remove strcasecmp

There are 2 reasons to remove strcasecmp and strncasecmp.
1) They are also modeled in CStringChecker and the related argumentum
   contraints are checked there.
2) The argument constraints are checked in CStringChecker::evalCall.
   This is fundamentally flawed, they should be checked in checkPreCall.
   Even if we set up CStringChecker as a weak dependency for
   StdLibraryFunctionsChecker then the latter reports the warning always.
   Besides, CStringChecker fails to discover the constraint violation
   before the call, so, its evalCall returns with `true` and then
   StdCLibraryFunctions also tries to evaluate, this causes an assertion
   in CheckerManager.

Either we fix CStringChecker to handle the call prerequisites in
checkPreCall, or we must not evaluate any pure functions in
StdCLibraryFunctions that are also handled in CStringChecker.
We do the latter in this patch.

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

Added: 
    clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c

Modified: 
    clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
    clang/test/Analysis/std-c-library-functions-POSIX.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index b71c19a80da9..c6c37a85306e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1676,22 +1676,6 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
                                               RetType{IntTy}, NoEvalCall)
                                           .ArgConstraint(NotNull(ArgNo(0))));
 
-    // int strcasecmp(const char *s1, const char *s2);
-    addToFunctionSummaryMap("strcasecmp",
-                            Summary(ArgTypes{ConstCharPtrTy, ConstCharPtrTy},
-                                    RetType{IntTy}, EvalCallAsPure)
-                                .ArgConstraint(NotNull(ArgNo(0)))
-                                .ArgConstraint(NotNull(ArgNo(1))));
-
-    // int strncasecmp(const char *s1, const char *s2, size_t n);
-    addToFunctionSummaryMap(
-        "strncasecmp", Summary(ArgTypes{ConstCharPtrTy, ConstCharPtrTy, SizeTy},
-                               RetType{IntTy}, EvalCallAsPure)
-                           .ArgConstraint(NotNull(ArgNo(0)))
-                           .ArgConstraint(NotNull(ArgNo(1)))
-                           .ArgConstraint(ArgumentCondition(
-                               2, WithinRange, Range(0, SizeMax))));
-
     // int fileno(FILE *stream);
     addToFunctionSummaryMap(
         "fileno", Summary(ArgTypes{FilePtrTy}, RetType{IntTy}, NoEvalCall)

diff  --git a/clang/test/Analysis/std-c-library-functions-POSIX.c b/clang/test/Analysis/std-c-library-functions-POSIX.c
index c2c98df86489..9285aee6178b 100644
--- a/clang/test/Analysis/std-c-library-functions-POSIX.c
+++ b/clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -63,8 +63,6 @@
 // CHECK: Loaded summary for: void rewinddir(DIR *dir)
 // CHECK: Loaded summary for: void seekdir(DIR *dirp, long loc)
 // CHECK: Loaded summary for: int rand_r(unsigned int *seedp)
-// CHECK: Loaded summary for: int strcasecmp(const char *s1, const char *s2)
-// CHECK: Loaded summary for: int strncasecmp(const char *s1, const char *s2, size_t n)
 // CHECK: Loaded summary for: int fileno(FILE *stream)
 // CHECK: Loaded summary for: int fseeko(FILE *stream, off_t offset, int whence)
 // CHECK: Loaded summary for: off_t ftello(FILE *stream)
@@ -195,8 +193,6 @@ FILE *fdopen(int fd, const char *mode);
 void rewinddir(DIR *dir);
 void seekdir(DIR *dirp, long loc);
 int rand_r(unsigned int *seedp);
-int strcasecmp(const char *s1, const char *s2);
-int strncasecmp(const char *s1, const char *s2, size_t n);
 int fileno(FILE *stream);
 int fseeko(FILE *stream, off_t offset, int whence);
 off_t ftello(FILE *stream);

diff  --git a/clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c b/clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c
new file mode 100644
index 000000000000..37425e4e3e16
--- /dev/null
+++ b/clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c
@@ -0,0 +1,21 @@
+// This test case crashes if strncasecmp is modeled in StdCLibraryFunctions.
+// Either we fix CStringChecker to handle the call prerequisites in
+// checkPreCall, or we must not evaluate any pure functions in
+// StdCLibraryFunctions that are also handled in CStringChecker.
+
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=unix.cstring.NullArg \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -triple x86_64-unknown-linux-gnu \
+// RUN:   -verify
+
+typedef __typeof(sizeof(int)) size_t;
+int strncasecmp(const char *s1, const char *s2, size_t n);
+
+int strncasecmp_null_argument(char *a, size_t n) {
+  char *b = 0;
+  return strncasecmp(a, b, n); // expected-warning{{Null pointer passed as 2nd argument to string comparison function}}
+}