NoQ updated this revision to Diff 178959.
NoQ added a comment.
Fxd.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55873/new/
https://reviews.llvm.org/D55873
Files:
lib/StaticAnalyzer/Checkers/CStringChecker.cpp
test/Analysis/string.cpp
Index: test/Analysis/string.cpp
===================================================================
--- /dev/null
+++ test/Analysis/string.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s
+
+// expected-no-diagnostics
+
+// Test functions that are called "memcpy" but aren't the memcpy
+// we're looking for. Unfortunately, this test cannot be put into
+// a namespace. The out-of-class weird memcpy needs to be recognized
+// as a normal C function for the test to make sense.
+typedef __typeof(sizeof(int)) size_t;
+void *memcpy(void *, const void *, size_t);
+
+struct S {
+ static S s1, s2;
+
+ // A weird overload within the class that accepts a structure reference
+ // instead of a pointer.
+ void memcpy(void *, const S &, size_t);
+ void test_in_class_weird_memcpy() {
+ memcpy(this, s2, 1); // no-crash
+ }
+};
+
+// A similarly weird overload outside of the class.
+void *memcpy(void *, const S &, size_t);
+
+void test_out_of_class_weird_memcpy() {
+ memcpy(&S::s1, S::s2, 1); // no-crash
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -188,7 +188,7 @@
const Expr *Buf,
const char *message = nullptr,
bool WarnAboutSize = false) const {
- // This is a convenience override.
+ // This is a convenience overload.
return CheckBufferAccess(C, state, Size, Buf, nullptr, message, nullptr,
WarnAboutSize);
}
@@ -2254,64 +2254,86 @@
// The driver method, and other Checker callbacks.
//===----------------------------------------------------------------------===//
-bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
+static CStringChecker::FnCheck identifyCall(const CallExpr *CE,
+ CheckerContext &C) {
const FunctionDecl *FDecl = C.getCalleeDecl(CE);
-
if (!FDecl)
- return false;
+ return nullptr;
+
+ // Pro-actively check that argument types are safe to do arithmetic upon.
+ // We do not want to crash if someone accidentally passes a structure
+ // into, say, a C++ overload of any of these functions.
+ if (isCPPStdLibraryFunction(FDecl, "copy")) {
+ if (CE->getNumArgs() < 3 || !CE->getArg(2)->getType()->isPointerType())
+ return nullptr;
+ return &CStringChecker::evalStdCopy;
+ } else if (isCPPStdLibraryFunction(FDecl, "copy_backward")) {
+ if (CE->getNumArgs() < 3 || !CE->getArg(2)->getType()->isPointerType())
+ return nullptr;
+ return &CStringChecker::evalStdCopyBackward;
+ } else {
+ // An umbrella check for all C library functions.
+ for (auto I: CE->arguments()) {
+ QualType T = I->getType();
+ if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
+ return nullptr;
+ }
+ }
// FIXME: Poorly-factored string switches are slow.
- FnCheck evalFunction = nullptr;
if (C.isCLibraryFunction(FDecl, "memcpy"))
- evalFunction = &CStringChecker::evalMemcpy;
+ return &CStringChecker::evalMemcpy;
else if (C.isCLibraryFunction(FDecl, "mempcpy"))
- evalFunction = &CStringChecker::evalMempcpy;
+ return &CStringChecker::evalMempcpy;
else if (C.isCLibraryFunction(FDecl, "memcmp"))
- evalFunction = &CStringChecker::evalMemcmp;
+ return &CStringChecker::evalMemcmp;
else if (C.isCLibraryFunction(FDecl, "memmove"))
- evalFunction = &CStringChecker::evalMemmove;
- else if (C.isCLibraryFunction(FDecl, "memset") ||
- C.isCLibraryFunction(FDecl, "explicit_memset"))
- evalFunction = &CStringChecker::evalMemset;
+ return &CStringChecker::evalMemmove;
+ else if (C.isCLibraryFunction(FDecl, "memset") ||
+ C.isCLibraryFunction(FDecl, "explicit_memset"))
+ return &CStringChecker::evalMemset;
else if (C.isCLibraryFunction(FDecl, "strcpy"))
- evalFunction = &CStringChecker::evalStrcpy;
+ return &CStringChecker::evalStrcpy;
else if (C.isCLibraryFunction(FDecl, "strncpy"))
- evalFunction = &CStringChecker::evalStrncpy;
+ return &CStringChecker::evalStrncpy;
else if (C.isCLibraryFunction(FDecl, "stpcpy"))
- evalFunction = &CStringChecker::evalStpcpy;
+ return &CStringChecker::evalStpcpy;
else if (C.isCLibraryFunction(FDecl, "strlcpy"))
- evalFunction = &CStringChecker::evalStrlcpy;
+ return &CStringChecker::evalStrlcpy;
else if (C.isCLibraryFunction(FDecl, "strcat"))
- evalFunction = &CStringChecker::evalStrcat;
+ return &CStringChecker::evalStrcat;
else if (C.isCLibraryFunction(FDecl, "strncat"))
- evalFunction = &CStringChecker::evalStrncat;
+ return &CStringChecker::evalStrncat;
else if (C.isCLibraryFunction(FDecl, "strlcat"))
- evalFunction = &CStringChecker::evalStrlcat;
+ return &CStringChecker::evalStrlcat;
else if (C.isCLibraryFunction(FDecl, "strlen"))
- evalFunction = &CStringChecker::evalstrLength;
+ return &CStringChecker::evalstrLength;
else if (C.isCLibraryFunction(FDecl, "strnlen"))
- evalFunction = &CStringChecker::evalstrnLength;
+ return &CStringChecker::evalstrnLength;
else if (C.isCLibraryFunction(FDecl, "strcmp"))
- evalFunction = &CStringChecker::evalStrcmp;
+ return &CStringChecker::evalStrcmp;
else if (C.isCLibraryFunction(FDecl, "strncmp"))
- evalFunction = &CStringChecker::evalStrncmp;
+ return &CStringChecker::evalStrncmp;
else if (C.isCLibraryFunction(FDecl, "strcasecmp"))
- evalFunction = &CStringChecker::evalStrcasecmp;
+ return &CStringChecker::evalStrcasecmp;
else if (C.isCLibraryFunction(FDecl, "strncasecmp"))
- evalFunction = &CStringChecker::evalStrncasecmp;
+ return &CStringChecker::evalStrncasecmp;
else if (C.isCLibraryFunction(FDecl, "strsep"))
- evalFunction = &CStringChecker::evalStrsep;
+ return &CStringChecker::evalStrsep;
else if (C.isCLibraryFunction(FDecl, "bcopy"))
- evalFunction = &CStringChecker::evalBcopy;
+ return &CStringChecker::evalBcopy;
else if (C.isCLibraryFunction(FDecl, "bcmp"))
- evalFunction = &CStringChecker::evalMemcmp;
- else if (isCPPStdLibraryFunction(FDecl, "copy"))
- evalFunction = &CStringChecker::evalStdCopy;
- else if (isCPPStdLibraryFunction(FDecl, "copy_backward"))
- evalFunction = &CStringChecker::evalStdCopyBackward;
+ return &CStringChecker::evalMemcmp;
else if (C.isCLibraryFunction(FDecl, "bzero") ||
- C.isCLibraryFunction(FDecl, "explicit_bzero"))
- evalFunction = &CStringChecker::evalBzero;
+ C.isCLibraryFunction(FDecl, "explicit_bzero"))
+ return &CStringChecker::evalBzero;
+
+ return nullptr;
+}
+
+bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
+
+ FnCheck evalFunction = identifyCall(CE, C);
// If the callee isn't a string function, let another checker handle it.
if (!evalFunction)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
