Skip to content

Commit

Permalink
[analysis] assume expr is not mutated after analysis to avoid recursi…
Browse files Browse the repository at this point in the history
…ve (#90581)

Fixes: #89376.
  • Loading branch information
HerrCai0907 authored May 1, 2024
1 parent fa53545 commit 6e31714
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 5 deletions.
3 changes: 2 additions & 1 deletion clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,8 @@ Changes in existing checks

- Improved :doc:`misc-const-correctness
<clang-tidy/checks/misc/const-correctness>` check by avoiding infinite recursion
for recursive forwarding reference.
for recursive functions with forwarding reference parameters and reference
variables which refer to themselves.

- Improved :doc:`misc-definitions-in-headers
<clang-tidy/checks/misc/definitions-in-headers>` check by replacing the local
Expand Down
6 changes: 4 additions & 2 deletions clang/lib/Analysis/ExprMutationAnalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,15 +235,17 @@ const Stmt *ExprMutationAnalyzer::Analyzer::findMutationMemoized(
if (Memoized != MemoizedResults.end())
return Memoized->second;

// Assume Exp is not mutated before analyzing Exp.
MemoizedResults[Exp] = nullptr;
if (isUnevaluated(Exp))
return MemoizedResults[Exp] = nullptr;
return nullptr;

for (const auto &Finder : Finders) {
if (const Stmt *S = (this->*Finder)(Exp))
return MemoizedResults[Exp] = S;
}

return MemoizedResults[Exp] = nullptr;
return nullptr;
}

const Stmt *
Expand Down
26 changes: 24 additions & 2 deletions clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
#include "clang/AST/TypeLoc.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Frontend/ASTUnit.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/SmallString.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <cctype>
Expand Down Expand Up @@ -43,7 +43,7 @@ std::unique_ptr<ASTUnit> buildASTFromCode(const Twine &Code) {
}

ExprMatcher declRefTo(StringRef Name) {
return declRefExpr(to(namedDecl(hasName(Name))));
return declRefExpr(to(namedDecl(hasName(Name)).bind("decl")));
}

StmtMatcher withEnclosingCompound(ExprMatcher Matcher) {
Expand All @@ -57,6 +57,13 @@ bool isMutated(const SmallVectorImpl<BoundNodes> &Results, ASTUnit *AST) {
return ExprMutationAnalyzer(*S, AST->getASTContext()).isMutated(E);
}

bool isDeclMutated(const SmallVectorImpl<BoundNodes> &Results, ASTUnit *AST) {
const auto *const S = selectFirst<Stmt>("stmt", Results);
const auto *const D = selectFirst<Decl>("decl", Results);
TraversalKindScope RAII(AST->getASTContext(), TK_AsIs);
return ExprMutationAnalyzer(*S, AST->getASTContext()).isMutated(D);
}

SmallVector<std::string, 1>
mutatedBy(const SmallVectorImpl<BoundNodes> &Results, ASTUnit *AST) {
const auto *const S = selectFirst<Stmt>("stmt", Results);
Expand Down Expand Up @@ -1552,6 +1559,21 @@ TEST(ExprMutationAnalyzerTest, UniquePtr) {

// section: complex problems detected on real code

TEST(ExprMutationAnalyzerTest, SelfRef) {
std::unique_ptr<ASTUnit> AST{};
SmallVector<BoundNodes, 1> Results{};

AST = buildASTFromCodeWithArgs("void f() { int &x = x; }",
{"-Wno-unused-value", "-Wno-uninitialized"});
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_FALSE(isDeclMutated(Results, AST.get()));

AST = buildASTFromCodeWithArgs("void f() { int &x = x; x = 1; }",
{"-Wno-unused-value", "-Wno-uninitialized"});
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_TRUE(isDeclMutated(Results, AST.get()));
}

TEST(ExprMutationAnalyzerTest, UnevaluatedContext) {
const std::string Example =
"template <typename T>"
Expand Down

0 comments on commit 6e31714

Please sign in to comment.