[clang] 44e803e - [analyzer][NFCI] Move a block from `getBindingForElement` to separate functions
Denys Petrov via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 25 05:14:19 PDT 2021
Author: Denys Petrov
Date: 2021-10-25T15:14:10+03:00
New Revision: 44e803ef6d41770adf309960e37bcc6a75dbbe2c
URL: https://github.com/llvm/llvm-project/commit/44e803ef6d41770adf309960e37bcc6a75dbbe2c
DIFF: https://github.com/llvm/llvm-project/commit/44e803ef6d41770adf309960e37bcc6a75dbbe2c.diff
LOG: [analyzer][NFCI] Move a block from `getBindingForElement` to separate functions
Summary:
1. Improve readability by moving deeply nested block of code from RegionStoreManager::getBindingForElement to new separate functions:
- getConstantValFromConstArrayInitializer;
- getSValFromInitListExpr.
2. Handle the case when index is a symbolic value. Write specific test cases.
3. Add test cases when there is no initialization expression presented.
This patch implies to make next patches clearer and easier for review process.
Differential Revision: https://reviews.llvm.org/D106681
Added:
Modified:
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/initialization.c
clang/test/Analysis/initialization.cpp
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 5e7352cc8756b..e0e6bca1e7cc3 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -437,6 +437,10 @@ class RegionStoreManager : public StoreManager {
RegionBindingsRef removeSubRegionBindings(RegionBindingsConstRef B,
const SubRegion *R);
+ Optional<SVal> getConstantValFromConstArrayInitializer(
+ RegionBindingsConstRef B, const VarRegion *VR, const ElementRegion *R);
+ Optional<SVal> getSValFromInitListExpr(const InitListExpr *ILE,
+ uint64_t Offset, QualType ElemT);
public: // Part of public interface to class.
@@ -1625,6 +1629,95 @@ RegionStoreManager::findLazyBinding(RegionBindingsConstRef B,
return Result;
}
+Optional<SVal> RegionStoreManager::getConstantValFromConstArrayInitializer(
+ RegionBindingsConstRef B, const VarRegion *VR, const ElementRegion *R) {
+ assert(R && VR && "Regions should not be null");
+
+ // Check if the containing array has an initialized value that we can trust.
+ // We can trust a const value or a value of a global initializer in main().
+ const VarDecl *VD = VR->getDecl();
+ if (!VD->getType().isConstQualified() &&
+ !R->getElementType().isConstQualified() &&
+ (!B.isMainAnalysis() || !VD->hasGlobalStorage()))
+ return None;
+
+ // Array's declaration should have an initializer.
+ const Expr *Init = VD->getAnyInitializer();
+ if (!Init)
+ return None;
+
+ // Array's declaration should have ConstantArrayType type, because only this
+ // type contains an array extent.
+ const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(VD->getType());
+ if (!CAT)
+ return None;
+
+ // Array should be one-dimensional.
+ // TODO: Support multidimensional array.
+ if (isa<ConstantArrayType>(CAT->getElementType())) // is multidimensional
+ return None;
+
+ // Array's offset should be a concrete value.
+ // Return Unknown value if symbolic index presented.
+ // FIXME: We also need to take ElementRegions with symbolic
+ // indexes into account.
+ const auto OffsetVal = R->getIndex().getAs<nonloc::ConcreteInt>();
+ if (!OffsetVal.hasValue())
+ return UnknownVal();
+
+ // Check offset for being out of bounds.
+ // C++20 [expr.add] 7.6.6.4 (excerpt):
+ // If P points to an array element i of an array object x with n
+ // elements, where i < 0 or i > n, the behavior is undefined.
+ // Dereferencing is not allowed on the "one past the last
+ // element", when i == n.
+ // Example:
+ // const int arr[4] = {1, 2};
+ // const int *ptr = arr;
+ // int x0 = ptr[0]; // 1
+ // int x1 = ptr[1]; // 2
+ // int x2 = ptr[2]; // 0
+ // int x3 = ptr[3]; // 0
+ // int x4 = ptr[4]; // UB
+ // int x5 = ptr[-1]; // UB
+ const llvm::APSInt &OffsetInt = OffsetVal->getValue();
+ const auto Offset = static_cast<uint64_t>(OffsetInt.getExtValue());
+ // Use `getZExtValue` because array extent can not be negative.
+ const uint64_t Extent = CAT->getSize().getZExtValue();
+ // Check for `OffsetInt < 0` but NOT for `Offset < 0`, because `OffsetInt`
+ // CAN be negative, but `Offset` can NOT, because `Offset` is an uint64_t.
+ if (OffsetInt < 0 || Offset >= Extent)
+ return UndefinedVal();
+ // From here `Offset` is in the bounds.
+
+ // Handle InitListExpr.
+ if (const auto *ILE = dyn_cast<InitListExpr>(Init))
+ return getSValFromInitListExpr(ILE, Offset, R->getElementType());
+
+ // FIXME: Handle StringLiteral.
+
+ // FIXME: Handle CompoundLiteralExpr.
+
+ return None;
+}
+
+Optional<SVal>
+RegionStoreManager::getSValFromInitListExpr(const InitListExpr *ILE,
+ uint64_t Offset, QualType ElemT) {
+ assert(ILE && "InitListExpr should not be null");
+
+ // C++20 [expr.add] 9.4.17.5 (excerpt):
+ // i-th array element is value-initialized for each k < i ≤ n,
+ // where k is an expression-list size and n is an array extent.
+ if (Offset >= ILE->getNumInits())
+ return svalBuilder.makeZeroVal(ElemT);
+
+ // Return a constant value, if it is presented.
+ // FIXME: Support other SVals.
+ const Expr *E = ILE->getInit(Offset);
+ return svalBuilder.getConstantVal(E);
+}
+
SVal RegionStoreManager::getBindingForElement(RegionBindingsConstRef B,
const ElementRegion* R) {
// Check if the region has a binding.
@@ -1658,64 +1751,8 @@ SVal RegionStoreManager::getBindingForElement(RegionBindingsConstRef B,
return svalBuilder.makeIntVal(c, T);
}
} else if (const VarRegion *VR = dyn_cast<VarRegion>(superR)) {
- // Check if the containing array has an initialized value that we can trust.
- // We can trust a const value or a value of a global initializer in main().
- const VarDecl *VD = VR->getDecl();
- if (VD->getType().isConstQualified() ||
- R->getElementType().isConstQualified() ||
- (B.isMainAnalysis() && VD->hasGlobalStorage())) {
- if (const Expr *Init = VD->getAnyInitializer()) {
- if (const auto *InitList = dyn_cast<InitListExpr>(Init)) {
- // The array index has to be known.
- if (auto CI = R->getIndex().getAs<nonloc::ConcreteInt>()) {
- // If it is not an array, return Undef.
- QualType T = VD->getType();
- const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(T);
- if (!CAT)
- return UndefinedVal();
-
- // Support one-dimensional array.
- // C++20 [expr.add] 7.6.6.4 (excerpt):
- // If P points to an array element i of an array object x with n
- // elements, where i < 0 or i > n, the behavior is undefined.
- // Dereferencing is not allowed on the "one past the last
- // element", when i == n.
- // Example:
- // const int arr[4] = {1, 2};
- // const int *ptr = arr;
- // int x0 = ptr[0]; // 1
- // int x1 = ptr[1]; // 2
- // int x2 = ptr[2]; // 0
- // int x3 = ptr[3]; // 0
- // int x4 = ptr[4]; // UB
- // TODO: Support multidimensional array.
- if (!isa<ConstantArrayType>(CAT->getElementType())) {
- // One-dimensional array.
- const llvm::APSInt &Idx = CI->getValue();
- const auto I = static_cast<uint64_t>(Idx.getExtValue());
- // Use `getZExtValue` because array extent can not be negative.
- const uint64_t Extent = CAT->getSize().getZExtValue();
- // Check for `Idx < 0`, NOT for `I < 0`, because `Idx` CAN be
- // negative, but `I` can NOT.
- if (Idx < 0 || I >= Extent)
- return UndefinedVal();
-
- // C++20 [expr.add] 9.4.17.5 (excerpt):
- // i-th array element is value-initialized for each k < i ≤ n,
- // where k is an expression-list size and n is an array extent.
- if (I >= InitList->getNumInits())
- return svalBuilder.makeZeroVal(R->getElementType());
-
- // Return a constant value, if it is presented.
- // FIXME: Support other SVals.
- const Expr *E = InitList->getInit(I);
- if (Optional<SVal> V = svalBuilder.getConstantVal(E))
- return *V;
- }
- }
- }
- }
- }
+ if (Optional<SVal> V = getConstantValFromConstArrayInitializer(B, VR, R))
+ return *V;
}
// Check for loads from a code text region. For such loads, just give up.
diff --git a/clang/test/Analysis/initialization.c b/clang/test/Analysis/initialization.c
index a0899e678699b..7981465394eac 100644
--- a/clang/test/Analysis/initialization.c
+++ b/clang/test/Analysis/initialization.c
@@ -97,3 +97,9 @@ void glob_invalid_index4() {
// FIXME: Should warn {{garbage or undefined}}.
int res = glob_arr2[x][y]; // no-warning
}
+
+const int glob_arr_no_init[10];
+void glob_arr_index4() {
+ // FIXME: Should warn {{FALSE}}, since the array has a static storage.
+ clang_analyzer_eval(glob_arr_no_init[2]); // expected-warning{{UNKNOWN}}
+}
diff --git a/clang/test/Analysis/initialization.cpp b/clang/test/Analysis/initialization.cpp
index ad9acc880aec7..dc515170665a3 100644
--- a/clang/test/Analysis/initialization.cpp
+++ b/clang/test/Analysis/initialization.cpp
@@ -1,5 +1,7 @@
// RUN: %clang_cc1 -std=c++14 -triple i386-apple-darwin10 -analyze -analyzer-config eagerly-assume=false -analyzer-checker=core.uninitialized.Assign,core.builtin,debug.ExprInspection,core.uninitialized.UndefReturn -verify %s
+template <typename T>
+void clang_analyzer_dump(T x);
void clang_analyzer_eval(int);
struct S {
@@ -32,6 +34,10 @@ void glob_invalid_index1() {
auto x = ptr[idx]; // expected-warning{{garbage or undefined}}
}
+void glob_symbolic_index1(int idx) {
+ clang_analyzer_dump(glob_arr1[idx]); // expected-warning{{Unknown}}
+}
+
int const glob_arr2[4] = {1, 2};
void glob_ptr_index1() {
int const *ptr = glob_arr2;
@@ -128,3 +134,15 @@ void glob_invalid_index6() {
// FIXME: Should warn {{garbage or undefined}}.
auto x = ptr[idx]; // // no-warning
}
+
+extern const int glob_arr_no_init[10];
+void glob_array_index4() {
+ clang_analyzer_eval(glob_arr_no_init[2]); // expected-warning{{UNKNOWN}}
+}
+
+struct S2 {
+ static const int arr_no_init[10];
+};
+void struct_arr_index1() {
+ clang_analyzer_eval(S2::arr_no_init[2]); // expected-warning{{UNKNOWN}}
+}
More information about the cfe-commits
mailing list