Author: Marco Antognini Date: 2022-05-03T11:27:45+02:00 New Revision: 68ee5ec07d4a169baf287acad9ad7fa85d764a22
URL: https://github.com/llvm/llvm-project/commit/68ee5ec07d4a169baf287acad9ad7fa85d764a22 DIFF: https://github.com/llvm/llvm-project/commit/68ee5ec07d4a169baf287acad9ad7fa85d764a22.diff LOG: [Analyzer] Fix assumptions about const field with member-initializer Essentially, having a default member initializer for a constant member does not necessarily imply the member will have the given default value. Remove part of a2e053638bbf ([analyzer] Treat more const variables and fields as known contants., 2018-05-04). Fix #47878 Reviewed By: r.stahl, steakhal Differential Revision: https://reviews.llvm.org/D124621 Added: clang/test/Analysis/cxx-member-initializer-const-field.cpp Modified: clang/lib/StaticAnalyzer/Core/RegionStore.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp index 9d062ddb3e8c1..20b167c9b3b22 100644 --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -1983,15 +1983,9 @@ SVal RegionStoreManager::getBindingForField(RegionBindingsConstRef B, if (const Optional<SVal> &V = B.getDirectBinding(R)) return *V; - // Is the field declared constant and has an in-class initializer? + // If the containing record was initialized, try to get its constant value. const FieldDecl *FD = R->getDecl(); QualType Ty = FD->getType(); - if (Ty.isConstQualified()) - if (const Expr *Init = FD->getInClassInitializer()) - if (Optional<SVal> V = svalBuilder.getConstantVal(Init)) - return *V; - - // If the containing record was initialized, try to get its constant value. const MemRegion* superR = R->getSuperRegion(); if (const auto *VR = dyn_cast<VarRegion>(superR)) { const VarDecl *VD = VR->getDecl(); diff --git a/clang/test/Analysis/cxx-member-initializer-const-field.cpp b/clang/test/Analysis/cxx-member-initializer-const-field.cpp new file mode 100644 index 0000000000000..f0abbddbc4441 --- /dev/null +++ b/clang/test/Analysis/cxx-member-initializer-const-field.cpp @@ -0,0 +1,120 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s + +// This tests false-positive issues related to PR48534. +// +// Essentially, having a default member initializer for a constant member does +// not necessarily imply the member will have the given default value. + +struct WithConstructor { + int *const ptr = nullptr; + WithConstructor(int *x) : ptr(x) {} + + static auto compliant() { + WithConstructor c(new int); + return *(c.ptr); // no warning + } + + static auto compliantWithParam(WithConstructor c) { + return *(c.ptr); // no warning + } + + static auto issue() { + WithConstructor c(nullptr); + return *(c.ptr); // expected-warning{{Dereference of null pointer (loaded from field 'ptr')}} + } +}; + +struct RegularAggregate { + int *const ptr = nullptr; + + static int compliant() { + RegularAggregate c{new int}; + return *(c.ptr); // no warning + } + + static int issue() { + RegularAggregate c; + return *(c.ptr); // expected-warning{{Dereference of null pointer (loaded from field 'ptr')}} + } +}; + +struct WithConstructorAndArithmetic { + int const i = 0; + WithConstructorAndArithmetic(int x) : i(x + 1) {} + + static int compliant(int y) { + WithConstructorAndArithmetic c(0); + return y / c.i; // no warning + } + + static int issue(int y) { + WithConstructorAndArithmetic c(-1); + return y / c.i; // expected-warning{{Division by zero}} + } +}; + +struct WithConstructorDeclarationOnly { + int const i = 0; + WithConstructorDeclarationOnly(int x); // definition not visible. + + static int compliant1(int y) { + WithConstructorDeclarationOnly c(0); + return y / c.i; // no warning + } + + static int compliant2(int y) { + WithConstructorDeclarationOnly c(-1); + return y / c.i; // no warning + } +}; + +// NonAggregateFP is not an aggregate (j is a private non-static field) and has no custom constructor. +// So we know i and j will always be 0 and 42, respectively. +// That being said, this is not implemented because it is deemed too rare to be worth the complexity. +struct NonAggregateFP { +public: + int const i = 0; + +private: + int const j = 42; + +public: + static int falsePositive1(NonAggregateFP c) { + return 10 / c.i; // FIXME: Currently, no warning. + } + + static int falsePositive2(NonAggregateFP c) { + return 10 / (c.j - 42); // FIXME: Currently, no warning. + } +}; + +struct NonAggregate { +public: + int const i = 0; + +private: + int const j = 42; + + NonAggregate(NonAggregate const &); // not provided, could set i and j to arbitrary values. + +public: + static int compliant1(NonAggregate c) { + return 10 / c.i; // no warning + } + + static int compliant2(NonAggregate c) { + return 10 / (c.j - 42); // no warning + } +}; + +struct WithStaticMember { + static int const i = 0; + + static int issue1(WithStaticMember c) { + return 10 / c.i; // expected-warning{{division by zero is undefined}} expected-warning{{Division by zero}} + } + + static int issue2() { + return 10 / WithStaticMember::i; // expected-warning{{division by zero is undefined}} expected-warning{{Division by zero}} + } +}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
