diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -360,23 +360,23 @@ // FIXME: This was originally copied from ArrayBoundChecker.cpp. Refactor? ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C, - ProgramStateRef state, + ProgramStateRef State, AnyArgExpr Buffer, SVal Element, AccessKind Access, CharKind CK) const { // If a previous check has failed, propagate the failure. - if (!state) + if (!State) return nullptr; // Check for out of bound array element access. const MemRegion *R = Element.getAsRegion(); if (!R) - return state; + return State; const auto *ER = dyn_cast(R); if (!ER) - return state; + return State; SValBuilder &svalBuilder = C.getSValBuilder(); ASTContext &Ctx = svalBuilder.getContext(); @@ -386,10 +386,10 @@ if (CK == CharKind::Regular) { if (ER->getValueType() != Ctx.CharTy) - return state; + return State; } else { if (ER->getValueType() != Ctx.WideCharTy) - return state; + return State; QualType SizeTy = Ctx.getSizeType(); NonLoc WideSize = @@ -397,19 +397,19 @@ .makeIntVal(Ctx.getTypeSizeInChars(Ctx.WideCharTy).getQuantity(), SizeTy) .castAs(); - SVal Offset = svalBuilder.evalBinOpNN(state, BO_Mul, Idx, WideSize, SizeTy); + SVal Offset = svalBuilder.evalBinOpNN(State, BO_Mul, Idx, WideSize, SizeTy); if (Offset.isUnknown()) - return state; + return State; Idx = Offset.castAs(); } // Get the size of the array. const auto *superReg = cast(ER->getSuperRegion()); DefinedOrUnknownSVal Size = - getDynamicExtent(state, superReg, C.getSValBuilder()); + getDynamicExtent(State, superReg, C.getSValBuilder()); ProgramStateRef StInBound, StOutBound; - std::tie(StInBound, StOutBound) = state->assumeInBoundDual(Idx, Size); + std::tie(StInBound, StOutBound) = State->assumeInBoundDual(Idx, Size); if (StOutBound && !StInBound) { // These checks are either enabled by the CString out-of-bounds checker // explicitly or implicitly by the Malloc checker. @@ -426,10 +426,26 @@ // Ensure that we wouldn't read uninitialized value. if (Access == AccessKind::read) { - if (Filter.CheckCStringUninitializedRead && - StInBound->getSVal(ER).isUndef()) { - emitUninitializedReadBug(C, StInBound, Buffer.Expression); - return nullptr; + if (Filter.CheckCStringUninitializedRead) { + // get the right type for future construction + const LocationContext *LCtx = C.getLocationContext(); + SVal SrcVal = State->getSVal(Buffer.Expression, LCtx); + QualType SourceValType = SrcVal.getType(C.getASTContext()); + // attempt to use type "as is" + QualType ConstructType = SourceValType; + // if it is a pointer ==> obtain the true type + if (SourceValType->isPointerType()) { + ConstructType = SourceValType->getPointeeType(); + } + // if the type unsuitable for analysis treat it as we have no + // information about the type + if (ConstructType->isVoidType()) { + ConstructType = QualType(); + } + if (StInBound->getSVal(ER, ConstructType).isUndef()) { + emitUninitializedReadBug(C, StInBound, Buffer.Expression); + return nullptr; + } } } diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -27,6 +27,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" +#include "llvm/ADT/ImmutableList.h" #include "llvm/ADT/ImmutableMap.h" #include "llvm/Support/raw_ostream.h" #include @@ -427,9 +428,9 @@ const SubRegion *R); std::optional getConstantValFromConstArrayInitializer(RegionBindingsConstRef B, - const ElementRegion *R); + const ElementRegion *R, QualType T); std::optional - getSValFromInitListExpr(const InitListExpr *ILE, + getSValFromInitListExpr(const InitListExpr *ILE, QualType VarT, const SmallVector &ConcreteOffsets, QualType ElemT); SVal getSValFromStringLiteral(const StringLiteral *SL, uint64_t Offset, @@ -560,7 +561,8 @@ SVal getBinding(RegionBindingsConstRef B, Loc L, QualType T = QualType()); - SVal getBindingForElement(RegionBindingsConstRef B, const ElementRegion *R); + SVal getBindingForElement(RegionBindingsConstRef B, const ElementRegion *R, + QualType T = QualType()); SVal getBindingForField(RegionBindingsConstRef B, const FieldRegion *R); @@ -1476,7 +1478,7 @@ // more intelligently. For example, an 'element' can encompass multiple // bound regions (e.g., several bound bytes), or could be a subset of // a larger value. - return svalBuilder.evalCast(getBindingForElement(B, ER), T, QualType{}); + return svalBuilder.evalCast(getBindingForElement(B, ER, T), T, QualType{}); } if (const ObjCIvarRegion *IVR = dyn_cast(R)) { @@ -1723,7 +1725,7 @@ } std::optional RegionStoreManager::getConstantValFromConstArrayInitializer( - RegionBindingsConstRef B, const ElementRegion *R) { + RegionBindingsConstRef B, const ElementRegion *R, QualType targetType) { assert(R && "ElementRegion should not be null"); // Treat an n-dimensional array. @@ -1779,15 +1781,17 @@ return std::nullopt; SmallVector ConcreteOffsets; - if (std::optional V = convertOffsetsFromSvalToUnsigneds( - SValOffsets, Extents, ConcreteOffsets)) + std::optional V = + convertOffsetsFromSvalToUnsigneds(SValOffsets, Extents, ConcreteOffsets); + if (V && + (!targetType->isStructureOrClassType() && !targetType->isUnionType())) return *V; - // Handle InitListExpr. // Example: // const char arr[4][2] = { { 1, 2 }, { 3 }, 4, 5 }; if (const auto *ILE = dyn_cast(Init)) - return getSValFromInitListExpr(ILE, ConcreteOffsets, R->getElementType()); + return getSValFromInitListExpr(ILE, targetType, ConcreteOffsets, + R->getElementType()); // Handle StringLiteral. // Example: @@ -1819,8 +1823,8 @@ /// for the given initialization list. Otherwise SVal can be an equivalent to 0 /// or lead to assertion. std::optional RegionStoreManager::getSValFromInitListExpr( - const InitListExpr *ILE, const SmallVector &Offsets, - QualType ElemT) { + const InitListExpr *ILE, QualType VarT, + const SmallVector &Offsets, QualType ElemT) { assert(ILE && "InitListExpr should not be null"); for (uint64_t Offset : Offsets) { @@ -1840,12 +1844,83 @@ return svalBuilder.makeZeroVal(ElemT); const Expr *E = ILE->getInit(Offset); + // go to next if there is no expression + if (!E) + continue; + // obtain init list expression if it is really one const auto *IL = dyn_cast(E); if (!IL) // Return a constant value, if it is presented. - // FIXME: Support other SVals. return svalBuilder.getConstantVal(E); - + else { + // if value a list to process it getting individual values + llvm::ImmutableListFactory factory; + llvm::ImmutableList InitList = factory.getEmptyList(); + const Expr *ElemExpr = nullptr; + // HACK: This if is required for init lists like {[0][0]=1,[1][1]=2} + // However it is a HACK as it will fail on this kind of init list of + // structures + if (VarT->isBuiltinType()) { + // Go to the nested initializer list. + ILE = IL; + continue; + } + // if we are here we have struct or union? + if (!VarT->isStructureType()) { + // TODO: support other options like unions or arrays or VLAs + // but still attempt to deduce what we can + std::optional ReplacementConst = svalBuilder.getConstantVal(E); + if (ReplacementConst) { + return ReplacementConst; + } + // + // we will return UnknownVal for something we are not properly + // supporting + return UnknownVal(); + } + // obtain type iterator + RecordDecl *RecDecl = VarT->getAsRecordDecl(); + RecordDecl::field_iterator RecIter = RecDecl->field_begin(); + // iterate over all discovered elements + for (size_t Idx = 0; Idx < IL->getNumInits(); Idx++) { + // having extra inits for structure is UB, so... return UB + if (RecIter == RecDecl->field_end()) { + return UndefinedVal(); + } + ElemExpr = IL->getInit(Idx); + std::optional ConstVal = svalBuilder.getConstantVal(ElemExpr); + // if there is no value create a zero one + if (!ConstVal.has_value()) { + // we can create a substitution with + // constVal = svalBuilder.makeZeroVal(elemExpr->getType()); + if (InitList.isEmpty()) { + // or go deeper if no value and extra parsing needed + ILE = IL; + } else { + // do a recursive call here and fill constVal with the result + const InitListExpr *InListExpr = dyn_cast(ElemExpr); + // we need this if here as there can be a function call + if (InListExpr) { + SmallVector InternalOffsetsArr; + InternalOffsetsArr.append(1, Offset); + QualType RecElemTrgType = (*RecIter)->getType(); + ConstVal = this->getSValFromInitListExpr( + InListExpr, RecElemTrgType, InternalOffsetsArr, ElemT); + } else { + // and thus we have no idea without properly tracking + return UndefinedVal(); + } + } + RecIter++; + continue; + } + InitList = factory.add(ConstVal.value(), InitList); + } + if (ElemExpr) { + return svalBuilder.makeCompoundVal(VarT, InitList); + } + RecIter++; + } // Go to the nested initializer list. ILE = IL; } @@ -1915,7 +1990,8 @@ } SVal RegionStoreManager::getBindingForElement(RegionBindingsConstRef B, - const ElementRegion* R) { + const ElementRegion *R, + QualType targetType) { // Check if the region has a binding. if (const std::optional &V = B.getDirectBinding(R)) return *V; @@ -1938,7 +2014,8 @@ return getSValFromStringLiteral(SL, Idx.getZExtValue(), T); } } else if (isa(superR)) { - if (std::optional V = getConstantValFromConstArrayInitializer(B, R)) + if (std::optional V = + getConstantValFromConstArrayInitializer(B, R, targetType)) return *V; } diff --git a/clang/test/Analysis/initializer-list-struct-analysis-crash.c b/clang/test/Analysis/initializer-list-struct-analysis-crash.c new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/initializer-list-struct-analysis-crash.c @@ -0,0 +1,30 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-checker=alpha.unix.cstring.OutOfBounds,alpha.unix.cstring.UninitializedRead -verify %s +// expected-no-diagnostics + + +// Just don't crash. + +typedef __typeof(sizeof(int)) size_t; +void *memcpy(void *to, void const *from, size_t count); + +// test 1 + +typedef struct _shortStruct { + short a; + short b; +} shortStruct, *p_shortStruct; +const shortStruct Info[2] = { { 0, 1, }, { 2, 3, },}; +void clang_analyzer_dump(short); +static void xxx_shortStruct(void) +{ + shortStruct local_info[2]; + memcpy(local_info, Info, sizeof(struct _shortStruct) * 2); +} + +// test 2 +const short InfoArray[2][2] = { { 0, 1, }, { 2, 3, },}; +static void xxxArray(void) +{ + short local_info[2][2]; + memcpy(local_info, InfoArray, sizeof(short) * 4); +}