Author: David Meng Date: 2026-08-12T11:22:46+03:00 New Revision: 0cc5755f0b67c49cf02747d2de1af8961b24d156
URL: https://github.com/llvm/llvm-project/commit/0cc5755f0b67c49cf02747d2de1af8961b24d156 DIFF: https://github.com/llvm/llvm-project/commit/0cc5755f0b67c49cf02747d2de1af8961b24d156.diff LOG: [clang-tidy] Fix `misc-const-correctness` false positive on writes through adjusted pointers (#215285) `ExprPointeeResolve` did not resolve through `++`/`--` or `+=`/`-=`, so a write such as `*p++ = 0` was not recognised as mutating `p`'s pointee and the check suggested a `const` pointee that does not compile: ```cpp void func(int *ptr1) { int *ptr2 = ptr1; *ptr2++ = 0; // warning: pointee of 'ptr2' can be declared 'const' } ``` Godbolt: https://godbolt.org/z/fxW8Ea1WT Resolve through increment/decrement, like the existing additive case, and through `+=`/`-=`, where only the LHS can be the pointer. Reads through an adjusted pointer (e.g. `int i = *p++;`) still get the `const` suggestion. Fixes https://github.com/llvm/llvm-project/issues/215161 Added: Modified: clang-tools-extra/docs/ReleaseNotes.md clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp clang/lib/Analysis/ExprMutationAnalyzer.cpp clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 29de9aef9e4b6..e181a7d22a948 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -122,6 +122,11 @@ infrastructure are described first, followed by tool-specific sections. <clang-tidy/checks/cppcoreguidelines/pro-type-member-init>` check by treating `std::array` the same as built-in arrays when `IgnoreArrays` option is enabled. +- Improved {doc}`misc-const-correctness + <clang-tidy/checks/misc/const-correctness>` check by fixing false positives + when the pointee is written through a pointer that is incremented, + decremented or adjusted with `+=` or `-=`, such as `*p++ = 0`. + - Improved {doc}`misc-redundant-expression <clang-tidy/checks/misc/redundant-expression>` by fixing false positives in nested expressions involving diff erent macros or a mix of macro and 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 811d5e08cb219..639ea0933c10b 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 @@ -88,6 +88,97 @@ void pass_as_const_void_pointer() { take_const_void_pointer(p_local0); } +void write_through_post_increment(int *p) { + int *p_local0 = p; + // CHECK-NOT: warning + *p_local0++ = 0; +} + +void write_through_pre_increment(int *p) { + int *p_local0 = p; + // CHECK-NOT: warning + *++p_local0 = 0; +} + +void write_through_post_decrement(int *p) { + int *p_local0 = p; + // CHECK-NOT: warning + *p_local0-- = 0; +} + +void write_through_pre_decrement(int *p) { + int *p_local0 = p; + // CHECK-NOT: warning + *--p_local0 = 0; +} + +void write_through_increment_subscript(int *p) { + int *p_local0 = p; + // CHECK-NOT: warning + (p_local0++)[0] = 0; +} + +void write_through_add_assign(int *p) { + int *p_local0 = p; + // CHECK-NOT: warning + *(p_local0 += 1) = 0; +} + +void write_through_sub_assign(int *p) { + int *p_local0 = p; + // CHECK-NOT: warning + *(p_local0 -= 1) = 0; +} + +void write_through_add_assign_subscript(int *p) { + int *p_local0 = p; + // CHECK-NOT: warning + (p_local0 += 1)[0] = 0; +} + +void alias_through_increment(int *p) { + int *p_local0 = p; + // CHECK-NOT: warning + int *q = p_local0++; + *q = 0; +} + +void alias_through_add_assign(int *p) { + int *p_local0 = p; + // CHECK-NOT: warning + int *q = (p_local0 += 1); + *q = 0; +} + +template <class T> +void template_write_through_pointer_arithmetic() { + T a[] = {1, 2}; + T *p_local0 = &a[0]; + // CHECK-NOT: warning + *p_local0++ = 0; + T *p_local1 = &a[0]; + // CHECK-NOT: warning + *(p_local1 += 1) = 0; +} + +void instantiate_write_through_pointer_arithmetic() { + template_write_through_pointer_arithmetic<int>(); +} + +void read_through_increment(int *p) { + int *p_local0 = p; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: pointee of variable 'p_local0' of type 'int *' can be declared 'const' + // CHECK-FIXES: int const*p_local0 = p; + int i = *p_local0++; +} + +void read_through_add_assign(int *p) { + int *p_local0 = p; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: pointee of variable 'p_local0' of type 'int *' can be declared 'const' + // CHECK-FIXES: int const*p_local0 = p; + int i = *(p_local0 += 1); +} + void function_pointer_basic() { void (*const fp)() = nullptr; fp(); diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp index 59fad2a41675c..436308f558b66 100644 --- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp +++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp @@ -127,6 +127,8 @@ class ExprPointeeResolve { if (const auto *BO = dyn_cast<BinaryOperator>(E)) { if (BO->isAdditiveOp()) return (resolveExpr(BO->getLHS()) || resolveExpr(BO->getRHS())); + if (BO->getOpcode() == BO_AddAssign || BO->getOpcode() == BO_SubAssign) + return resolveExpr(BO->getLHS()); if (BO->isCommaOp()) return resolveExpr(BO->getRHS()); return false; @@ -136,7 +138,7 @@ class ExprPointeeResolve { return resolveExpr(PE->getSubExpr()); if (const auto *UO = dyn_cast<UnaryOperator>(E)) { - if (UO->getOpcode() == UO_AddrOf) + if (UO->getOpcode() == UO_AddrOf || UO->isIncrementDecrementOp()) return resolveExpr(UO->getSubExpr()); } diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp index 4ccd1f554eefc..0f4245a323f84 100644 --- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp +++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp @@ -2155,6 +2155,91 @@ TEST(ExprMutationAnalyzerTest, PointeeMutatedByPointerArithmeticSubElement) { EXPECT_TRUE(isPointeeMutated(Results, AST.get())); } +TEST(ExprMutationAnalyzerTest, PointeeMutatedByPointerArithmeticIncDec) { + for (const std::string Deref : {"*x++", "*++x", "*x--", "*--x", "(x++)[0]"}) { + const std::string Code = "void f() { int* x; " + Deref + " = 0; }"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())) << Code; + } + for (const std::string Deref : {"*x++", "*++x", "*x--", "*--x", "(x++)[0]"}) { + const std::string Code = "void f() { int* x; int y = " + Deref + "; }"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())) << Code; + } + { + // The adjusted pointer escapes into a non-const pointer, which can be used + // to mutate the pointee. + const std::string Code = R"( + void f() { + int* x; + int* y = x++; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + // Adjusting the pointer itself does not mutate the pointee. + const std::string Code = R"( + void f() { + int* x; + x++; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + } +} + +TEST(ExprMutationAnalyzerTest, + PointeeMutatedByPointerArithmeticCompoundAssign) { + for (const std::string Deref : {"*(x += 1)", "*(x -= 1)", "(x += 1)[0]"}) { + const std::string Code = "void f() { int* x; " + Deref + " = 0; }"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())) << Code; + } + for (const std::string Deref : {"*(x += 1)", "*(x -= 1)", "(x += 1)[0]"}) { + const std::string Code = "void f() { int* x; int y = " + Deref + "; }"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())) << Code; + } + { + // The adjusted pointer escapes into a non-const pointer, which can be used + // to mutate the pointee. + const std::string Code = R"( + void f() { + int* x; + int* y = (x += 1); + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_TRUE(isPointeeMutated(Results, AST.get())); + } + { + // Adjusting the pointer itself does not mutate the pointee. + const std::string Code = R"( + void f() { + int* x; + x += 1; + })"; + auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-everything"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_FALSE(isPointeeMutated(Results, AST.get())); + } +} + TEST(ExprMutationAnalyzerTest, PointeeMutatedByConditionOperator) { const std::string Code = R"( void f() { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
