[PATCH] D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy

[András Leitereg via Phabricator via cfe-commits]

leanil created this revision.
leanil added reviewers: dcoughlin, xazax.hun.
Herald added subscribers: a.sidorin, rnkovacs, szepet.

It is safe to copy a string literal to an array which is compile time known to 
be large enough.
This reduces the number of false positives, while (hopefully) not introducing 
any false negatives.


https://reviews.llvm.org/D41384

Files:
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  test/Analysis/security-syntax-checks-no-emit.c


Index: test/Analysis/security-syntax-checks-no-emit.c
===================================================================
--- test/Analysis/security-syntax-checks-no-emit.c
+++ test/Analysis/security-syntax-checks-no-emit.c
@@ -32,3 +32,31 @@
   rand_r(&b);  // no-warning
   random();    // no-warning
 }
+
+#ifdef USE_BUILTINS
+#define BUILTIN(f) __builtin_##f
+#else /* USE_BUILTINS */
+#define BUILTIN(f) f
+#endif /* USE_BUILTINS */
+
+//===----------------------------------------------------------------------===
+// strcpy()
+//===----------------------------------------------------------------------===
+#ifdef VARIANT
+
+#define __strcpy_chk BUILTIN(__strcpy_chk)
+char *__strcpy_chk(char *restrict s1, const char *restrict s2, size_t destlen);
+
+#define strcpy(a, b) __strcpy_chk(a, b, (size_t)-1)
+
+#else /* VARIANT */
+
+#define strcpy BUILTIN(strcpy)
+char *strcpy(char *restrict s1, const char *restrict s2);
+
+#endif /* VARIANT */
+
+void test_strcpy() {
+  char x[5];
+  strcpy(x, "abcd");
+}
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -510,6 +510,18 @@
   if (!checkCall_strCommon(CE, FD))
     return;
 
+  int ArraySize = -1, StrLen = -1;
+  const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
+             *Source = CE->getArg(1)->IgnoreImpCasts();
+  if (const auto *DeclRef = dyn_cast<DeclRefExpr>(Target))
+    if (const auto *Array = dyn_cast<ConstantArrayType>(
+            DeclRef->getDecl()->getType().getTypePtr()))
+      ArraySize = Array->getSize().getLimitedValue();
+  if (const auto *String = dyn_cast<StringLiteral>(Source))
+    StrLen = String->getLength();
+  if (StrLen != -1 && ArraySize >= StrLen + 1)
+    return;
+
   // Issue a warning.
   PathDiagnosticLocation CELoc =
     PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);



Index: test/Analysis/security-syntax-checks-no-emit.c
===================================================================
--- test/Analysis/security-syntax-checks-no-emit.c
+++ test/Analysis/security-syntax-checks-no-emit.c
@@ -32,3 +32,31 @@
   rand_r(&b);	// no-warning
   random();	// no-warning
 }
+
+#ifdef USE_BUILTINS
+#define BUILTIN(f) __builtin_##f
+#else /* USE_BUILTINS */
+#define BUILTIN(f) f
+#endif /* USE_BUILTINS */
+
+//===----------------------------------------------------------------------===
+// strcpy()
+//===----------------------------------------------------------------------===
+#ifdef VARIANT
+
+#define __strcpy_chk BUILTIN(__strcpy_chk)
+char *__strcpy_chk(char *restrict s1, const char *restrict s2, size_t destlen);
+
+#define strcpy(a, b) __strcpy_chk(a, b, (size_t)-1)
+
+#else /* VARIANT */
+
+#define strcpy BUILTIN(strcpy)
+char *strcpy(char *restrict s1, const char *restrict s2);
+
+#endif /* VARIANT */
+
+void test_strcpy() {
+  char x[5];
+  strcpy(x, "abcd");
+}
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -510,6 +510,18 @@
   if (!checkCall_strCommon(CE, FD))
     return;
 
+  int ArraySize = -1, StrLen = -1;
+  const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
+             *Source = CE->getArg(1)->IgnoreImpCasts();
+  if (const auto *DeclRef = dyn_cast<DeclRefExpr>(Target))
+    if (const auto *Array = dyn_cast<ConstantArrayType>(
+            DeclRef->getDecl()->getType().getTypePtr()))
+      ArraySize = Array->getSize().getLimitedValue();
+  if (const auto *String = dyn_cast<StringLiteral>(Source))
+    StrLen = String->getLength();
+  if (StrLen != -1 && ArraySize >= StrLen + 1)
+    return;
+
   // Issue a warning.
   PathDiagnosticLocation CELoc =
     PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits