martong created this revision.
martong added reviewers: Szelethus, balazske, NoQ, vsavchenko.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, 
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware, xazax.hun, whisperity.
Herald added a project: clang.
martong requested review of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87239

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c


Index: clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c
===================================================================
--- /dev/null
+++ 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}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1674,22 +1674,6 @@
                                               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)


Index: clang/test/Analysis/std-c-library-functions-arg-cstring-dependency.c
===================================================================
--- /dev/null
+++ 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}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1674,22 +1674,6 @@
                                               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)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to