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

George Karpenkov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 28 11:10:51 PST 2018


george.karpenkov added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:100
   void checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD);
+  void checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE, const FunctionDecl *FD);
   void checkCall_rand(const CallExpr *CE, const FunctionDecl *FD);
----------------
80 chars


================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:165
+    .Case("strncat", &WalkAST::checkDeprecatedOrUnsafeBufferHandling)
+    .Case("memset", &WalkAST::checkDeprecatedOrUnsafeBufferHandling)
     .Case("drand48", &WalkAST::checkCall_rand)
----------------
That's a lot of duplicated `WalkAST::checkDeprecatedOrUnsafeBufferHandling`. Could that be simplified?


================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:618
+  StringRef Name = FD->getIdentifier()->getName();
+  int ArgIndex = llvm::StringSwitch<int>(Name)
+                     .Case("sprintf", 1)
----------------
That's a lot of duplication of 1/0/-1.


And also 1/0/-1 are cryptic symbols, why not use an enum with a descriptive name?
Maybe use
`.Cases("sprintf", "vsprintf", "vfscanf", WARN_UNSAFE)` ?


https://reviews.llvm.org/D35068





More information about the cfe-commits mailing list