Index: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h =================================================================== --- include/clang/Analysis/Analyses/ExprMutationAnalyzer.h +++ include/clang/Analysis/Analyses/ExprMutationAnalyzer.h @@ -66,6 +66,9 @@ const Stmt *findReferenceMutation(const Expr *Exp); const Stmt *findFunctionArgMutation(const Expr *Exp); + const Stmt *findPointeeDirectMutation(const Expr *Exp); + const Stmt *findPointeeCastMutation(const Expr *Exp); + const Stmt &Stm; ASTContext &Context; llvm::DenseMapgetType().isConstant(Context)) + return nullptr; + const bool ExpIsPointer = Exp->getType()->getAs(); + // LHS of any assignment operators. const auto AsAssignmentLhs = binaryOperator(isAssignmentOperator(), hasLHS(equalsNode(Exp))); @@ -204,14 +213,17 @@ // Invoking non-const member function. // A member function is assumed to be non-const when it is unresolved. const auto NonConstMethod = cxxMethodDecl(unless(isConst())); - const auto AsNonConstThis = - expr(anyOf(cxxMemberCallExpr(callee(NonConstMethod), on(equalsNode(Exp))), - cxxOperatorCallExpr(callee(NonConstMethod), - hasArgument(0, equalsNode(Exp))), - callExpr(callee(expr(anyOf( - unresolvedMemberExpr(hasObjectExpression(equalsNode(Exp))), - cxxDependentScopeMemberExpr( - hasObjectExpression(equalsNode(Exp))))))))); + const auto AsNonConstNonPointerThis = + ExpIsPointer + ? expr(unless(anything())) + : expr(anyOf( + cxxMemberCallExpr(callee(NonConstMethod), on(equalsNode(Exp))), + cxxOperatorCallExpr(callee(NonConstMethod), + hasArgument(0, equalsNode(Exp))), + callExpr(callee(expr(anyOf( + unresolvedMemberExpr(hasObjectExpression(equalsNode(Exp))), + cxxDependentScopeMemberExpr( + hasObjectExpression(equalsNode(Exp))))))))); // Taking address of 'Exp'. // We're assuming 'Exp' is mutated as soon as its address is taken, though in @@ -224,15 +236,19 @@ hasUnaryOperand(equalsNode(Exp))); const auto AsPointerFromArrayDecay = castExpr(hasCastKind(CK_ArrayToPointerDecay), + // A NoOp implicit cast is adding const. + unless(hasParent(implicitCastExpr(hasCastKind(CK_NoOp)))), unless(hasParent(arraySubscriptExpr())), has(equalsNode(Exp))); // Treat calling `operator->()` of move-only classes as taking address. // These are typically smart pointers with unique ownership so we treat // mutation of pointee as mutation of the smart pointer itself. const auto AsOperatorArrowThis = - cxxOperatorCallExpr(hasOverloadedOperatorName("->"), - callee(cxxMethodDecl(ofClass(isMoveOnly()), - returns(nonConstPointerType()))), - argumentCountIs(1), hasArgument(0, equalsNode(Exp))); + ExpIsPointer ? cxxOperatorCallExpr(unless(anything())) + : cxxOperatorCallExpr( + hasOverloadedOperatorName("->"), + callee(cxxMethodDecl(ofClass(isMoveOnly()), + returns(nonConstPointerType()))), + argumentCountIs(1), hasArgument(0, equalsNode(Exp))); // Used as non-const-ref argument when calling a function. // An argument is assumed to be non-const-ref when the function is unresolved. @@ -265,10 +281,11 @@ const auto AsNonConstRefReturn = returnStmt(hasReturnValue(equalsNode(Exp))); const auto Matches = - match(findAll(stmt(anyOf(AsAssignmentLhs, AsIncDecOperand, AsNonConstThis, - AsAmpersandOperand, AsPointerFromArrayDecay, - AsOperatorArrowThis, AsNonConstRefArg, - AsLambdaRefCaptureInit, AsNonConstRefReturn)) + match(findAll(stmt(anyOf(AsAssignmentLhs, AsIncDecOperand, + AsNonConstNonPointerThis, AsAmpersandOperand, + AsPointerFromArrayDecay, AsOperatorArrowThis, + AsNonConstRefArg, AsLambdaRefCaptureInit, + AsNonConstRefReturn)) .bind("stmt")), Stm, Context); return selectFirst("stmt", Matches); @@ -412,6 +429,84 @@ return nullptr; } +const Stmt *ExprMutationAnalyzer::findPointeeDirectMutation(const Expr *Exp) { + // Pointee of `Exp` can be *directly* mutated if the type of `Exp` is: + // - Pointer to non-const + // - Pointer-like type with `operator*` returning non-const reference + // Bail out early otherwise. + if (const auto *PT = Exp->getType()->getAs()) { + if (PT->getPointeeType().isConstant(Context)) + return nullptr; + } else if (const auto *RT = Exp->getType()->getAs()) { + if (const CXXRecordDecl *RecordDecl = RT->getAsCXXRecordDecl()) { + const bool ExpIsConst = Exp->getType().isConstant(Context); + if (llvm::any_of( + RecordDecl->methods(), + [ExpIsConst, this](const CXXMethodDecl *MethodDecl) { + // Look for `operator*` with the same constness as `Exp`. + if (MethodDecl->getOverloadedOperator() != OO_Star || + MethodDecl->isConst() != ExpIsConst) + return false; + // Check whether returning non-const reference. + if (const auto *RT = + MethodDecl->getReturnType()->getAs()) + return !RT->getPointeeType().isConstant(Context); + return false; + })) + return nullptr; + } + } + const bool ExpIsPointer = Exp->getType()->getAs(); + + // Invoking non-const member function. + // A member function is assumed to be non-const when it is unresolved. + // We don't handle pointer-like type here as non-const member function of + // pointee can't be directly invoked without dereferencing the pointer-like + // object first. + const auto NonConstMethod = cxxMethodDecl(unless(isConst())); + const auto AsNonConstPointerThis = + ExpIsPointer + ? expr(anyOf( + cxxMemberCallExpr(callee(NonConstMethod), on(equalsNode(Exp))), + cxxOperatorCallExpr(callee(NonConstMethod), + hasArgument(0, equalsNode(Exp))), + callExpr(callee(expr(anyOf( + unresolvedMemberExpr(hasObjectExpression(equalsNode(Exp))), + cxxDependentScopeMemberExpr( + hasObjectExpression(equalsNode(Exp))))))))) + : expr(unless(anything())); + + // Used as an argument when calling a function. + const auto AsArg = + anyOf(callExpr(hasAnyArgument(equalsNode(Exp))), + cxxConstructExpr(hasAnyArgument(equalsNode(Exp))), + cxxUnresolvedConstructExpr(hasAnyArgument(equalsNode(Exp)))); + + // Captured by a lambda. + const auto AsLambdaCaptureInit = lambdaExpr(hasCaptureInit(Exp)); + + // Returned. + const auto AsReturnValue = returnStmt(hasReturnValue(equalsNode(Exp))); + + const auto Matches = + match(findAll(stmt(anyOf(AsNonConstPointerThis, AsArg, + AsLambdaCaptureInit, AsReturnValue)) + .bind("stmt")), + Stm, Context); + return selectFirst("stmt", Matches); +} + +const Stmt *ExprMutationAnalyzer::findPointeeCastMutation(const Expr *Exp) { + // Only handling LValueToRValue for now for easier unit testing during + // implementation. + const auto Casts = + match(findAll(implicitCastExpr(hasSourceExpression(equalsNode(Exp)), + hasCastKind(CK_LValueToRValue)) + .bind(NodeID::value)), + Stm, Context); + return findExprPointeeMutation(Casts); +} + FunctionParmMutationAnalyzer::FunctionParmMutationAnalyzer( const FunctionDecl &Func, ASTContext &Context) : BodyAnalyzer(*Func.getBody(), Context) { Index: unittests/Analysis/ExprMutationAnalyzerTest.cpp =================================================================== --- unittests/Analysis/ExprMutationAnalyzerTest.cpp +++ unittests/Analysis/ExprMutationAnalyzerTest.cpp @@ -52,16 +52,34 @@ bool isMutated(const SmallVectorImpl &Results, ASTUnit *AST) { const auto *const S = selectFirst("stmt", Results); const auto *const E = selectFirst("expr", Results); + assert(E); return ExprMutationAnalyzer(*S, AST->getASTContext()).isMutated(E); } +bool isPointeeMutated(const SmallVectorImpl &Results, + ASTUnit *AST) { + const auto *const S = selectFirst("stmt", Results); + const auto *const E = selectFirst("expr", Results); + assert(E); + return ExprMutationAnalyzer(*S, AST->getASTContext()).isPointeeMutated(E); +} + SmallVector mutatedBy(const SmallVectorImpl &Results, ASTUnit *AST) { + const Stmt *(ExprMutationAnalyzer::*MutationFinder)(const Expr *) = + &ExprMutationAnalyzer::findMutation; + const Stmt *(ExprMutationAnalyzer::*PointeeMutationFinder)(const Expr *) = + &ExprMutationAnalyzer::findPointeeMutation; const auto *const S = selectFirst("stmt", Results); SmallVector Chain; ExprMutationAnalyzer Analyzer(*S, AST->getASTContext()); for (const auto *E = selectFirst("expr", Results); E != nullptr;) { - const Stmt *By = Analyzer.findMutation(E); + auto Finder = MutationFinder; + if (const auto *DRE = dyn_cast(E)) { + if (DRE->getNameInfo().getAsString()[0] == 'p') + Finder = PointeeMutationFinder; + } + const Stmt *By = (Analyzer.*Finder)(E); std::string buffer; llvm::raw_string_ostream stream(buffer); By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy()); @@ -100,10 +118,14 @@ } // namespace TEST(ExprMutationAnalyzerTest, Trivial) { - const auto AST = buildASTFromCode("void f() { int x; x; }"); - const auto Results = + auto AST = buildASTFromCode("void f() { int x; x; }"); + auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = buildASTFromCode("void f() { const int x = 0; x; }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); } class AssignmentTest : public ::testing::TestWithParam {}; @@ -134,41 +156,104 @@ Values("++x", "--x", "x++", "x--"), ); TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) { - const auto AST = buildASTFromCode( + auto AST = buildASTFromCode( "void f() { struct Foo { void mf(); }; Foo x; x.mf(); }"); - const auto Results = + auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isMutated(Results, AST.get())); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()")); + + AST = buildASTFromCode( + "void f() { struct Foo { void mf(); }; Foo *p; p->mf(); }"); + Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("p->mf()")); } TEST(ExprMutationAnalyzerTest, AssumedNonConstMemberFunc) { auto AST = buildASTFromCodeWithArgs( "struct X { template void mf(); };" - "template void f() { X x; x.mf(); }", + "template void f() { X x; x.mf(); }" + "template void g() { X *p; p->mf(); }", {"-fno-delayed-template-parsing"}); auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()")); + Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("p->mf()")); - AST = buildASTFromCodeWithArgs("template void f() { T x; x.mf(); }", - {"-fno-delayed-template-parsing"}); + AST = + buildASTFromCodeWithArgs("template void f() { T x; x.mf(); }" + "template void g() { T *p; p->mf(); }", + {"-fno-delayed-template-parsing"}); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()")); + Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("p->mf()")); AST = buildASTFromCodeWithArgs( "template struct X;" - "template void f() { X x; x.mf(); }", + "template void f() { X x; x.mf(); }" + "template void g() { X *p; p->mf(); }", {"-fno-delayed-template-parsing"}); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()")); + Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("p->mf()")); + + AST = buildASTFromCodeWithArgs( + "struct X { template void mf() const; };" + "template void f() { const X x; x.mf(); }" + "template void g() { const X *p; p->mf(); }", + {"-fno-delayed-template-parsing"}); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + + AST = buildASTFromCodeWithArgs( + "template void f() { const T x; x.mf(); }" + "template void g() { const T *p; p->mf(); }", + {"-fno-delayed-template-parsing"}); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + + AST = buildASTFromCodeWithArgs( + "template struct X;" + "template void f() { const X x; x.mf(); }" + "template void g() { const X *p; p->mf(); }", + {"-fno-delayed-template-parsing"}); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); } TEST(ExprMutationAnalyzerTest, ConstMemberFunc) { - const auto AST = buildASTFromCode( + auto AST = buildASTFromCode( "void f() { struct Foo { void mf() const; }; Foo x; x.mf(); }"); - const auto Results = + auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = buildASTFromCode( + "void f() { struct Foo { void mf() const; }; const Foo x; x.mf(); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = buildASTFromCode( + "void f() { struct Foo { void mf() const; }; Foo *p; p->mf(); }"); + Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + + AST = buildASTFromCode( + "void f() { struct Foo { void mf() const; }; const Foo *p; p->mf(); }"); + Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); } TEST(ExprMutationAnalyzerTest, NonConstOperator) { @@ -193,9 +278,11 @@ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_FALSE(isMutated(Results, AST.get())); - AST = buildASTFromCode("void g(int*); void f() { int* x; g(x); }"); - Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + AST = buildASTFromCode("void g(int*); void f() { int* p; g(p); }"); + Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext()); EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(p)")); AST = buildASTFromCode("typedef int* IntPtr;" "void g(IntPtr); void f() { int* x; g(x); }"); @@ -559,18 +646,27 @@ } TEST(ExprMutationAnalyzerTest, TakeAddress) { - const auto AST = buildASTFromCode("void g(int*); void f() { int x; g(&x); }"); - const auto Results = + auto AST = buildASTFromCode("void g(int*); void f() { int x; g(&x); }"); + auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("&x")); + + AST = buildASTFromCode( + "void g(const int*); void f() { const int x = 10; g(&x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); } TEST(ExprMutationAnalyzerTest, ArrayToPointerDecay) { - const auto AST = - buildASTFromCode("void g(int*); void f() { int x[2]; g(x); }"); - const auto Results = + auto AST = buildASTFromCode("void g(int*); void f() { int x[2]; g(x); }"); + auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x")); + + AST = buildASTFromCode( + "void g(const int*); void f() { const int x[2] = {}; g(x); }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); } TEST(ExprMutationAnalyzerTest, TemplateWithArrayToPointerDecay) { @@ -850,33 +946,55 @@ } TEST(ExprMutationAnalyzerTest, LambdaDefaultCaptureByValue) { - const auto AST = buildASTFromCode("void f() { int x; [=]() { x; }; }"); - const auto Results = + auto AST = buildASTFromCode("void f() { int x; [=]() { x; }; }"); + auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = buildASTFromCode("void f() { int *p; [=]() { *p = 10; }; }"); + Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre(ResultOf(removeSpace, "[=](){*p=10;}"))); } TEST(ExprMutationAnalyzerTest, LambdaExplicitCaptureByValue) { - const auto AST = buildASTFromCode("void f() { int x; [x]() { x; }; }"); - const auto Results = + auto AST = buildASTFromCode("void f() { int x; [x]() { x; }; }"); + auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_FALSE(isMutated(Results, AST.get())); + + AST = buildASTFromCode("void f() { int *p; [p]() { *p = 10; }; }"); + Results = match(withEnclosingCompound(declRefTo("p")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre(ResultOf(removeSpace, "[p](){*p=10;}"))); } TEST(ExprMutationAnalyzerTest, LambdaDefaultCaptureByRef) { - const auto AST = buildASTFromCode("void f() { int x; [&]() { x = 10; }; }"); - const auto Results = + auto AST = buildASTFromCode("void f() { int x; [&]() { x = 10; }; }"); + auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ResultOf(removeSpace, "[&](){x=10;}"))); + + AST = buildASTFromCode("void f() { const int x = 10; [&]() { x; }; }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); } TEST(ExprMutationAnalyzerTest, LambdaExplicitCaptureByRef) { - const auto AST = buildASTFromCode("void f() { int x; [&x]() { x = 10; }; }"); - const auto Results = + auto AST = buildASTFromCode("void f() { int x; [&x]() { x = 10; }; }"); + auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ResultOf(removeSpace, "[&x](){x=10;}"))); + + AST = buildASTFromCode("void f() { const int x = 10; [&x]() { x; }; }"); + Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isMutated(Results, AST.get())); } TEST(ExprMutationAnalyzerTest, RangeForArrayByRefModified) {