[clang-tools-extra] 3373e84 - [clang-tidy] Add bugprone-suspicious-memory-comparison check
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 26 00:29:08 PDT 2021
Author: Gabor Bencze
Date: 2021-08-26T09:23:37+02:00
New Revision: 3373e845398bfb8fa0e3c81b7ca84cbfedbad3ae
URL: https://github.com/llvm/llvm-project/commit/3373e845398bfb8fa0e3c81b7ca84cbfedbad3ae
DIFF: https://github.com/llvm/llvm-project/commit/3373e845398bfb8fa0e3c81b7ca84cbfedbad3ae.diff
LOG: [clang-tidy] Add bugprone-suspicious-memory-comparison check
The check warns on suspicious calls to `memcmp`.
It currently checks for comparing types that do not have
unique object representations or are non-standard-layout.
Based on
https://wiki.sei.cmu.edu/confluence/display/c/EXP42-C.+Do+not+compare+padding+data
https://wiki.sei.cmu.edu/confluence/display/c/FLP37-C.+Do+not+use+object+representations+to+compare+floating-point+values
and part of
https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP57-CPP.+Prefer+special+member+functions+and+overloaded+operators+to+C+Standard+Library+functions
Add alias `cert-exp42-c` and `cert-flp37-c`.
Some tests are currently failing at head, the check depends on D89649.
Originally started in D71973
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D89651
Added:
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.h
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/cert-flp37-c.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison-32bits.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.c
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
Modified:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 35b5f2c37df6..9f0e6e3d7335 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -52,6 +52,7 @@
#include "StringLiteralWithEmbeddedNulCheck.h"
#include "SuspiciousEnumUsageCheck.h"
#include "SuspiciousIncludeCheck.h"
+#include "SuspiciousMemoryComparisonCheck.h"
#include "SuspiciousMemsetUsageCheck.h"
#include "SuspiciousMissingCommaCheck.h"
#include "SuspiciousSemicolonCheck.h"
@@ -160,6 +161,8 @@ class BugproneModule : public ClangTidyModule {
"bugprone-suspicious-enum-usage");
CheckFactories.registerCheck<SuspiciousIncludeCheck>(
"bugprone-suspicious-include");
+ CheckFactories.registerCheck<SuspiciousMemoryComparisonCheck>(
+ "bugprone-suspicious-memory-comparison");
CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>(
"bugprone-suspicious-memset-usage");
CheckFactories.registerCheck<SuspiciousMissingCommaCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 78a70c703dc0..6bb9ed947439 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -47,6 +47,7 @@ add_clang_library(clangTidyBugproneModule
StringLiteralWithEmbeddedNulCheck.cpp
SuspiciousEnumUsageCheck.cpp
SuspiciousIncludeCheck.cpp
+ SuspiciousMemoryComparisonCheck.cpp
SuspiciousMemsetUsageCheck.cpp
SuspiciousMissingCommaCheck.cpp
SuspiciousSemicolonCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp
new file mode 100644
index 000000000000..12497122bd88
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp
@@ -0,0 +1,85 @@
+//===--- 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 llvm::Optional<uint64_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(allOf(callee(namedDecl(
+ anyOf(hasName("::memcmp"), hasName("::std::memcmp")))),
+ unless(isInstantiationDependent())))
+ .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<uint64_t> ComparedBits = tryEvaluateSizeExpr(SizeExpr, Ctx);
+
+ for (unsigned int ArgIndex = 0; ArgIndex < 2; ++ArgIndex) {
+ const Expr *ArgExpr = CE->getArg(ArgIndex);
+ QualType ArgType = ArgExpr->IgnoreImplicit()->getType();
+ const Type *PointeeType = ArgType->getPointeeOrArrayElementType();
+ assert(PointeeType != nullptr && "PointeeType should always be available.");
+ QualType PointeeQualifiedType(PointeeType, 0);
+
+ if (PointeeType->isRecordType()) {
+ if (const RecordDecl *RD =
+ PointeeType->getAsRecordDecl()->getDefinition()) {
+ 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")
+ << PointeeQualifiedType;
+ break;
+ }
+ }
+ }
+ }
+
+ if (!PointeeType->isIncompleteType()) {
+ uint64_t PointeeSize = Ctx.getTypeSize(PointeeType);
+ if (ComparedBits.hasValue() && *ComparedBits >= PointeeSize &&
+ !Ctx.hasUniqueObjectRepresentations(PointeeQualifiedType)) {
+ diag(CE->getBeginLoc(),
+ "comparing object representation of type %0 which does not have a "
+ "unique object representation; consider comparing %select{the "
+ "values|the members of the object}1 manually")
+ << PointeeQualifiedType << (PointeeType->isRecordType() ? 1 : 0);
+ break;
+ }
+ }
+ }
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.h b/clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.h
new file mode 100644
index 000000000000..24093c54f600
--- /dev/null
+++ b/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()`` based on properties of the
+/// arguments.
+///
+/// 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
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index 5475e2ee7bf4..c3cfe12cd851 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -14,6 +14,7 @@
#include "../bugprone/SignalHandlerCheck.h"
#include "../bugprone/SignedCharMisuseCheck.h"
#include "../bugprone/SpuriouslyWakeUpFunctionsCheck.h"
+#include "../bugprone/SuspiciousMemoryComparisonCheck.h"
#include "../bugprone/UnhandledSelfAssignmentCheck.h"
#include "../concurrency/ThreadCanceltypeAsynchronousCheck.h"
#include "../google/UnnamedNamespaceInHeaderCheck.h"
@@ -98,8 +99,13 @@ class CERTModule : public ClangTidyModule {
"cert-dcl37-c");
// ENV
CheckFactories.registerCheck<CommandProcessorCheck>("cert-env33-c");
+ // EXP
+ CheckFactories.registerCheck<bugprone::SuspiciousMemoryComparisonCheck>(
+ "cert-exp42-c");
// FLP
CheckFactories.registerCheck<FloatLoopCounter>("cert-flp30-c");
+ CheckFactories.registerCheck<bugprone::SuspiciousMemoryComparisonCheck>(
+ "cert-flp37-c");
// FIO
CheckFactories.registerCheck<misc::NonCopyableObjectsCheck>("cert-fio38-c");
// ERR
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 1cbb79fbab2c..c997b2b37a2f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -72,6 +72,11 @@ The improvements are...
New checks
^^^^^^^^^^
+- New :doc:`bugprone-suspicious-memory-comparison
+ <clang-tidy/checks/bugprone-suspicious-memory-comparison>` check.
+
+ Finds potentially incorrect calls to ``memcmp()`` based on properties of the arguments.
+
- New :doc:`readability-identifier-length
<clang-tidy/checks/readability-identifier-length>` check.
@@ -80,6 +85,16 @@ New checks
New check aliases
^^^^^^^^^^^^^^^^^
+- 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.
+
+- New alias :doc:`cert-flp37-c
+ <clang-tidy/checks/cert-flp37-c>` to
+ :doc:`bugprone-suspicious-memory-comparison
+ <clang-tidy/checks/bugprone-suspicious-memory-comparison>` was added.
+
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst
new file mode 100644
index 000000000000..a2d5c6d9a23c
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst
@@ -0,0 +1,31 @@
+.. title:: clang-tidy - bugprone-suspicious-memory-comparison
+
+bugprone-suspicious-memory-comparison
+=====================================
+
+Finds potentially incorrect calls to ``memcmp()`` based on properties of the
+arguments. The following cases are covered:
+
+**Case 1: Non-standard-layout type**
+
+Comparing the object representaions of non-standard-layout objects may not
+properly compare the value representations.
+
+**Case 2: Types with no unique object representation**
+
+Objects with the same value may not have the same object representaion.
+This may be caused by padding or floating-point types.
+
+See also:
+`EXP42-C. Do not compare padding data
+<https://wiki.sei.cmu.edu/confluence/display/c/EXP42-C.+Do+not+compare+padding+data>`_
+and
+`FLP37-C. Do not use object representations to compare floating-point values
+<https://wiki.sei.cmu.edu/confluence/display/c/FLP37-C.+Do+not+use+object+representations+to+compare+floating-point+values>`_
+
+This check is also related to and partially overlaps the CERT C++ Coding Standard rules
+`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>`_
+and
+`EXP62-CPP. Do not access the bits of an object representation that are not part of the object's value representation
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/EXP62-CPP.+Do+not+access+the+bits+of+an+object+representation+that+are+not+part+of+the+object%27s+value+representation>`_
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert-exp42-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert-exp42-c.rst
new file mode 100644
index 000000000000..1ed5fbab97e6
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/cert-exp42-c.rst
@@ -0,0 +1,8 @@
+.. 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.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert-flp37-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert-flp37-c.rst
new file mode 100644
index 000000000000..c73fe7f201d1
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/cert-flp37-c.rst
@@ -0,0 +1,8 @@
+.. meta::
+ :http-equiv=refresh: 5;URL=bugprone-suspicious-memory-comparison.html
+
+cert-flp37-c
+============
+
+The cert-flp37-c check is an alias, please see
+`bugprone-suspicious-memory-comparison <bugprone-suspicious-memory-comparison.html>`_ for more information.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index ee0c6e3c55a0..7ba7181c5db0 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -93,6 +93,7 @@ Clang-Tidy Checks
`bugprone-string-literal-with-embedded-nul <bugprone-string-literal-with-embedded-nul.html>`_,
`bugprone-suspicious-enum-usage <bugprone-suspicious-enum-usage.html>`_,
`bugprone-suspicious-include <bugprone-suspicious-include.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"
@@ -117,7 +118,9 @@ Clang-Tidy Checks
`cert-err52-cpp <cert-err52-cpp.html>`_,
`cert-err58-cpp <cert-err58-cpp.html>`_,
`cert-err60-cpp <cert-err60-cpp.html>`_,
+ `cert-exp42-c <cert-exp42-c.html>`_,
`cert-flp30-c <cert-flp30-c.html>`_,
+ `cert-flp37-c <cert-flp37-c.html>`_,
`cert-mem57-cpp <cert-mem57-cpp.html>`_,
`cert-msc50-cpp <cert-msc50-cpp.html>`_,
`cert-msc51-cpp <cert-msc51-cpp.html>`_,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison-32bits.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison-32bits.cpp
new file mode 100644
index 000000000000..1b70f79518dc
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison-32bits.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t \
+// RUN: -- -- -target i386-unknown-unknown
+
+static_assert(sizeof(int *) == sizeof(int));
+
+namespace std {
+typedef __SIZE_TYPE__ size_t;
+int memcmp(const void *lhs, const void *rhs, size_t count);
+} // namespace std
+
+namespace no_padding_on_32bit {
+struct S {
+ int x;
+ int *y;
+};
+
+void test() {
+ S a, b;
+ std::memcmp(&a, &b, sizeof(S));
+}
+} // namespace no_padding_on_32bit
+
+namespace inner_padding {
+struct S {
+ char x;
+ int y;
+};
+void test() {
+ S a, b;
+ std::memcmp(&a, &b, sizeof(S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'inner_padding::S' which does not have a unique object representation; consider comparing the members of the object manually
+}
+} // namespace inner_padding
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.c
new file mode 100644
index 000000000000..1f8fc7cbb07f
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.c
@@ -0,0 +1,294 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t \
+// RUN: -- -- -target x86_64-unknown-unknown -std=c99
+
+typedef __SIZE_TYPE__ size_t;
+int memcmp(const void *lhs, const void *rhs, size_t count);
+
+// Examples from cert rule exp42-c
+
+struct S {
+ char c;
+ int i;
+ char buffer[13];
+};
+
+void exp42_c_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 object representation of type 'struct S' which does not have a unique object representation; consider comparing the members of the object manually
+ }
+}
+
+void exp42_c_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))) {
+ }
+}
+
+#pragma pack(push, 1)
+struct Packed_S {
+ char c;
+ int i;
+ char buffer[13];
+};
+#pragma pack(pop)
+
+void compliant_packed(const struct Packed_S *left,
+ const struct Packed_S *right) {
+ if ((left && right) && (0 == memcmp(left, right, sizeof(struct Packed_S)))) {
+ // no-warning
+ }
+}
+
+// Examples from cert rule flp37-c
+
+struct S2 {
+ int i;
+ float f;
+};
+
+int flp37_c_noncompliant(const struct S2 *s1, const struct S2 *s2) {
+ if (!s1 && !s2)
+ return 1;
+ else if (!s1 || !s2)
+ return 0;
+ return 0 == memcmp(s1, s2, sizeof(struct S2));
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: comparing object representation of type 'struct S2' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+int flp37_c_compliant(const struct S2 *s1, const struct S2 *s2) {
+ if (!s1 && !s2)
+ return 1;
+ else if (!s1 || !s2)
+ return 0;
+ return s1->i == s2->i && s1->f == s2->f;
+ // no-warning
+}
+
+void Test_Float() {
+ float a, b;
+ memcmp(&a, &b, sizeof(float));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'float' which does not have a unique object representation; consider comparing the values manually
+}
+
+void TestArray_Float() {
+ float a[3], b[3];
+ memcmp(a, b, sizeof(a));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'float' which does not have a unique object representation; consider comparing the values manually
+}
+
+struct PredeclaredType;
+
+void Test_PredeclaredType(const struct PredeclaredType *lhs,
+ const struct PredeclaredType *rhs) {
+ memcmp(lhs, rhs, 1); // no-warning: predeclared type
+}
+
+struct NoPadding {
+ int x;
+ int y;
+};
+
+void Test_NoPadding() {
+ struct NoPadding a, b;
+ memcmp(&a, &b, sizeof(struct NoPadding));
+}
+
+void TestArray_NoPadding() {
+ struct NoPadding a[3], b[3];
+ memcmp(a, b, 3 * sizeof(struct NoPadding));
+}
+
+struct TrailingPadding {
+ int i;
+ char c;
+};
+
+void Test_TrailingPadding() {
+ struct TrailingPadding a, b;
+ memcmp(&a, &b, sizeof(struct TrailingPadding));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'struct TrailingPadding' which does not have a unique object representation; consider comparing the members of the object manually
+ memcmp(&a, &b, sizeof(int)); // no-warning: not comparing entire object
+ memcmp(&a, &b, 2 * sizeof(int));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'struct TrailingPadding' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+struct TrailingPadding2 {
+ int i[2];
+ char c;
+};
+
+void Test_TrailingPadding2() {
+ struct TrailingPadding2 a, b;
+ memcmp(&a, &b, 2 * sizeof(int)); // no-warning: not comparing entire object
+ memcmp(&a, &b, sizeof(struct TrailingPadding2));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'struct TrailingPadding2' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+void Test_UnknownCount(size_t count) {
+ struct TrailingPadding a, b;
+ memcmp(&a, &b, count); // no-warning: unknown count value
+}
+
+void Test_ExplicitVoidCast() {
+ struct TrailingPadding a, b;
+ memcmp((void *)&a, (void *)&b,
+ sizeof(struct TrailingPadding)); // no-warning: explicit cast
+}
+
+void TestArray_TrailingPadding() {
+ struct TrailingPadding a[3], b[3];
+ memcmp(a, b, 3 * sizeof(struct TrailingPadding));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'struct TrailingPadding' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+struct InnerPadding {
+ char c;
+ int i;
+};
+
+void Test_InnerPadding() {
+ struct InnerPadding a, b;
+ memcmp(&a, &b, sizeof(struct InnerPadding));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'struct InnerPadding' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+struct Bitfield_TrailingPaddingBytes {
+ int x : 10;
+ int y : 6;
+};
+
+void Test_Bitfield_TrailingPaddingBytes() {
+ struct Bitfield_TrailingPaddingBytes a, b;
+ memcmp(&a, &b, sizeof(struct S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'struct Bitfield_TrailingPaddingBytes' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+struct Bitfield_TrailingPaddingBits {
+ int x : 10;
+ int y : 20;
+};
+
+void Test_Bitfield_TrailingPaddingBits() {
+ struct Bitfield_TrailingPaddingBits a, b;
+ memcmp(&a, &b, sizeof(struct Bitfield_TrailingPaddingBits));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'struct Bitfield_TrailingPaddingBits' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+struct Bitfield_InnerPaddingBits {
+ char x : 2;
+ int : 0;
+ char y : 8;
+};
+
+void Test_Bitfield_InnerPaddingBits() {
+ struct Bitfield_InnerPaddingBits a, b;
+ memcmp(&a, &b, sizeof(struct Bitfield_InnerPaddingBits));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'struct Bitfield_InnerPaddingBits' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+struct Bitfield_NoPadding {
+ int i : 10;
+ int j : 10;
+ int k : 10;
+ int l : 2;
+};
+_Static_assert(sizeof(struct Bitfield_NoPadding) == sizeof(int),
+ "Bit-fields should line up perfectly");
+
+void Test_Bitfield_NoPadding() {
+ struct Bitfield_NoPadding a, b;
+ memcmp(&a, &b, sizeof(struct Bitfield_NoPadding)); // no-warning
+}
+
+struct Bitfield_TrailingUnnamed {
+ int i[2];
+ int : 0;
+};
+
+void Bitfield_TrailingUnnamed() {
+ struct Bitfield_TrailingUnnamed a, b;
+ memcmp(&a, &b, 2 * sizeof(int)); // no-warning
+ memcmp(&a, &b, sizeof(struct Bitfield_TrailingUnnamed)); // no-warning
+}
+
+struct PaddingAfterUnion {
+ union {
+ unsigned short a;
+ short b;
+ } x;
+
+ int y;
+};
+
+void Test_PaddingAfterUnion() {
+ struct PaddingAfterUnion a, b;
+ memcmp(&a, &b, sizeof(struct PaddingAfterUnion));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'struct PaddingAfterUnion' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+struct Union_NoPadding {
+ union {
+ int a;
+ unsigned int b;
+ } x;
+
+ int y;
+};
+
+void Test_Union_NoPadding() {
+ struct Union_NoPadding a, b;
+ memcmp(&a, &b, 2 * sizeof(int));
+ memcmp(&a, &b, sizeof(struct Union_NoPadding));
+}
+
+union UnionWithPaddingInNestedStruct {
+ int i;
+
+ struct {
+ int i;
+ char c;
+ } x;
+};
+
+void Test_UnionWithPaddingInNestedStruct() {
+ union UnionWithPaddingInNestedStruct a, b;
+ memcmp(&a, &b, sizeof(union UnionWithPaddingInNestedStruct));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'union UnionWithPaddingInNestedStruct' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+struct PaddingInNested {
+ struct TrailingPadding x;
+ char y;
+};
+
+void Test_PaddingInNested() {
+ struct PaddingInNested a, b;
+ memcmp(&a, &b, sizeof(struct PaddingInNested));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'struct PaddingInNested' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+struct PaddingAfterNested {
+ struct {
+ char a;
+ char b;
+ } x;
+ int y;
+};
+
+void Test_PaddingAfterNested() {
+ struct PaddingAfterNested a, b;
+ memcmp(&a, &b, sizeof(struct PaddingAfterNested));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'struct PaddingAfterNested' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+struct AtomicMember {
+ _Atomic(int) x;
+};
+
+void Test_AtomicMember() {
+ // FIXME: this is a false positive as the list of objects with unique object
+ // representations is incomplete.
+ struct AtomicMember a, b;
+ memcmp(&a, &b, sizeof(struct AtomicMember));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'struct AtomicMember' which does not have a unique object representation; consider comparing the members of the object manually
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
new file mode 100644
index 000000000000..f4406b222eeb
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp
@@ -0,0 +1,233 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t \
+// RUN: -- -- -target x86_64-unknown-unknown
+
+namespace std {
+typedef __SIZE_TYPE__ size_t;
+int memcmp(const void *lhs, const void *rhs, size_t count);
+} // namespace std
+
+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 inner_padding_64bit_only {
+struct S {
+ int x;
+ int *y;
+};
+
+void test() {
+ S a, b;
+ std::memcmp(&a, &b, sizeof(S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'inner_padding_64bit_only::S' which does not have a unique object representation; consider comparing the members of the object manually
+}
+} // namespace inner_padding_64bit_only
+
+namespace padding_in_base {
+class Base {
+ char c;
+ int i;
+};
+
+class Derived : public Base {};
+
+class Derived2 : public Derived {};
+
+void testDerived() {
+ Derived a, b;
+ std::memcmp(&a, &b, sizeof(Base));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'padding_in_base::Derived' which does not have a unique object representation; consider comparing the members of the object manually
+ std::memcmp(&a, &b, sizeof(Derived));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'padding_in_base::Derived' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+void testDerived2() {
+ Derived2 a, b;
+ std::memcmp(&a, &b, sizeof(Base));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'padding_in_base::Derived2' which does not have a unique object representation; consider comparing the members of the object manually
+ std::memcmp(&a, &b, sizeof(Derived2));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'padding_in_base::Derived2' which does not have a unique object representation; consider comparing the members of the object 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;
+ std::memcmp(&a, &b, sizeof(Base));
+ std::memcmp(&a, &b, sizeof(Derived));
+}
+
+void testDerived2() {
+ Derived2 a, b;
+ std::memcmp(&a, &b, sizeof(char));
+ std::memcmp(&a, &b, sizeof(Base));
+ std::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;
+ std::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;
+ std::memcmp(&a, &b, sizeof(S));
+}
+} // namespace static_ignored
+
+namespace operator_void_ptr {
+struct S {
+ operator void *() const;
+};
+
+void test() {
+ S s;
+ std::memcmp(s, s, sizeof(s));
+}
+} // namespace operator_void_ptr
+
+namespace empty_struct {
+struct S {};
+
+void test() {
+ S a, b;
+ std::memcmp(&a, &b, sizeof(S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'empty_struct::S' which does not have a unique object representation; consider comparing the members of the object manually
+}
+} // namespace empty_struct
+
+namespace empty_field {
+struct Empty {};
+struct S {
+ Empty e;
+};
+
+void test() {
+ S a, b;
+ std::memcmp(&a, &b, sizeof(S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'empty_field::S' which does not have a unique object representation; consider comparing the members of the object manually
+}
+} // namespace empty_field
+
+namespace no_unique_address_attribute {
+struct Empty {};
+
+namespace no_padding {
+struct S {
+ char c;
+ [[no_unique_address]] Empty e;
+};
+
+void test() {
+ S a, b;
+ std::memcmp(&a, &b, sizeof(S));
+}
+
+} // namespace no_padding
+
+namespace multiple_empties_same_type {
+struct S {
+ char c;
+ [[no_unique_address]] Empty e1, e2;
+};
+
+void test() {
+ S a, b;
+ std::memcmp(&a, &b, sizeof(S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'no_unique_address_attribute::multiple_empties_same_type::S' which does not have a unique object representation; consider comparing the members of the object manually
+}
+
+} // namespace multiple_empties_same_type
+
+namespace multiple_empties_
diff erent_types {
+struct Empty2 {};
+
+struct S {
+ char c;
+ [[no_unique_address]] Empty e1;
+ [[no_unique_address]] Empty2 e2;
+};
+
+void test() {
+ S a, b;
+ std::memcmp(&a, &b, sizeof(S));
+}
+} // namespace multiple_empties_
diff erent_types
+} // namespace no_unique_address_attribute
+
+namespace alignment {
+struct S {
+ char x;
+ alignas(sizeof(int)) char y[sizeof(int)];
+};
+
+void test() {
+ S a, b;
+ std::memcmp(&a, &b, sizeof(S));
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing object representation of type 'alignment::S' which does not have a unique object representation; consider comparing the members of the object manually
+}
+} // namespace alignment
+
+namespace no_warning_in_template {
+template <typename T>
+int compare(const T *l, const T *r) {
+ return std::memcmp(l, r, sizeof(T));
+}
+
+void test() {
+ int a, b;
+ compare(&a, &b);
+}
+} // namespace no_warning_in_template
+
+namespace warning_in_template {
+template <typename T>
+int compare(const T *l, const T *r) {
+ return std::memcmp(l, r, sizeof(T));
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: comparing object representation of type 'float' which does not have a unique object representation; consider comparing the values manually
+}
+
+void test() {
+ float a, b;
+ compare(&a, &b);
+}
+} // namespace warning_in_template
More information about the cfe-commits
mailing list