[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

[Daniel Kolozsvari via Phabricator via cfe-commits]

koldaniel updated this revision to Diff 192120.
koldaniel added a comment.

Bug fixing: faulty handling of built-in functions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D35068/new/

https://reviews.llvm.org/D35068

Files:
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp


Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -738,7 +738,7 @@
     return;
 
   // Issue a warning. ArgIndex == -1: Deprecated but not unsafe (has size
-  // restrictions).
+  // restrictions), ArgIndex == -2: built-in functions, etc.
   enum { DEPR_ONLY = -1, UNKNOWN_CALL = -2 };
   StringRef Name = FD->getIdentifier()->getName();
   int ArgIndex =
@@ -750,8 +750,7 @@
                  "memmove", "memset", "strncpy", "strncat", DEPR_ONLY)
           .Default(UNKNOWN_CALL);
 
-  assert(ArgIndex != UNKNOWN_CALL && "Unsupported function");
-  bool BoundsProvided = ArgIndex == DEPR_ONLY;
+  bool BoundsProvided = ArgIndex < 0;
 
   if (!BoundsProvided) {
     // Currently we only handle (not wide) string literals. It is possible to 
do
@@ -781,8 +780,12 @@
 
   Out2 << "security checks introduced "
           "in the C11 standard. Replace with analogous functions that "
-          "support length arguments or provides boundary checks such as '"
-       << Name << "_s' in case of C11";
+          "support length arguments or provides boundary checks";
+
+  // We know the function has a secure version introduced in C11.
+  if(ArgIndex != UNKNOWN_CALL) {
+    Out2 << " such as '" << Name << "_s' in case of C11";
+  }
 
   PathDiagnosticLocation CELoc =
       PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);



Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -738,7 +738,7 @@
     return;
 
   // Issue a warning. ArgIndex == -1: Deprecated but not unsafe (has size
-  // restrictions).
+  // restrictions), ArgIndex == -2: built-in functions, etc.
   enum { DEPR_ONLY = -1, UNKNOWN_CALL = -2 };
   StringRef Name = FD->getIdentifier()->getName();
   int ArgIndex =
@@ -750,8 +750,7 @@
                  "memmove", "memset", "strncpy", "strncat", DEPR_ONLY)
           .Default(UNKNOWN_CALL);
 
-  assert(ArgIndex != UNKNOWN_CALL && "Unsupported function");
-  bool BoundsProvided = ArgIndex == DEPR_ONLY;
+  bool BoundsProvided = ArgIndex < 0;
 
   if (!BoundsProvided) {
     // Currently we only handle (not wide) string literals. It is possible to do
@@ -781,8 +780,12 @@
 
   Out2 << "security checks introduced "
           "in the C11 standard. Replace with analogous functions that "
-          "support length arguments or provides boundary checks such as '"
-       << Name << "_s' in case of C11";
+          "support length arguments or provides boundary checks";
+
+  // We know the function has a secure version introduced in C11.
+  if(ArgIndex != UNKNOWN_CALL) {
+    Out2 << " such as '" << Name << "_s' in case of C11";
+  }
 
   PathDiagnosticLocation CELoc =
       PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);

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