diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index d2d79dcc92ec2..00c2f3afacae9 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -329,7 +329,8 @@ Changes in existing checks - Improved :doc:`misc-const-correctness ` check to avoid false positives when pointers is tranferred to non-const references - and avoid false positives of function pointer. + and avoid false positives of function pointer and fix false + positives on return of non-const pointer. - Improved :doc:`misc-header-include-cycle ` check performance. diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp index a44a712cd4025..c9443e6ce2121 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp @@ -48,6 +48,11 @@ void ignore_const_alias() { p_local0 = &a[1]; } +void *return_non_const() { + void *const a = nullptr; + return a; +} + void function_pointer_basic() { void (*const fp)() = nullptr; fp(); diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp index 1e376da1be83d..75b17c545bb78 100644 --- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp +++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp @@ -140,7 +140,8 @@ class ExprPointeeResolve { // explicit cast will be checked in `findPointeeToNonConst` const CastKind kind = ICE->getCastKind(); if (kind == CK_LValueToRValue || kind == CK_DerivedToBase || - kind == CK_UncheckedDerivedToBase) + kind == CK_UncheckedDerivedToBase || + (kind == CK_NoOp && (ICE->getType() == ICE->getSubExpr()->getType()))) return resolveExpr(ICE->getSubExpr()); return false; } @@ -788,13 +789,16 @@ ExprMutationAnalyzer::Analyzer::findPointeeToNonConst(const Expr *Exp) { // FIXME: false positive if the pointee does not change in lambda const auto CaptureNoConst = lambdaExpr(hasCaptureInit(Exp)); - const auto Matches = - match(stmt(anyOf(forEachDescendant( - stmt(anyOf(AssignToNonConst, PassAsNonConstArg, - CastToNonConst, CaptureNoConst)) - .bind("stmt")), - forEachDescendant(InitToNonConst))), - Stm, Context); + const auto ReturnNoConst = + returnStmt(hasReturnValue(canResolveToExprPointee(Exp))); + + const auto Matches = match( + stmt(anyOf(forEachDescendant( + stmt(anyOf(AssignToNonConst, PassAsNonConstArg, + CastToNonConst, CaptureNoConst, ReturnNoConst)) + .bind("stmt")), + forEachDescendant(InitToNonConst))), + Stm, Context); return selectFirst("stmt", Matches); } diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp index 95f8ae266ae10..ef229606de0f0 100644 --- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp +++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp @@ -2038,4 +2038,42 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByConditionOperator) { EXPECT_TRUE(isPointeeMutated(Results, AST.get())); } +TEST(ExprMutationAnalyzerTest, PointeeMutatedByReturn) { + { + const std::string Code = R"( + int * f() { + int *const x = nullptr; + return x; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = R"( + int * f() { + int *const x = nullptr; + return x; + })"; + // in C++23, AST will have NoOp cast. + auto AST = + buildASTFromCodeWithArgs(Code, {"-Wno-everything", "-std=c++23"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = R"( + int const* f() { + int *const x = nullptr; + return x; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + } +} + } // namespace clang