[clang-tidy] More strict on matching the standard memset function in memset-usage check.

[hokein]

Summary

The check assumed the matched function call has 3 arguments, but the matcher didn't guaranteed that.

[njames93]

If you want to be super explicit. Why not add parameterCountIs(3)?

[hokein]

add parameterCountIs(3).

[hokein]

In D83301#2137140, @njames93 wrote:

If you want to be super explicit. Why not add parameterCountIs(3)?

good idea.

[hokein]

thanks for the review.

clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp

 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace bugprone {
 
 void SuspiciousMemsetUsageCheck::registerMatchers(MatchFinder *Finder) {
-  // Note: void *memset(void *buffer, int fill_char, size_t byte_count);
+  // Match the standard memset:
+  // void *memset(void *buffer, int fill_char, size_t byte_count);
+  auto MemsetDecl =
+      functionDecl(hasName("::memset"),
+                   parameterCountIs(3),
+                   hasParameter(0, hasType(pointerType(pointee(voidType())))),
+                   hasParameter(1, hasType(isInteger())),
+                   hasParameter(2, hasType(isInteger())));
+
   // Look for memset(x, '0', z). Probably memset(x, 0, z) was intended.
   Finder->addMatcher(
       callExpr(
-          callee(functionDecl(hasName("::memset"))),
+          callee(MemsetDecl),
           hasArgument(1, characterLiteral(equals(static_cast<unsigned>('0')))
                              .bind("char-zero-fill")),
           unless(
               eachOf(hasArgument(0, anyOf(hasType(pointsTo(isAnyCharacter())),
                                           hasType(arrayType(hasElementType(
                                               isAnyCharacter()))))),
                      isInTemplateInstantiation()))),
       this);
 
   // Look for memset with an integer literal in its fill_char argument.
   // Will check if it gets truncated.
-  Finder->addMatcher(callExpr(callee(functionDecl(hasName("::memset"))),
+  Finder->addMatcher(callExpr(callee(MemsetDecl),
                               hasArgument(1, integerLiteral().bind("num-fill")),
                               unless(isInTemplateInstantiation())),
                      this);
 
   // Look for memset(x, y, 0) as that is most likely an argument swap.
   Finder->addMatcher(
-      callExpr(callee(functionDecl(hasName("::memset"))),
+      callExpr(callee(MemsetDecl),
                unless(hasArgument(1, anyOf(characterLiteral(equals(
                                                static_cast<unsigned>('0'))),
                                            integerLiteral()))),
                unless(isInTemplateInstantiation()))
           .bind("call"),
       this);
 }
 

clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memset-usage.cpp

// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, potentially swapped arguments [bugprone-suspicious-memset-usage]
   // Don't warn when the fill char and the length are both known to be
   // zero.  No bug is possible.
   memset(p, 0, v);
 
   // -1 is clearly not a length by virtue of being negative, so no warning
   // despite v == 0.
   memset(p, -1, v);
 }
+
+void *memset(int);
+void NoCrash() {
+  memset(1);
+}