This is an archive of the discontinued LLVM Phabricator instance.

Differential D97150

[clang-tidy] Simplify suspicious memset usage check
ClosedPublic

Authored by steveire on Feb 21 2021, 7:32 AM.

Details

Reviewers
aaron.ballman
njames93
Commits
rG42ce00ec3947: [clang-tidy] Simplify suspicious memset usage check

Diff Detail

Event Timeline

steveire created this revision.Feb 21 2021, 7:32 AM
Herald added a subscriber: xazax.hun. · View Herald TranscriptFeb 21 2021, 7:32 AM
steveire requested review of this revision.Feb 21 2021, 7:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 21 2021, 7:32 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Harbormaster completed remote builds in B90129: Diff 325304.Feb 21 2021, 8:04 AM
njames93 accepted this revision.Feb 21 2021, 1:27 PM
njames93 added inline comments.
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp
39

This eachOf looked suspicious inside of an unless. Removing it should have no effect tho.

This revision is now accepted and ready to land.Feb 21 2021, 1:27 PM
Closed by commit rG42ce00ec3947: [clang-tidy] Simplify suspicious memset usage check (authored by • stephenkelly). · Explain WhyFeb 27 2021, 4:17 AM
This revision was automatically updated to reflect the committed changes.
• stephenkelly added a commit: rG42ce00ec3947: [clang-tidy] Simplify suspicious memset usage check.

Revision Contents

PathSize
clang-tools-extra/
clang-tidy/
bugprone/
3 lines
19 lines

Diff 326897

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

Show All 19 Lines
/// For the user-facing documentation see: /// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-memset-usage.html /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-memset-usage.html
class SuspiciousMemsetUsageCheck : public ClangTidyCheck { class SuspiciousMemsetUsageCheck : public ClangTidyCheck {
public: public:
SuspiciousMemsetUsageCheck(StringRef Name, ClangTidyContext *Context) SuspiciousMemsetUsageCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {} : ClangTidyCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
llvm::Optional<TraversalKind> getCheckTraversalKind() const override {
return TK_IgnoreUnlessSpelledInSource;
}
}; };
} // namespace bugprone } // namespace bugprone
} // namespace tidy } // namespace tidy
} // namespace clang } // namespace clang
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUS_MEMSET_USAGE_H #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUS_MEMSET_USAGE_H

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

Show All 29 Lines auto MemsetDecl =
hasParameter(2, hasType(isInteger()))); hasParameter(2, hasType(isInteger())));
// Look for memset(x, '0', z). Probably memset(x, 0, z) was intended. // Look for memset(x, '0', z). Probably memset(x, 0, z) was intended.
Finder->addMatcher( Finder->addMatcher(
callExpr( callExpr(
callee(MemsetDecl), callee(MemsetDecl),
hasArgument(1, characterLiteral(equals(static_cast<unsigned>('0'))) hasArgument(1, characterLiteral(equals(static_cast<unsigned>('0')))
.bind("char-zero-fill")), .bind("char-zero-fill")),
unless( unless(hasArgument(
eachOf(hasArgument(0, anyOf(hasType(pointsTo(isAnyCharacter())), 0, anyOf(hasType(pointsTo(isAnyCharacter())),
njames93Unsubmitted
Not Done
ReplyInline Actions

This eachOf looked suspicious inside of an unless. Removing it should have no effect tho.

njames93: This eachOf looked suspicious inside of an unless. Removing it should have no effect tho.
hasType(arrayType(hasElementType( hasType(arrayType(hasElementType(isAnyCharacter()))))))),
isAnyCharacter()))))),
isInTemplateInstantiation()))),
this); this);
// Look for memset with an integer literal in its fill_char argument. // Look for memset with an integer literal in its fill_char argument.
// Will check if it gets truncated. // Will check if it gets truncated.
Finder->addMatcher(callExpr(callee(MemsetDecl), Finder->addMatcher(
hasArgument(1, integerLiteral().bind("num-fill")), callExpr(callee(MemsetDecl),
unless(isInTemplateInstantiation())), hasArgument(1, integerLiteral().bind("num-fill"))),
this); this);
// Look for memset(x, y, 0) as that is most likely an argument swap. // Look for memset(x, y, 0) as that is most likely an argument swap.
Finder->addMatcher( Finder->addMatcher(
callExpr(callee(MemsetDecl), callExpr(callee(MemsetDecl),
unless(hasArgument(1, anyOf(characterLiteral(equals( unless(hasArgument(1, anyOf(characterLiteral(equals(
static_cast<unsigned>('0'))), static_cast<unsigned>('0'))),
integerLiteral()))), integerLiteral()))))
unless(isInTemplateInstantiation()))
.bind("call"), .bind("call"),
this); this);
} }
void SuspiciousMemsetUsageCheck::check(const MatchFinder::MatchResult &Result) { void SuspiciousMemsetUsageCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *CharZeroFill = if (const auto *CharZeroFill =
Result.Nodes.getNodeAs<CharacterLiteral>("char-zero-fill")) { Result.Nodes.getNodeAs<CharacterLiteral>("char-zero-fill")) {
// Case 1: fill_char of memset() is a character '0'. Probably an // Case 1: fill_char of memset() is a character '0'. Probably an
▲ Show 20 Lines • Show All 75 Lines • Show Last 20 Lines