[PATCH] D55873: [analyzer] CStringChecker: Fix a crash when an argument of a weird type is encountered.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 178938.
NoQ marked an inline comment as done.
NoQ added a comment.

Indeed :)


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);
   }
@@ -2255,11 +2255,20 @@
 //===----------------------------------------------------------------------===//
 
 bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
-  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
 
+  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
   if (!FDecl)
     return false;
 
+  // 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.
+  for (auto I: CE->arguments()) {
+    QualType T = I->getType();
+    if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
+      return false;
+  }
+
   // FIXME: Poorly-factored string switches are slow.
   FnCheck evalFunction = nullptr;
   if (C.isCLibraryFunction(FDecl, "memcpy"))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55873.178938.patch
Type: text/x-patch
Size: 2548 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181219/8db687eb/attachment.bin>