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 - getConstantValFromConstArrayInitializer(RegionBindingsConstRef B, - QualType ElemT, const VarRegion *VR, - const llvm::APSInt &Idx); + Optional getConstantValFromConstArrayInitializer( + RegionBindingsConstRef B, const VarRegion *VR, const llvm::APSInt &Idx, + QualType ElemT); + SVal getSValFromStringLiteralByIndex(const StringLiteral *SL, + const llvm::APSInt &Idx, QualType ElemT); + Optional getSValFromInitListExprByIndex(const InitListExpr *ILE, + const llvm::APSInt &Idx, + QualType ElemT); public: // Part of public interface to class. @@ -1628,38 +1632,69 @@ return Result; } + Optional 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(Init); - if (!InitList) + if (!Init) return None; - // Choose a correct function depending on index signedness. + + // Handle StringLiteral. + if (const auto *SL = dyn_cast(Init)) + return getSValFromStringLiteralByIndex(SL, Idx, ElemT); + + // Handle InitListExpr. + if (const auto *ILE = dyn_cast(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(I) >= SL->getLength()) ? 0 : SL->getCodeUnit(I); + return svalBuilder.makeIntVal(Code, ElemT); +} + +Optional 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(I)); + ? ILE->getExprForConstArrayByRawIndex(I) + : ILE->getExprForConstArrayByRawIndex(static_cast(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 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 CI = Idx.getAs()) { - 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(superR) || isa(superR)) { const VarRegion *VR = nullptr; @@ -1718,7 +1742,7 @@ } if (VR) if (Optional 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(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(Init)) + // FIXME: Ignore `CompoundLiteralExpr` as well when + // `getConstantValFromConstArrayInitializer` supports it. + if (Init && (isa(Init) || isa(Init))) return B; } } 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}} +}