rnkovacs updated this revision to Diff 97917.
rnkovacs edited the summary of this revision.
rnkovacs added a comment.
- Removed case related to virtual pointers as there is a diagnostic flag for
that.
- Added case warning for calls on classes with a constructor or destructor.
Changed tests and docs accordingly.
https://reviews.llvm.org/D32700
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/SuspiciousMemsetUsageCheck.cpp
clang-tidy/misc/SuspiciousMemsetUsageCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-suspicious-memset-usage.rst
test/clang-tidy/misc-suspicious-memset-usage.cpp
Index: test/clang-tidy/misc-suspicious-memset-usage.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-suspicious-memset-usage.cpp
@@ -0,0 +1,126 @@
+// RUN: %check_clang_tidy %s misc-suspicious-memset-usage %t
+
+void *memset(void *, int, __SIZE_TYPE__);
+
+template <typename T>
+void mtempl(int *ptr) {
+ memset(ptr, '0', sizeof(T));
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: memset fill value is char '0', potentially mistaken for int 0 [misc-suspicious-memset-usage]
+ // CHECK-FIXES: memset(ptr, 0, sizeof(T));
+ memset(ptr, 256, sizeof(T));
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: memset fill value is out of unsigned character range, gets truncated [misc-suspicious-memset-usage]
+}
+
+void FillValue() {
+ int i[5] = {1, 2, 3, 4, 5};
+ int *p = i;
+ int l = 5;
+ char z = '1';
+ char *c = &z;
+
+ memset(p, '0', l);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: memset fill value is char '0', potentially mistaken for int 0 [misc-suspicious-memset-usage]
+ // CHECK-FIXES: memset(p, 0, l);
+ memset(p, 0xabcd, l);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: memset fill value is out of unsigned character range, gets truncated [misc-suspicious-memset-usage]
+
+#define M_CHAR_ZERO memset(p, '0', l);
+ M_CHAR_ZERO
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset fill value is char '0', potentially mistaken for int 0 [misc-suspicious-memset-usage]
+#define M_OUTSIDE_RANGE memset(p, 0xabcd, l);
+ M_OUTSIDE_RANGE
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset fill value is out of unsigned character range, gets truncated [misc-suspicious-memset-usage]
+
+ memset(p, '2', l);
+ memset(p, 0, l);
+ memset(c, '0', 1);
+ memset(p, 0x00, l);
+ mtempl<int>(p);
+}
+
+struct Ctor {
+ Ctor();
+ void fCtor() {
+ memset(this, 0, sizeof(*this));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: memset used on class with user-defined constructor or destructor [misc-suspicious-memset-usage]
+ }
+};
+
+struct Dtor {
+ ~Dtor();
+ void fDtor() {
+ memset(this, 0, sizeof(*this));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: memset used on class with user-defined constructor or destructor [misc-suspicious-memset-usage]
+ }
+};
+
+struct Both {
+ Both();
+ ~Both();
+ void fBoth() {
+ memset(this, 0, sizeof(*this));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: memset used on class with user-defined constructor or destructor [misc-suspicious-memset-usage]
+ }
+};
+
+struct Neither {
+ void fNeither() {
+ memset(this, 0, sizeof(*this));
+ }
+};
+
+void Destination() {
+ Ctor a;
+ memset(&a, 0, 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: memset used on class with user-defined constructor or destructor [misc-suspicious-memset-usage]
+ Dtor b;
+ memset(&b, 0, 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: memset used on class with user-defined constructor or destructor [misc-suspicious-memset-usage]
+ Both c;
+ memset(&c, 0, 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: memset used on class with user-defined constructor or destructor [misc-suspicious-memset-usage]
+ Neither d;
+ memset(&d, 0, 1);
+}
+
+struct Inside {
+ Inside() {
+ memset(this, 0, sizeof(*this));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: memset used on class with user-defined constructor or destructor [misc-suspicious-memset-usage]
+ }
+ ~Inside() {
+ memset(this, 0, sizeof(*this));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: memset used on class with user-defined constructor or destructor [misc-suspicious-memset-usage]
+ }
+};
+
+struct Base {
+ Base();
+ ~Base();
+};
+
+struct Derived : public Base {
+ void fDerived() {
+ memset(this, 0, sizeof(*this));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: memset used on class with user-defined constructor or destructor [misc-suspicious-memset-usage]
+ }
+};
+
+struct Lambda {
+ Lambda();
+ ~Lambda();
+ void fLambda() {
+ [this] { memset(this, 0, sizeof(*this)); };
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: memset used on class with user-defined constructor or destructor [misc-suspicious-memset-usage]
+ }
+};
+
+struct Outer {
+ Outer();
+ ~Outer();
+ struct Inner {
+ void fInner() {
+ memset(this, 0, sizeof(*this));
+ }
+ };
+};
Index: docs/clang-tidy/checks/misc-suspicious-memset-usage.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-suspicious-memset-usage.rst
@@ -0,0 +1,54 @@
+.. title:: clang-tidy - misc-suspicious-memset-usage
+
+misc-suspicious-memset-usage
+============================
+
+This check finds ``memset()`` calls with potential mistakes in their arguments.
+Considering the function as ``void* memset(void* destination, int fill_value,
+size_t byte_count)``, the following cases are covered:
+
+**Case 1: Fill value is a character '0'**
+
+Filling up a memory area with ASCII code 48 characters is not customary,
+possibly integer zeroes were intended instead.
+The check offers a replacement of ``'0'`` with ``0``. Memsetting character
+pointers with ``'0'`` is allowed.
+
+**Case 2: Fill value is truncated**
+
+Memset converts ``fill_value`` to ``unsigned char`` before using it. If
+``fill_value`` is out of unsigned character range, it gets truncated
+and memory will not contain the desired pattern.
+
+**Case 3: Destination is a class with a constructor/destructor**
+
+Calling ``memset()`` on a non-POD type object risks undefined behavior.
+The check warns if ``destination`` points to a class with a non-trivial,
+user-defined constructor or destructor.
+
+Examples:
+
+.. code-block:: c++
+
+ void SuspiciousFillValue() {
+ int i[5] = {1, 2, 3, 4, 5};
+ int *ip = i;
+ char c = '1';
+ char *cp = &z;
+
+ // Case 1
+ memset(ip, '0', 1); // suspicious
+ memset(cp, '0', 1); // OK
+
+ // Case 2
+ memset(ip, 0xabcd, 1); // fill value gets truncated
+ memset(ip, 0x00, 1); // OK
+ }
+
+ // Case 3
+ struct NonTrivial {
+ NonTrivial(); // user-defined constructor
+ void f() {
+ memset(this, 0, sizeof(*this)); // no guarantees
+ }
+ };
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -86,6 +86,7 @@
misc-string-integer-assignment
misc-string-literal-with-embedded-nul
misc-suspicious-enum-usage
+ misc-suspicious-memset-usage
misc-suspicious-missing-comma
misc-suspicious-semicolon
misc-suspicious-string-compare
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -72,6 +72,11 @@
Finds perfect forwarding constructors that can unintentionally hide copy or move constructors.
+- New `misc-suspicious-memset-usage
+ <http://clang.llvm.org/extra/clang-tidy/checks/misc-suspicious-memset-usage.html>`_ check
+
+ Finds ``memset()`` calls with potential mistakes in their arguments.
+
- New `modernize-replace-random-shuffle
<http://clang.llvm.org/extra/clang-tidy/checks/modernize-replace-random-shuffle.html>`_ check
Index: clang-tidy/misc/SuspiciousMemsetUsageCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/SuspiciousMemsetUsageCheck.h
@@ -0,0 +1,35 @@
+//===--- SuspiciousMemsetUsageCheck.h - clang-tidy---------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_MEMSET_USAGE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_MEMSET_USAGE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds memset calls with potential mistakes in their arguments.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-suspicious-memset-usage.html
+class SuspiciousMemsetUsageCheck : public ClangTidyCheck {
+public:
+ SuspiciousMemsetUsageCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_MEMSET_USAGE_H
Index: clang-tidy/misc/SuspiciousMemsetUsageCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/SuspiciousMemsetUsageCheck.cpp
@@ -0,0 +1,97 @@
+//===--- SuspiciousMemsetUsageCheck.cpp - clang-tidy-----------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "SuspiciousMemsetUsageCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void SuspiciousMemsetUsageCheck::registerMatchers(MatchFinder *Finder) {
+ const auto HasCtorOrDtor =
+ eachOf(hasMethod(cxxConstructorDecl(unless(anyOf(
+ isCopyConstructor(), isMoveConstructor(), isImplicit())))),
+ hasMethod(cxxDestructorDecl(unless(isImplicit()))));
+
+ Finder->addMatcher(
+ callExpr(
+ callee(functionDecl(hasName("::memset"))),
+ hasArgument(1, characterLiteral(equals(static_cast<unsigned>('0')))
+ .bind("char-zero-fill")),
+ unless(eachOf(hasArgument(0, hasType(pointsTo(isAnyCharacter()))),
+ isInTemplateInstantiation()))),
+ this);
+
+ Finder->addMatcher(callExpr(callee(functionDecl(hasName("::memset"))),
+ hasArgument(1, integerLiteral().bind("num-fill")),
+ unless(isInTemplateInstantiation())),
+ this);
+
+ Finder->addMatcher(
+ callExpr(callee(functionDecl(hasName("::memset"))),
+ hasArgument(0, hasType(pointsTo(cxxRecordDecl(anyOf(
+ HasCtorOrDtor, isDerivedFrom(cxxRecordDecl(
+ HasCtorOrDtor))))))),
+ unless(isInTemplateInstantiation()))
+ .bind("call-ctor-dtor"),
+ this);
+}
+
+void SuspiciousMemsetUsageCheck::check(const MatchFinder::MatchResult &Result) {
+ // Case 1: Fill value of memset() is a character '0'.
+ // Possibly an integer zero was intended.
+ if (const auto *CharZeroFill =
+ Result.Nodes.getNodeAs<CharacterLiteral>("char-zero-fill")) {
+
+ SourceRange CharRange = CharZeroFill->getSourceRange();
+ auto Diag =
+ diag(CharZeroFill->getLocStart(), "memset fill value is char '0', "
+ "potentially mistaken for int 0");
+
+ // Only suggest a fix if no macros are involved.
+ if (CharRange.getBegin().isMacroID())
+ return;
+ Diag << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(CharRange), "0");
+ }
+
+ // Case 2: Fill value of memset() is larger in size than an
+ // unsigned char, which means it gets truncated during conversion.
+ else if (const auto *NumFill =
+ Result.Nodes.getNodeAs<IntegerLiteral>("num-fill")) {
+
+ llvm::APSInt NumValue;
+ const auto UCharMax = (1 << Result.Context->getCharWidth()) - 1;
+ if (!NumFill->EvaluateAsInt(NumValue, *Result.Context) ||
+ (NumValue >= 0 && NumValue <= UCharMax))
+ return;
+
+ diag(NumFill->getLocStart(), "memset fill value is out of unsigned "
+ "character range, gets truncated");
+ }
+
+ // Case 3: Destination is a class with a user-defined constructor
+ // or destructor. As memset() is only guaranteed to work on POD type
+ // objects, this should be avoided.
+ else if (const auto *Call =
+ Result.Nodes.getNodeAs<CallExpr>("call-ctor-dtor")) {
+
+ const Expr *Dest = Call->getArg(0);
+ diag(Dest->getLocStart(), "memset used on class with user-defined "
+ "constructor or destructor");
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -40,6 +40,7 @@
#include "StringIntegerAssignmentCheck.h"
#include "StringLiteralWithEmbeddedNulCheck.h"
#include "SuspiciousEnumUsageCheck.h"
+#include "SuspiciousMemsetUsageCheck.h"
#include "SuspiciousMissingCommaCheck.h"
#include "SuspiciousSemicolonCheck.h"
#include "SuspiciousStringCompareCheck.h"
@@ -66,6 +67,8 @@
CheckFactories.registerCheck<AssertSideEffectCheck>(
"misc-assert-side-effect");
CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
+ CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>(
+ "misc-suspicious-memset-usage");
CheckFactories.registerCheck<UnconventionalAssignOperatorCheck>(
"misc-unconventional-assign-operator");
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -4,6 +4,7 @@
ArgumentCommentCheck.cpp
AssertSideEffectCheck.cpp
MisplacedConstCheck.cpp
+ SuspiciousMemsetUsageCheck.cpp
UnconventionalAssignOperatorCheck.cpp
BoolPointerImplicitConversionCheck.cpp
DanglingHandleCheck.cpp
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
