ASDenysPetrov updated this revision to Diff 363713.
ASDenysPetrov added a comment.
Added tests.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107339/new/
https://reviews.llvm.org/D107339
Files:
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/initialization.cpp
Index: clang/test/Analysis/initialization.cpp
===================================================================
--- clang/test/Analysis/initialization.cpp
+++ clang/test/Analysis/initialization.cpp
@@ -295,3 +295,68 @@
int x = 42;
int res = local_arr[x]; // expected-warning{{garbage or undefined}}
}
+
+const char glob_str_arr1[8] = "1234";
+void glob_str_arr_index1() {
+ clang_analyzer_eval(glob_str_arr1[0] == '1'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(glob_str_arr1[1] == '2'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(glob_str_arr1[2] == '3'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(glob_str_arr1[3] == '4'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(glob_str_arr1[4] == '\0'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(glob_str_arr1[5] == '\0'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(glob_str_arr1[6] == '\0'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(glob_str_arr1[7] == '\0'); // expected-warning{{TRUE}}
+}
+
+void glob_str_arr_out_of_bound_index1() {
+ int x = -42;
+ int res = glob_str_arr1[x]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_str_arr_out_of_bound_index2() {
+ int x = 42;
+ // FIXME: This should warn about "garbage or undefined".
+ int res = glob_str_arr1[x]; // no-warning
+ // FIXME-cont: `res` is `\0` but should be undefined.
+ clang_analyzer_eval(res == '\0'); // expected-warning{{TRUE}}
+}
+
+void local_str_arr_index1() {
+ const char str_arr[8] = "1234";
+ clang_analyzer_eval(str_arr[0] == '1'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(str_arr[1] == '2'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(str_arr[2] == '3'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(str_arr[3] == '4'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(str_arr[4] == '\0'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(str_arr[5] == '\0'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(str_arr[6] == '\0'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(str_arr[7] == '\0'); // expected-warning{{TRUE}}
+}
+
+void local_str_arr_out_of_bound_index3() {
+ const char str_arr[8] = "1234";
+ int x = -42;
+ int res = str_arr[x]; // expected-warning{{garbage or undefined}}
+}
+
+void local_str_arr_out_of_bound_index4() {
+ const char str_arr[8] = "1234";
+ int x = 42;
+ // FIXME: This should warn about "garbage or undefined".
+ int res = str_arr[x]; // no-warning
+ // FIXME-cont: `res` is `\0` but should be undefined.
+ clang_analyzer_eval(res == '\0'); // expected-warning{{TRUE}}
+}
+
+void ptr_local_str_arr_index1() {
+ const char str_arr[8] = "1234";
+ const char *ptr = str_arr;
+ clang_analyzer_eval(ptr[0] == '1'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(ptr[1] == '2'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(ptr[2] == '3'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(ptr[3] == '4'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(ptr[4] == '\0'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(ptr[5] == '\0'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(ptr[6] == '\0'); // expected-warning{{TRUE}}
+ clang_analyzer_eval(ptr[7] == '\0'); // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -437,10 +437,14 @@
RegionBindingsRef removeSubRegionBindings(RegionBindingsConstRef B,
const SubRegion *R);
- Optional<SVal>
- getConstantValFromConstArrayInitializer(RegionBindingsConstRef B,
- QualType ElemT, const VarRegion *VR,
- const llvm::APSInt &Idx);
+ Optional<SVal> getConstantValFromConstArrayInitializer(
+ RegionBindingsConstRef B, const VarRegion *VR, const llvm::APSInt &Idx,
+ QualType ElemT);
+ SVal getSValFromStringLiteralByIndex(const StringLiteral *SL,
+ const llvm::APSInt &Idx, QualType ElemT);
+ Optional<SVal> getSValFromInitListExprByIndex(const InitListExpr *ILE,
+ const llvm::APSInt &Idx,
+ QualType ElemT);
public: // Part of public interface to class.
@@ -1628,38 +1632,69 @@
return Result;
}
+
Optional<SVal> RegionStoreManager::getConstantValFromConstArrayInitializer(
- RegionBindingsConstRef B, QualType ElemT, const VarRegion *VR,
- const llvm::APSInt &Idx) {
+ RegionBindingsConstRef B, const VarRegion *VR, const llvm::APSInt &Idx,
+ QualType ElemT) {
// Array should constant a const value or a value of a global initializer in
// main().
const VarDecl *VD = VR->getDecl();
if (!VD->getType().isConstQualified() && !ElemT.isConstQualified() &&
(!B.isMainAnalysis() || !VD->hasGlobalStorage()))
return None;
+
// Array should have an initialization list.
const Expr *Init = VD->getAnyInitializer();
- // FIXME: Support StringLiteral and CompoundLiteralExpr as well.
- const auto *InitList = dyn_cast_or_null<InitListExpr>(Init);
- if (!InitList)
+ if (!Init)
return None;
- // Choose a correct function depending on index signedness.
+
+ // Handle StringLiteral.
+ if (const auto *SL = dyn_cast<StringLiteral>(Init))
+ return getSValFromStringLiteralByIndex(SL, Idx, ElemT);
+
+ // Handle InitListExpr.
+ if (const auto *ILE = dyn_cast<InitListExpr>(Init))
+ return getSValFromInitListExprByIndex(ILE, Idx, ElemT);
+
+ // FIXME: Handle CompoundLiteralExpr.
+
+ return None;
+}
+
+SVal RegionStoreManager::getSValFromStringLiteralByIndex(
+ const StringLiteral *SL, const llvm::APSInt &Idx, QualType ElemT) {
+ assert(SL && "StringLiteral should not be null");
+ // If index is out of bounds, return Undef.
+ const int64_t I = Idx.getExtValue();
+ if (Idx.isSigned() && I < 0)
+ return UndefinedVal();
+ // Technically, only i == length is guaranteed to be null.
+ // However, such overflows should be caught before reaching this point;
+ // the only time such an access would be made is if a string literal was
+ // used to initialize a larger array.
+ uint32_t Code =
+ (static_cast<uint64_t>(I) >= SL->getLength()) ? 0 : SL->getCodeUnit(I);
+ return svalBuilder.makeIntVal(Code, ElemT);
+}
+
+Optional<SVal> RegionStoreManager::getSValFromInitListExprByIndex(
+ const InitListExpr *ILE, const llvm::APSInt &Idx, QualType ElemT) {
+ assert(ILE && "InitListExpr should not be null");
const int64_t I = Idx.getExtValue();
+ // Choose a correct function depending on index signedness.
const Expr *E =
Idx.isSigned()
- ? InitList->getExprForConstArrayByRawIndex(I)
- : InitList->getExprForConstArrayByRawIndex(static_cast<uint64_t>(I));
+ ? ILE->getExprForConstArrayByRawIndex(I)
+ : ILE->getExprForConstArrayByRawIndex(static_cast<uint64_t>(I));
// If E is null, then the index is out of bounds. Return Undef.
if (!E)
return UndefinedVal();
- // If E is the same as InitList, then there is no value specified
+ // If E is the same as ILE, then there is no value specified
// in the list and we shall return a zero value.
- if (E == InitList)
+ if (E == ILE)
return svalBuilder.makeZeroVal(ElemT);
// Return a constant value.
- if (Optional<SVal> V = svalBuilder.getConstantVal(E))
- return *V;
- return None;
+ return svalBuilder.getConstantVal(E);
}
SVal RegionStoreManager::getBindingForElement(RegionBindingsConstRef B,
@@ -1678,21 +1713,10 @@
if (!Ctx.hasSameUnqualifiedType(T, R->getElementType()))
return UnknownVal();
- const StringLiteral *Str = StrR->getStringLiteral();
SVal Idx = R->getIndex();
if (Optional<nonloc::ConcreteInt> CI = Idx.getAs<nonloc::ConcreteInt>()) {
- int64_t i = CI->getValue().getSExtValue();
- // Abort on string underrun. This can be possible by arbitrary
- // clients of getBindingForElement().
- if (i < 0)
- return UndefinedVal();
- int64_t length = Str->getLength();
- // Technically, only i == length is guaranteed to be null.
- // However, such overflows should be caught before reaching this point;
- // the only time such an access would be made is if a string literal was
- // used to initialize a larger array.
- char c = (i >= length) ? '\0' : Str->getCodeUnit(i);
- return svalBuilder.makeIntVal(c, T);
+ const StringLiteral *Str = StrR->getStringLiteral();
+ return getSValFromStringLiteralByIndex(Str, CI->getValue(), T);
}
} else if (isa<ElementRegion>(superR) || isa<VarRegion>(superR)) {
const VarRegion *VR = nullptr;
@@ -1718,7 +1742,7 @@
}
if (VR)
if (Optional<SVal> V = getConstantValFromConstArrayInitializer(
- B, R->getElementType(), VR, Idx))
+ B, VR, Idx, R->getElementType()))
return *V;
}
@@ -2218,10 +2242,11 @@
RegionBindingsRef RegionStoreManager::bindArray(RegionBindingsConstRef B,
const TypedValueRegion *R,
SVal Init) {
- // Ignore binding `InitListExpr` to arrays of const type,
- // since we can directly retrieve values from initializer using
+ // Ignore binding `InitListExpr` and `StringLiteral` to arrays of const type,
+ // since we can directly retrieve values from initializers using
// `getConstantValFromConstArrayInitializer`.For example:
// const int arr[42] = { 1, 2, 3 };
+ // const char arr[42] = "123";
// The init values of this array will never change, so we don't have to
// store them additionally in the RegionStore.
if (const auto *VR = dyn_cast<VarRegion>(R)) {
@@ -2229,9 +2254,9 @@
// Ignore only arrays which values can't change.
if (VD->getType().isConstQualified()) {
const Expr *Init = VD->getAnyInitializer();
- // FIXME: Ignore `StringLiteral` and `CompoundLiteralExpr` as well when
- // `getConstantValFromConstArrayInitializer` supports them.
- if (Init && isa<InitListExpr>(Init))
+ // FIXME: Ignore `CompoundLiteralExpr` as well when
+ // `getConstantValFromConstArrayInitializer` supports it.
+ if (Init && (isa<InitListExpr>(Init) || isa<StringLiteral>(Init)))
return B;
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
