[clang-tidy] Add bugprone-suspicious-memory-comparison check

[gbencze]

Summary

The check warns on suspicious calls to memcmp.
It currently checks for comparing padding and non-standard-layout types.
Based on
https://wiki.sei.cmu.edu/confluence/display/c/EXP42-C.+Do+not+compare+padding+data
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.

[Eugene.Zelenko]

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

39

You could use const auto * because type is specified in same statement.

45

Please don't use auto when type is not specified in same statement or iterator.

129

Please don't use auto when type is not specified in same statement or iterator.

140

You could use const auto * because type is specified in same statement.

clang-tools-extra/docs/ReleaseNotes.rst

103

Please move into aliases section (in alphabetical order)

[gbencze]

Update style based on comments.

[gbencze]

clang-tools-extra/docs/ReleaseNotes.rst

103

I'm not quite sure where these are supposed to be. I can only find one other alias (cert-pos44-c) in the release notes, should I put it immediately before that?

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

103

In 9.0 Release Notes aliases were placed after new checks.

[gbencze]

ReleaseNotes: move alias after new checks

[JonasToth]

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

20

I think you can remove the constant and hardcode it in the matcher. that is common practice and because its used twice not a big issue (and shorter).

22

You are in clang namespace, so you can ellide the clang:: here and in the following functions/decls.

28

Maybe return FD.isBitField() ? FD.getBitWidthValue(Ctx) : Ctx.getTypeSize(FieldType);? I find it cleaner, but its preference i guess.

47

value-types are not defined as const per coding guideline.

63

plz no const

64

a short descriptive error message for this assertion would be great.

67

Please make this comment a full sentence with punctuation. I think it should be above the if.

73

const

77

Please make this comment a full sentence with punctuation.

Both ifs can be merged to one with an &&?

89

Please provide an error message

99

Full sentence for comment.

110

No else after return. You can simply return None;

126

Always true, otherwise matcher don't work / didn't fire. I think this assert is not required.

129

please provide a short error message why this should always be true (i guess because memcmp is standardized and stuff, but when reading this code alone it might not be super obvious).

130

value-type that is const, please remove the const.

135

const, next line the * const is uncommon in llvm-code. I think you should remove the pointer-consting. The other pointers are not const either.

137

error message.

153

i think this diagnostic could be a bit more expressive, similar to the one above.
Unexperienced programmer might not understand the issue or even know what padding bytes are.

clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp

2

We should explicitly test, that the check runs under C-code, either with two run-lines or with separate test files (my preference).

4

I think the test could be sensitive to cpu-architectures.
If you could bring up a case that is e.g. problematic on X86, but not on anything else (powerpc, or arm or anything, 64bit vs 32bit) and demonstrate with tests that such cases are caught as well this would be great. I see no reason it wouldn't.

That would maybe justify another alias into portability, as this is an issue in that case.

[gbencze]

Coding guide and better diagnostic message for padding comparison

[gbencze]

clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp

2

Should I duplicate the tests that are legal C or try to split it up so that only c++ specific code is tested in this one?

4

I may have misunderstood you but the check currently only warns if you're comparing padding on the current compilation target.
Or do you mean adding another test file that is compiled e.g. with -m32 and testing if calling memcmp on the following doesn't warn there but does on 64bit?

struct S {
    int x;
    int* y;
};

[JonasToth]

clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp

2

Rather splitting. Duplication might be painfull in the future.

4

Yes. Because it is only tested for the current archtiecture the buildbots will probably fail on it, because they test many architectures.

I think it is necessary to specify the arch in the RUN-line (%t -- -- -target x86_64-unknown-unknown) at the end of it.

And yes: Another test-file would be great to demonstrate that it works as expected.

[gbencze]

Tests: Split C/C++ tests and add 32/64bit specific test.

[JonasToth]

LG from my side, besides the small nits.
Please let @aaron.ballman have a look as well.

thanks for the patch :)

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

46

Nit: Please make this comment a full sentence with proper punctuation.

clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst

11

Maybe this link is not proper, because of the newline. could you please check if the documentation builds? (you need sphinx for that and enable it in cmake.)

clang-tools-extra/docs/clang-tidy/checks/list.rst

75

i believe the classification of the checks was changed today. did you rebase to master? hopefully it still applies clean.

[gbencze]

Punctuation in comment

[gbencze]

Thanks for all the feedback @JonasToth :)

clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst

11

It seems to work fine

clang-tools-extra/docs/clang-tidy/checks/list.rst

75

I did rebase today and removed the classification for these two by hand

[gbencze]

ping @aaron.ballman - any thoughts on the patch?

[aaron.ballman]

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

31

Drop top-level const qualifiers on locals (it's not a style we typically use), elsewhere as well.

83–84

I am surprised that we have to do this much manual work, I would have expected this to be something that RecordLayout would handle for us. I am a bit worried about this manual work being split in a few places because we're likely to get it wrong in at least one of them. For instance, this doesn't seem to be taking into account things like [[no_unique_address]] or alignment when considering the layout of fields.

I sort of wonder if we should try using the RecordLayout class to do this work instead, even if that requires larger changes.

131

Can you add a test case like this:

struct S {
  operator void *() const;
};

S s;
memcmp(s, s, sizeof(s));

to ensure that this assertion doesn't trigger?

134–135

Sink the declaration into the if conditional to limit its scope.

141

You can pass in PointeeType->getAsTagDecl() -- the diagnostic printer will automatically get the correct name from any NamedDecl.

149

Same here as above.

[gbencze]

Address (most of the) comments by @aaron.ballman

• remove top-level const on locals
• move declaration into if
• pass TagDecl to diag
• added test for operator void *
• fixed [[no_unique_address]]
  • remove assertion that checks for overlapping fields
  • in hasPaddingBetweenFields: only do recursive call and add field size to TotalSize if not isZeroSize
  • + added tests

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

83–84

Thanks, I didn't realize the [[no_unique_address]] attribute existed but I fixed it (in this check for now) and also added some tests to cover it as well as alignas.

If you think this information should be provided by RecordLayout I'm willing to work on that as well (though I guess those changes should be in a different patch). Do you have any ideas as to how it should expose the necessary information?

[gbencze]

Another option that came to my mind is using a BitVector to (recursively) flag bits that are occupied by the fields. This solution would be slightly slower and uses more memory but is probably a lot easier to understand, maintain and more robust than the currently proposed implementation. This would also catch a few additional cases as it could also look inside unions.

I stared to experiment with an implementation of this here.

Which direction do you think would be a better solution?

[gbencze]

ping

[aaron.ballman]

In D71973#1817941, @gbencze wrote:

Another option that came to my mind is using a BitVector to (recursively) flag bits that are occupied by the fields. This solution would be slightly slower and uses more memory but is probably a lot easier to understand, maintain and more robust than the currently proposed implementation. This would also catch a few additional cases as it could also look inside unions.

I stared to experiment with an implementation of this here.

Which direction do you think would be a better solution?

This is a neat approach, but I think I still prefer asking the RecordLayout for this information if possible.

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

83–84

I do think this should be provided by RecordLayout (and yes, please as a separate patch -- feel free to add myself and Richard Smith as reviewers for it). Intuitively, I would expect to be able to query for whether a record has any padding within it whatsoever. For your needs, I would imagine an interface like bool hasPaddingWithin(CharUnits InitialOffset, CharUnits FinalOffset) would also help -- though I suspect we may not want to use CharUnits because of bit-fields that may have padding due to 0-sized bit-fields.

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

 #include "ParentVirtualCallCheck.h"
 #include "PosixReturnCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
 #include "StringConstructorCheck.h"
 #include "StringIntegerAssignmentCheck.h"
 #include "StringLiteralWithEmbeddedNulCheck.h"
 #include "SuspiciousEnumUsageCheck.h"
+#include "SuspiciousMemoryComparisonCheck.h"
 #include "SuspiciousMemsetUsageCheck.h"
 #include "SuspiciousMissingCommaCheck.h"
 #include "SuspiciousSemicolonCheck.h"
 #include "SuspiciousStringCompareCheck.h"
 #include "SwappedArgumentsCheck.h"
 #include "TerminatingContinueCheck.h"
 #include "ThrowKeywordMissingCheck.h"
 #include "TooSmallLoopVariableCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<StringConstructorCheck>(
         "bugprone-string-constructor");
     CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
         "bugprone-string-integer-assignment");
     CheckFactories.registerCheck<StringLiteralWithEmbeddedNulCheck>(
         "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>(
         "bugprone-suspicious-missing-comma");
     CheckFactories.registerCheck<SuspiciousSemicolonCheck>(
         "bugprone-suspicious-semicolon");
     CheckFactories.registerCheck<SuspiciousStringCompareCheck>(
         "bugprone-suspicious-string-compare");

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

add_clang_library(clangTidyBugproneModule
   ParentVirtualCallCheck.cpp
   PosixReturnCheck.cpp
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   StringConstructorCheck.cpp
   StringIntegerAssignmentCheck.cpp
   StringLiteralWithEmbeddedNulCheck.cpp
   SuspiciousEnumUsageCheck.cpp
+  SuspiciousMemoryComparisonCheck.cpp
   SuspiciousMemsetUsageCheck.cpp
   SuspiciousMissingCommaCheck.cpp
   SuspiciousSemicolonCheck.cpp
   SuspiciousStringCompareCheck.cpp
   SwappedArgumentsCheck.cpp
   TerminatingContinueCheck.cpp
   ThrowKeywordMissingCheck.cpp
   TooSmallLoopVariableCheck.cpp

clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.h

+//===--- 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

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

+//===--- 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) {
+  auto IsNotEmptyBase = [](const CXXBaseSpecifier &Base) {
+    return !Base.getType()->getAsCXXRecordDecl()->isEmpty();
+  };
+
+  if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+    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);
+
+    // Check if comparing padding before this field.
+    if (FieldOffset > TotalSize && TotalSize < ComparedBits)
+      return true;
+
+    if (FieldOffset >= ComparedBits)
+      return false;
+
+    if (!Field->isZeroSize(Ctx)) {
+      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()) {
+      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")
+                << PointeeType->getAsTagDecl();
+            break;
+          }
+        }
+
+        if (ComparedBits && hasPadding(Ctx, RD, *ComparedBits)) {
+          diag(CE->getBeginLoc(), "comparing padding data in type %0; "
+                                  "consider comparing the fields manually")
+              << PointeeType->getAsTagDecl();
+          break;
+        }
+      }
+    }
+  }
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp

 //===--- CERTTidyModule.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 "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #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"
 #include "../misc/StaticAssertCheck.h"
 #include "../misc/ThrowByValueCatchByReferenceCheck.h"
 #include "../performance/MoveConstructorInitCheck.h"
 #include "../readability/UppercaseLiteralSuffixCheck.h"
 #include "CommandProcessorCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
 
     // C checkers
     // DCL
     CheckFactories.registerCheck<misc::StaticAssertCheck>("cert-dcl03-c");
     CheckFactories.registerCheck<readability::UppercaseLiteralSuffixCheck>(
         "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
     CheckFactories.registerCheck<misc::NonCopyableObjectsCheck>("cert-fio38-c");
     // ERR
     CheckFactories.registerCheck<StrToNumCheck>("cert-err34-c");
     // MSC
     CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc30-c");

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`bugprone-not-null-terminated-result
   <clang-tidy/checks/bugprone-not-null-terminated-result>` check
 
   Finds function calls where it is possible to cause a not null-terminated
   result. Usually the proper length of a string is ``strlen(str) + 1`` or equal
   length of this expression, because the null terminator needs an extra space.
   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.
 
   Checks if an object of type with extended alignment is allocated by using
   the default ``operator new``.
 
 - New alias :doc:`cert-pos44-c
   <clang-tidy/checks/cert-pos44-c>` to
   :doc:`bugprone-bad-signal-to-kill-thread
- New :doc:`performance-no-automatic-move
   Finds local variables that cannot be automatically moved due to constness.
 
 - New :doc:`performance-trivially-destructible
   <clang-tidy/checks/performance-trivially-destructible>` check.
 
   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.
 
   Now also checks if any calls to ``pthread_*`` functions expect negative return
   values.
 
 - The 'objc-avoid-spinlock' check was renamed to :doc:`darwin-avoid-spinlock
   <clang-tidy/checks/darwin-avoid-spinlock>`

clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst

+.. 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>`_.

clang-tools-extra/docs/clang-tidy/checks/cert-exp42-c.rst

+.. 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.

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `bugprone-parent-virtual-call <bugprone-parent-virtual-call.html>`_, "Yes"
    `bugprone-posix-return <bugprone-posix-return.html>`_, "Yes"
    `bugprone-sizeof-container <bugprone-sizeof-container.html>`_,
    `bugprone-sizeof-expression <bugprone-sizeof-expression.html>`_,
    `bugprone-string-constructor <bugprone-string-constructor.html>`_, "Yes"
    `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"
    `bugprone-suspicious-string-compare <bugprone-suspicious-string-compare.html>`_, "Yes"
    `bugprone-swapped-arguments <bugprone-swapped-arguments.html>`_, "Yes"
    `bugprone-terminating-continue <bugprone-terminating-continue.html>`_, "Yes"
    `bugprone-throw-keyword-missing <bugprone-throw-keyword-missing.html>`_,
    `bugprone-too-small-loop-variable <bugprone-too-small-loop-variable.html>`_,
.. csv-table:: Aliases..
    :widths: 50, 50, 10
 
    `cert-dcl03-c <cert-dcl03-c.html>`_, `misc-static-assert <misc-static-assert.html>`_, "Yes"
    `cert-dcl16-c <cert-dcl16-c.html>`_, `readability-uppercase-literal-suffix <readability-uppercase-literal-suffix.html>`_, "Yes"
    `cert-dcl54-cpp <cert-dcl54-cpp.html>`_, `misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
    `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>`_,
    `cert-oop11-cpp <cert-oop11-cpp.html>`_, `performance-move-constructor-init <performance-move-constructor-init.html>`_,
    `cert-oop54-cpp <cert-oop54-cpp.html>`_, `bugprone-unhandled-self-assignment <bugprone-unhandled-self-assignment.html>`_,
    `cert-pos44-c <cert-pos44-c.html>`_, `bugprone-bad-signal-to-kill-thread <bugprone-bad-signal-to-kill-thread.html>`_,
    `clang-analyzer-core.CallAndMessage <clang-analyzer-core.CallAndMessage.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_,
    `clang-analyzer-core.DivideZero <clang-analyzer-core.DivideZero.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_,

clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison-32bits.cpp

+// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t \
+// RUN: -- -- -target x86_64-unknown-unknown -m32
+
+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 padding data in type
+  // inner_padding::S; consider comparing the fields manually
+}
+} // namespace inner_padding

clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.c

+// 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 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
+    // 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))) {
+  }
+}
+
+#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
+  }
+}
+
+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 padding data in type
+  // TrailingPadding; 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
+  // TrailingPadding; consider comparing the fields manually
+}
+
+void Test_UnknownCount(size_t count) {
+  struct TrailingPadding a, b;
+  memcmp(&a, &b, count); // no-warning: unknown count value
+}
+
+void TestArray_TrailingPadding() {
+  struct TrailingPadding a[3], b[3];
+  memcmp(a, b, 3 * sizeof(struct TrailingPadding));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // TrailingPadding; consider comparing the fields 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 padding data in type
+  // InnerPadding; consider comparing the fields manually
+  memcmp(&a, &b, sizeof(char) + sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // InnerPadding; 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
+  // InnerPadding; consider comparing the fields 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 padding data in type
+  // Bitfield_TrailingPaddingBytes; consider comparing the fields manually
+  memcmp(&a, &b, 2); // no-warning: no padding in first 2 bytes
+}
+
+struct Bitfield_TrailingPaddingBits {
+  int x : 10;
+  int y : 7;
+};
+
+void Test_Bitfield_TrailingPaddingBits() {
+  struct Bitfield_TrailingPaddingBits a, b;
+  memcmp(&a, &b, sizeof(struct Bitfield_TrailingPaddingBits));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // Bitfield_TrailingPaddingBits; 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_TrailingPaddingBits; consider comparing the fields manually
+}
+
+struct Bitfield_InnerPaddingBits {
+  int x : 2;
+  int : 0;
+  int y : 6;
+};
+
+void Test_Bitfield_InnerPaddingBits() {
+  struct Bitfield_InnerPaddingBits a, b;
+  memcmp(&a, &b, 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // Bitfield_InnerPaddingBits; consider comparing the fields manually
+}
+
+struct PaddingAfterUnion {
+  union {
+    char c;
+    short s;
+  } x;
+
+  int y;
+};
+
+void Test_PaddingAfterUnion() {
+  struct PaddingAfterUnion a, b;
+  memcmp(&a, &b, sizeof(short));
+  memcmp(&a, &b, sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // PaddingAfterUnion; consider comparing the fields manually
+  memcmp(&a, &b, sizeof(struct PaddingAfterUnion));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // PaddingAfterUnion; consider comparing the fields manually
+}
+
+struct Union_NoPadding {
+  union {
+    int a;
+    char 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));
+}
+
+struct PaddingInNested {
+  struct TrailingPadding x;
+  char y;
+};
+
+void Test_PaddingInNested() {
+  struct PaddingInNested 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
+  // PaddingInNested; consider comparing the fields manually
+  memcmp(&a, &b, sizeof(struct TrailingPadding));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // PaddingInNested; consider comparing the fields manually
+  memcmp(&a, &b, sizeof(struct PaddingInNested));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // PaddingInNested; consider comparing the fields manually
+}
+
+struct PaddingAfterNested {
+  struct {
+    char a;
+    char b;
+  } x;
+  int y;
+};
+
+void Test_PaddingAfterNested() {
+  struct PaddingAfterNested a, b;
+  memcmp(&a, &b, 2 * sizeof(char));
+  memcmp(&a, &b, sizeof(a.x));
+  memcmp(&a, &b, sizeof(struct PaddingAfterNested));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // PaddingAfterNested; consider comparing the fields manually
+}

clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp

+// 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 padding data in type
+  // inner_padding_64bit_only::S; consider comparing the fields 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(char));
+  std::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
+  std::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;
+  std::memcmp(&a, &b, sizeof(char));
+  std::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
+  std::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;
+  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 padding data in type
+  // empty_struct::S; consider comparing the fields 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 padding data in type
+  // empty_field::S; consider comparing the fields 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 padding data in type
+  // no_unique_address_attribute::multiple_empties_same_type::S; consider
+  // comparing the fields manually
+}
+
+} // namespace multiple_empties_same_type
+
+namespace multiple_empties_different_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_different_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(char));
+  std::memcmp(&a, &b, 2 * sizeof(char));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // alignment::S; consider comparing the fields manually
+  std::memcmp(&a, &b, sizeof(S));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: comparing padding data in type
+  // alignment::S; consider comparing the fields manually
+}
+} // namespace alignment