llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang-tools-extra Author: Daniil Dudkin (unterumarmung) <details> <summary>Changes</summary> Teach `ExprMutationAnalyzer` to recognize references to const-qualified pointer objects, such as `T *const &`, as non-const pointee sinks when the pointee type itself is non-const. Fixes #<!-- -->190218 Assisted by Codex. --- Full diff: https://github.com/llvm/llvm-project/pull/190421.diff 4 Files Affected: - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3) - (modified) clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp (+55) - (modified) clang/lib/Analysis/ExprMutationAnalyzer.cpp (+21-3) - (modified) clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp (+33) ``````````diff diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 36e311341f336..473d58035f58e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -317,6 +317,9 @@ Changes in existing checks - Fixed false positive where a pointer used with placement new was incorrectly diagnosed as allowing the pointee to be made ``const``. + - Fixed false positives when pointers were later passed or bound through + const-qualified pointer references. + - Improved :doc:`misc-multiple-inheritance <clang-tidy/checks/misc/multiple-inheritance>` by avoiding false positives when virtual inheritance causes concrete bases to be counted more than once. 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 4c42743af8f11..6d91de9c6fa2b 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 @@ -10,6 +10,8 @@ // RUN: }}' \ // RUN: -- -fno-delayed-template-parsing +#include <vector> + void pointee_to_const() { int a[] = {1, 2}; int *p_local0 = &a[0]; @@ -123,3 +125,56 @@ void pointer_in_emplacement_new() { // CHECK-NOT: warning new(ptr) int {123}; } + +void takesConstPointerRef(int *const &); +void takesConstPointerRRef(int *const &&); +using IntPtrAlias = int *; +typedef int *IntPtrTypedef; +void takesAliasConstPointerRef(IntPtrAlias const &); +void takesTypedefConstPointerRef(IntPtrTypedef const &); + +void ignore_const_pointer_reference_sinks() { + int value = 0; + + int *p_local0 = &value; + // CHECK-MESSAGES-NOT: warning: pointee of variable 'p_local0' + // CHECK-FIXES: int *p_local0 = &value; + takesConstPointerRef(p_local0); + + int *p_local1 = &value; + // CHECK-MESSAGES-NOT: warning: pointee of variable 'p_local1' + // CHECK-FIXES: int *p_local1 = &value; + int *const &ref = p_local1; + (void)ref; + + IntPtrAlias p_local2 = &value; + // CHECK-MESSAGES-NOT: warning: pointee of variable 'p_local2' + // CHECK-FIXES: IntPtrAlias p_local2 = &value; + takesAliasConstPointerRef(p_local2); + + int *p_local3 = &value; + // CHECK-MESSAGES-NOT: warning: pointee of variable 'p_local3' + // CHECK-FIXES: int *p_local3 = &value; + takesConstPointerRRef(static_cast<int *const &&>(p_local3)); + + IntPtrTypedef p_local4 = &value; + // CHECK-MESSAGES-NOT: warning: pointee of variable 'p_local4' + // CHECK-FIXES: IntPtrTypedef p_local4 = &value; + takesTypedefConstPointerRef(p_local4); + + int *p_local5 = &value; + // CHECK-MESSAGES-NOT: warning: pointee of variable 'p_local5' + // CHECK-FIXES: int *p_local5 = &value; + int *volatile &volatile_ref = p_local5; + (void)volatile_ref; +} + +void ignore_range_for_pointer_reference_sink() { + int value = 0; + std::vector<int *> const source = {&value}; + std::vector<int *> sink; + // CHECK-FIXES: for (int *element : source) + for (int *element : source) + // CHECK-MESSAGES-NOT: warning: pointee of variable 'element' + sink.push_back(element); +} diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp index 5def6ba3cac5a..224df4d2e9be2 100644 --- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp +++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp @@ -145,9 +145,20 @@ 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_NoOp && (ICE->getType() == ICE->getSubExpr()->getType()))) + kind == CK_UncheckedDerivedToBase) return resolveExpr(ICE->getSubExpr()); + if (kind == CK_NoOp) { + // Binding `T *` to `T *const &` only adds top-level qualifiers to the + // pointer object, so this `CK_NoOp` still refers to the same pointer. + const QualType CastType = + ICE->getType().getLocalUnqualifiedType().getCanonicalType(); + const QualType SubExprType = ICE->getSubExpr() + ->getType() + .getLocalUnqualifiedType() + .getCanonicalType(); + if (CastType == SubExprType) + return resolveExpr(ICE->getSubExpr()); + } return false; } @@ -227,6 +238,12 @@ const auto nonConstReferenceType = [] { referenceType(pointee(unless(isConstQualified())))); }; +const auto referenceToPointerWithNonConstPointeeType = [] { + return hasUnqualifiedDesugaredType( + referenceType(pointee(hasUnqualifiedDesugaredType( + pointerType(pointee(unless(isConstQualified()))))))); +}; + const auto nonConstPointerType = [] { return hasUnqualifiedDesugaredType( pointerType(pointee(unless(isConstQualified())))); @@ -767,7 +784,8 @@ ExprMutationAnalyzer::Analyzer::findPointeeMemberMutation(const Expr *Exp) { const Stmt * ExprMutationAnalyzer::Analyzer::findPointeeToNonConst(const Expr *Exp) { const auto NonConstPointerOrNonConstRefOrDependentType = type( - anyOf(nonConstPointerType(), nonConstReferenceType(), isDependentType())); + anyOf(nonConstPointerType(), nonConstReferenceType(), + referenceToPointerWithNonConstPointeeType(), isDependentType())); // assign const auto InitToNonConst = diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp index c63479dc26e0b..0304d2f527ca2 100644 --- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp +++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp @@ -1756,6 +1756,14 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByInitToNonConst) { match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_TRUE(isPointeeMutated(Results, AST.get())); } + { + const std::string Code = + "void f() { int* x = nullptr; int* const& b = x; }"; + auto AST = buildASTFromCodeWithArgs(Code, {}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } } TEST(ExprMutationAnalyzerTest, PointeeMutatedByAssignToNonConst) { @@ -1801,6 +1809,31 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByPassAsArgument) { match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_TRUE(isPointeeMutated(Results, AST.get())); } + { + const std::string Code = + "void b(int *const &); void f() { int* x = nullptr; b(x); }"; + auto AST = buildASTFromCodeWithArgs(Code, {}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = + "void b(int *const &&); void f() { int* x = nullptr; " + "b(static_cast<int *const &&>(x)); }"; + auto AST = buildASTFromCodeWithArgs(Code, {}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + const std::string Code = "using IntPtr = int *; void b(IntPtr const &); " + "void f() { int* x = nullptr; b(x); }"; + auto AST = buildASTFromCodeWithArgs(Code, {}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } { const std::string Code = "namespace std { typedef decltype(sizeof(int)) size_t; }" `````````` </details> https://github.com/llvm/llvm-project/pull/190421 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
