gbencze updated this revision to Diff 235900.
gbencze added a comment.
Coding guide and better diagnostic message for padding comparison
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71973/new/
https://reviews.llvm.org/D71973
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.h
clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst
clang-tools-extra/docs/clang-tidy/checks/cert-exp42-c.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
@@ -0,0 +1,373 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t
+
+typedef __SIZE_TYPE__ size_t;
+int memcmp(const void *lhs, const void *rhs, size_t count);
+
+namespace std {
+int memcmp(const void *lhs, const void *rhs, size_t count);
+}
+
+namespace sei_cert_example_exp42_c {
+struct s {
+ char c;
+ int i;
+ char buffer[13];
+};
+
+void noncompliant(const struct s *left, const struct s *right) {
+ if ((left && right) && (0 == memcmp(left, right, sizeof(struct s)))) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: comparing padding data in type
+ // sei_cert_example_exp42_c::s; consider comparing the fields manually
+ }
+}
+
+void compliant(const struct s *left, const struct s *right) {
+ if ((left && right) && (left->c == right->c) && (left->i == right->i) &&
+ (0 == memcmp(left->buffer, right->buffer, 13))) {
+ }
+}
+} // namespace sei_cert_example_exp42_c
+
+namespace sei_cert_example_exp42_c_ex1 {
+#pragma pack(push, 1)
+struct s {
+ char c;
+ int i;
+ char buffer[13];
+};
+#pragma pack(pop)
+
+void compare(const struct s *left, const struct s *right) {
+ if ((left && right) && (0 == memcmp(left, right, sizeof(struct s)))) {
+ // no-warning
+ }
+}
+} // namespace sei_cert_example_exp42_c_ex1
+
+namespace sei_cert_example_oop57_cpp {
+class C {
+ int i;
+
+public:
+ virtual void f();
+};
+
+void f(C &c1, C &c2) {
+ if (!std::memcmp(&c1, &c2, sizeof(C))) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: comparing object representation
+ // of non-standard-layout type sei_cert_example_oop57_cpp::C; consider
+ // using a comparison operator instead
+ }
+}
+} // namespace sei_cert_example_oop57_cpp
+
+namespace trailing_padding {
+struct S {
+ int i;
+ char c;
+};
+
+void test() {
+ S a, b;
+ memcmp(&a, &b, sizeof(S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // trailing_padding::S; consider comparing the fields manually
+ memcmp(&a, &b, sizeof(int));
+ memcmp(&a, &b, sizeof(int) + sizeof(char));
+ memcmp(&a, &b, 2 * sizeof(int));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // trailing_padding::S; consider comparing the fields manually
+}
+} // namespace trailing_padding
+
+namespace inner_padding {
+struct S {
+ char c;
+ int i;
+};
+
+void test() {
+ S a, b;
+ memcmp(&a, &b, sizeof(S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // inner_padding::S; consider comparing the fields manually
+ memcmp(&a, &b, sizeof(char) + sizeof(int));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // inner_padding::S; consider comparing the fields manually
+ memcmp(&a, &b, sizeof(char));
+ memcmp(&a, &b, 2 * sizeof(char));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // inner_padding::S; consider comparing the fields manually
+}
+} // namespace inner_padding
+
+namespace bitfield {
+struct S {
+ int x : 10;
+ int y : 6;
+};
+
+void test() {
+ S a, b;
+ memcmp(&a, &b, sizeof(S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // bitfield::S; consider comparing the fields manually
+ memcmp(&a, &b, 2); // no-warning: no padding in first 2 bytes
+}
+} // namespace bitfield
+
+namespace bitfield_2 {
+struct S {
+ int x : 10;
+ int y : 7;
+};
+
+void test() {
+ S a, b;
+ memcmp(&a, &b, sizeof(S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // bitfield_2::S; consider comparing the fields manually
+ memcmp(&a, &b, 2);
+ memcmp(&a, &b, 3);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // bitfield_2::S; consider comparing the fields manually
+}
+} // namespace bitfield_2
+
+namespace bitfield_3 {
+struct S {
+ int x : 2;
+ int : 0;
+ int y : 6;
+};
+
+void test() {
+ S a, b;
+ memcmp(&a, &b, 1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // bitfield_3::S; consider comparing the fields manually
+}
+} // namespace bitfield_3
+
+namespace array_no_padding {
+struct S {
+ int x;
+ int y;
+};
+
+void test() {
+ S a[3];
+ S b[3];
+ memcmp(a, b, 3 * sizeof(S));
+}
+} // namespace array_no_padding
+
+namespace array_with_padding {
+struct S {
+ int x;
+ char y;
+};
+
+void test() {
+ S a[3];
+ S b[3];
+ memcmp(a, b, 3 * sizeof(S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // array_with_padding::S; consider comparing the fields manually
+}
+} // namespace array_with_padding
+
+namespace unknown_count {
+struct S {
+ char c;
+ int i;
+};
+
+void test(size_t c) {
+ S a;
+ S b;
+ memcmp(&a, &b, c); // no-warning: unknown count value
+}
+} // namespace unknown_count
+
+namespace predeclared_type {
+struct S;
+
+void test(const S *lhs, const S *rhs) {
+ memcmp(lhs, rhs, 1); // no-warning: predeclared type
+}
+} // namespace predeclared_type
+
+namespace padding_after_union {
+struct S {
+ union {
+ char c;
+ short s;
+ } x;
+
+ int y;
+};
+
+void test() {
+ S a;
+ S b;
+ memcmp(&a, &b, sizeof(short));
+ memcmp(&a, &b, sizeof(int));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // padding_after_union::S; consider comparing the fields manually
+ memcmp(&a, &b, sizeof(S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // padding_after_union::S; consider comparing the fields manually
+}
+} // namespace padding_after_union
+
+namespace union_no_padding {
+struct S {
+ union {
+ int a;
+ char b;
+ } x;
+
+ int y;
+};
+
+void test() {
+ S a;
+ S b;
+ memcmp(&a, &b, 2 * sizeof(int));
+ memcmp(&a, &b, sizeof(S));
+}
+} // namespace union_no_padding
+
+namespace padding_in_nested_struct {
+struct S {
+ int a;
+ char b;
+};
+struct T {
+ S x;
+ char y;
+};
+
+void test() {
+ T a, b;
+ memcmp(&a, &b, sizeof(int) + sizeof(char));
+ memcmp(&a, &b, sizeof(int) + 2 * sizeof(char));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // padding_in_nested_struct::T; consider comparing the fields manually
+ memcmp(&a, &b, sizeof(S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // padding_in_nested_struct::T; consider comparing the fields manually
+ memcmp(&a, &b, sizeof(T));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // padding_in_nested_struct::T; consider comparing the fields manually
+}
+} // namespace padding_in_nested_struct
+
+namespace padding_after_nested_struct {
+struct S {
+ char a;
+};
+struct T {
+ S x;
+ int y;
+};
+
+void test() {
+ T a, b;
+ memcmp(&a, &b, sizeof(char));
+ memcmp(&a, &b, sizeof(S));
+ memcmp(&a, &b, sizeof(T));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // padding_after_nested_struct::T; consider comparing the fields manually
+}
+} // namespace padding_after_nested_struct
+
+namespace padding_in_base {
+class Base {
+ char c;
+ int i;
+};
+
+class Derived : public Base {};
+
+class Derived2 : public Derived {};
+
+void testDerived() {
+ Derived a, b;
+ memcmp(&a, &b, sizeof(char));
+ memcmp(&a, &b, sizeof(Base));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // padding_in_base::Derived; consider comparing the fields manually
+ memcmp(&a, &b, sizeof(Derived));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // padding_in_base::Derived; consider comparing the fields manually
+}
+
+void testDerived2() {
+ Derived2 a, b;
+ memcmp(&a, &b, sizeof(char));
+ memcmp(&a, &b, sizeof(Base));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // padding_in_base::Derived2; consider comparing the fields manually
+ memcmp(&a, &b, sizeof(Derived2));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+ // padding_in_base::Derived2; consider comparing the fields manually
+}
+
+} // namespace padding_in_base
+
+namespace no_padding_in_base {
+class Base {
+ int a, b;
+};
+
+class Derived : public Base {};
+
+class Derived2 : public Derived {};
+
+void testDerived() {
+ Derived a, b;
+ memcmp(&a, &b, sizeof(Base));
+ memcmp(&a, &b, sizeof(Derived));
+}
+
+void testDerived2() {
+ Derived2 a, b;
+ memcmp(&a, &b, sizeof(char));
+ memcmp(&a, &b, sizeof(Base));
+ memcmp(&a, &b, sizeof(Derived2));
+}
+} // namespace no_padding_in_base
+
+namespace non_standard_layout {
+class C {
+private:
+ int x;
+
+public:
+ int y;
+};
+
+void test() {
+ C a, b;
+ memcmp(&a, &b, sizeof(C));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation
+ // of non-standard-layout type non_standard_layout::C; consider using a
+ // comparison operator instead
+}
+
+} // namespace non_standard_layout
+
+namespace static_ignored {
+struct S {
+ static char c;
+ int i;
+};
+
+void test() {
+ S a, b;
+ memcmp(&a, &b, sizeof(S));
+}
+
+} // namespace static_ignored
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -72,6 +72,7 @@
`bugprone-string-integer-assignment <bugprone-string-integer-assignment.html>`_, "Yes"
`bugprone-string-literal-with-embedded-nul <bugprone-string-literal-with-embedded-nul.html>`_,
`bugprone-suspicious-enum-usage <bugprone-suspicious-enum-usage.html>`_,
+ `bugprone-suspicious-memory-comparison <bugprone-suspicious-memory-comparison.html>`_,
`bugprone-suspicious-memset-usage <bugprone-suspicious-memset-usage.html>`_, "Yes"
`bugprone-suspicious-missing-comma <bugprone-suspicious-missing-comma.html>`_,
`bugprone-suspicious-semicolon <bugprone-suspicious-semicolon.html>`_, "Yes"
@@ -294,6 +295,7 @@
`cert-dcl59-cpp <cert-dcl59-cpp.html>`_, `google-build-namespaces <google-build-namespaces.html>`_,
`cert-err09-cpp <cert-err09-cpp.html>`_, `misc-throw-by-value-catch-by-reference <misc-throw-by-value-catch-by-reference.html>`_,
`cert-err61-cpp <cert-err61-cpp.html>`_, `misc-throw-by-value-catch-by-reference <misc-throw-by-value-catch-by-reference.html>`_,
+ `cert-exp42-c <cert-exp42-c.html>`_, `bugprone-suspicious-memory-comparison <bugprone-suspicious-memory-comparison.html>`_,
`cert-fio38-c <cert-fio38-c.html>`_, `misc-non-copyable-objects <misc-non-copyable-objects.html>`_,
`cert-msc30-c <cert-msc30-c.html>`_, `cert-msc50-cpp <cert-msc50-cpp.html>`_,
`cert-msc32-c <cert-msc32-c.html>`_, `cert-msc51-cpp <cert-msc51-cpp.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/cert-exp42-c.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-exp42-c.rst
@@ -0,0 +1,9 @@
+.. title:: clang-tidy - cert-exp42-c
+.. meta::
+ :http-equiv=refresh: 5;URL=bugprone-suspicious-memory-comparison.html
+
+cert-exp42-c
+============
+
+The cert-exp42-c check is an alias, please see
+`bugprone-suspicious-memory-comparison <bugprone-suspicious-memory-comparison.html>`_ for more information.
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - bugprone-suspicious-memory-comparison
+
+bugprone-suspicious-memory-comparison
+=====================================
+
+Finds potentially incorrect calls to ``memcmp()`` that compare padding or
+non-standard-layout classes.
+
+This check corresponds to the CERT C Coding Standard rule
+`EXP42-C. Do not compare padding data
+<https://wiki.sei.cmu.edu/confluence/display/c/EXP42-C.+Do+not+compare+padding+data>`_.
+and part of the CERT C++ Coding Standard rule
+`OOP57-CPP. Prefer special member functions and overloaded operators to C Standard Library functions
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP57-CPP.+Prefer+special+member+functions+and+overloaded+operators+to+C+Standard+Library+functions>`_.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -94,6 +94,12 @@
Without the null terminator it can result in undefined behaviour when the
string is read.
+- New :doc:`bugprone-suspicious-memory-comparison
+ <clang-tidy/checks/bugprone-suspicious-memory-comparison>` check.
+
+ Finds potentially incorrect calls to ``memcmp()`` that compare padding or
+ non-standard-layout classes.
+
- New :doc:`cert-mem57-cpp
<clang-tidy/checks/cert-mem57-cpp>` check.
@@ -160,6 +166,11 @@
Finds types that could be made trivially-destructible by removing out-of-line
defaulted destructor declarations.
+- New alias :doc:`cert-exp42-c
+ <clang-tidy/checks/cert-exp42-c>` to
+ :doc:`bugprone-suspicious-memory-comparison
+ <clang-tidy/checks/bugprone-suspicious-memory-comparison>` was added.
+
- Improved :doc:`bugprone-posix-return
<clang-tidy/checks/bugprone-posix-return>` check.
Index: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "../bugprone/UnhandledSelfAssignmentCheck.h"
#include "../bugprone/BadSignalToKillThreadCheck.h"
+#include "../bugprone/SuspiciousMemoryComparisonCheck.h"
#include "../google/UnnamedNamespaceInHeaderCheck.h"
#include "../misc/NewDeleteOverloadsCheck.h"
#include "../misc/NonCopyableObjects.h"
@@ -80,6 +81,9 @@
"cert-dcl16-c");
// ENV
CheckFactories.registerCheck<CommandProcessorCheck>("cert-env33-c");
+ // EXP
+ CheckFactories.registerCheck<bugprone::SuspiciousMemoryComparisonCheck>(
+ "cert-exp42-c");
// FLP
CheckFactories.registerCheck<FloatLoopCounter>("cert-flp30-c");
// FIO
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.h
@@ -0,0 +1,35 @@
+//===--- SuspiciousMemoryComparisonCheck.h - clang-tidy ---------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSMEMORYCOMPARISONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSMEMORYCOMPARISONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds potentially incorrect calls to ``memcmp()`` that compare padding or
+/// non-standard-layout classes.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-memory-comparison.html
+class SuspiciousMemoryComparisonCheck : public ClangTidyCheck {
+public:
+ SuspiciousMemoryComparisonCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSMEMORYCOMPARISONCHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp
@@ -0,0 +1,158 @@
+//===--- SuspiciousMemoryComparisonCheck.cpp - clang-tidy -----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "SuspiciousMemoryComparisonCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+static bool hasPadding(const ASTContext &Ctx, const RecordDecl *RD,
+ uint64_t ComparedBits);
+
+static uint64_t getFieldSize(const FieldDecl &FD, QualType FieldType,
+ const ASTContext &Ctx) {
+ return FD.isBitField() ? FD.getBitWidthValue(Ctx)
+ : Ctx.getTypeSize(FieldType);
+}
+
+static bool hasPaddingInBase(const ASTContext &Ctx, const RecordDecl *RD,
+ uint64_t ComparedBits, uint64_t &TotalSize) {
+ const auto IsNotEmptyBase = [](const CXXBaseSpecifier &Base) {
+ return !Base.getType()->getAsCXXRecordDecl()->isEmpty();
+ };
+
+ if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+ const auto NonEmptyBaseIt = llvm::find_if(CXXRD->bases(), IsNotEmptyBase);
+ if (NonEmptyBaseIt != CXXRD->bases().end()) {
+ assert(llvm::count_if(CXXRD->bases(), IsNotEmptyBase) == 1 &&
+ "RD is expected to be a standard layout type");
+
+ const CXXRecordDecl *BaseRD =
+ NonEmptyBaseIt->getType()->getAsCXXRecordDecl();
+ uint64_t SizeOfBase = Ctx.getTypeSize(BaseRD->getTypeForDecl());
+ TotalSize += SizeOfBase;
+
+ // check if comparing padding in base
+ if (hasPadding(Ctx, BaseRD, ComparedBits))
+ return true;
+ }
+ }
+
+ return false;
+}
+
+static bool hasPaddingBetweenFields(const ASTContext &Ctx, const RecordDecl *RD,
+ uint64_t ComparedBits,
+ uint64_t &TotalSize) {
+ for (const auto &Field : RD->fields()) {
+ uint64_t FieldOffset = Ctx.getFieldOffset(Field);
+ assert(FieldOffset >= TotalSize &&
+ "Fields seem to overlap; this should never happen!");
+
+ // Check if comparing padding before this field.
+ if (FieldOffset > TotalSize && TotalSize < ComparedBits)
+ return true;
+
+ if (FieldOffset >= ComparedBits)
+ return false;
+
+ uint64_t SizeOfField = getFieldSize(*Field, Field->getType(), Ctx);
+ TotalSize += SizeOfField;
+
+ // Check if comparing padding in nested record.
+ if (Field->getType()->isRecordType() &&
+ hasPadding(Ctx, Field->getType()->getAsRecordDecl()->getDefinition(),
+ ComparedBits - FieldOffset))
+ return true;
+ }
+
+ return false;
+}
+
+static bool hasPadding(const ASTContext &Ctx, const RecordDecl *RD,
+ uint64_t ComparedBits) {
+ assert(RD && RD->isCompleteDefinition() &&
+ "Expected the complete record definition.");
+ if (RD->isUnion())
+ return false;
+
+ uint64_t TotalSize = 0;
+
+ if (hasPaddingInBase(Ctx, RD, ComparedBits, TotalSize) ||
+ hasPaddingBetweenFields(Ctx, RD, ComparedBits, TotalSize))
+ return true;
+
+ // Check if comparing trailing padding.
+ return TotalSize < Ctx.getTypeSize(RD->getTypeForDecl()) &&
+ ComparedBits > TotalSize;
+}
+
+static llvm::Optional<int64_t> tryEvaluateSizeExpr(const Expr *SizeExpr,
+ const ASTContext &Ctx) {
+ Expr::EvalResult Result;
+ if (SizeExpr->EvaluateAsRValue(Result, Ctx))
+ return Ctx.toBits(
+ CharUnits::fromQuantity(Result.Val.getInt().getExtValue()));
+ return None;
+}
+
+void SuspiciousMemoryComparisonCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(callee(namedDecl(
+ anyOf(hasName("::memcmp"), hasName("::std::memcmp")))))
+ .bind("call"),
+ this);
+}
+
+void SuspiciousMemoryComparisonCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const ASTContext &Ctx = *Result.Context;
+ const auto *CE = Result.Nodes.getNodeAs<CallExpr>("call");
+
+ const Expr *SizeExpr = CE->getArg(2);
+ assert(SizeExpr != nullptr && "Third argument of memcmp is mandatory.");
+ llvm::Optional<int64_t> ComparedBits = tryEvaluateSizeExpr(SizeExpr, Ctx);
+
+ for (unsigned int i = 0; i < 2; ++i) {
+ const Expr *ArgExpr = CE->getArg(i);
+ QualType ArgType = ArgExpr->IgnoreImplicit()->getType();
+ const Type *PointeeType = ArgType->getPointeeOrArrayElementType();
+ assert(PointeeType != nullptr && "PointeeType should always be available.");
+
+ if (PointeeType->isRecordType()) {
+ const RecordDecl *RD = PointeeType->getAsRecordDecl()->getDefinition();
+ if (RD != nullptr) {
+ if (const auto *CXXDecl = dyn_cast<CXXRecordDecl>(RD)) {
+ if (!CXXDecl->isStandardLayout()) {
+ diag(CE->getBeginLoc(),
+ "comparing object representation of non-standard-layout "
+ "type %0; consider using a comparison operator instead")
+ << PointeeType->getAsTagDecl()->getQualifiedNameAsString();
+ break;
+ }
+ }
+
+ if (ComparedBits && hasPadding(Ctx, RD, *ComparedBits)) {
+ diag(CE->getBeginLoc(), "comparing padding data in type %0; "
+ "consider comparing the fields manually")
+ << PointeeType->getAsTagDecl()->getQualifiedNameAsString();
+ break;
+ }
+ }
+ }
+ }
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -34,6 +34,7 @@
StringIntegerAssignmentCheck.cpp
StringLiteralWithEmbeddedNulCheck.cpp
SuspiciousEnumUsageCheck.cpp
+ SuspiciousMemoryComparisonCheck.cpp
SuspiciousMemsetUsageCheck.cpp
SuspiciousMissingCommaCheck.cpp
SuspiciousSemicolonCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -42,6 +42,7 @@
#include "StringIntegerAssignmentCheck.h"
#include "StringLiteralWithEmbeddedNulCheck.h"
#include "SuspiciousEnumUsageCheck.h"
+#include "SuspiciousMemoryComparisonCheck.h"
#include "SuspiciousMemsetUsageCheck.h"
#include "SuspiciousMissingCommaCheck.h"
#include "SuspiciousSemicolonCheck.h"
@@ -131,6 +132,8 @@
"bugprone-string-literal-with-embedded-nul");
CheckFactories.registerCheck<SuspiciousEnumUsageCheck>(
"bugprone-suspicious-enum-usage");
+ CheckFactories.registerCheck<SuspiciousMemoryComparisonCheck>(
+ "bugprone-suspicious-memory-comparison");
CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>(
"bugprone-suspicious-memset-usage");
CheckFactories.registerCheck<SuspiciousMissingCommaCheck>(
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
