earnol created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp,
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware,
xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
earnol requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
What had been done:
- Passing a proper QualType for the initlist conversion
- Compatibility fixes for unit tests introduced.
- Various contingencies added to make sure the new code will be
invoked only where it is needed.
- New tests added to cover some cases in which new code will work
properly. The cases where it will fail were not added
intentionally as proper solution requires less localized
changes.
- Account for the case when source type info unavailable.
NB: Negative test cases were not added as problem was not fully
fixed and interlaced combinations of compound structures will
still cause crash, like union in array or array in struct
containing array with struct.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D144977
Files:
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/initializer-list-struct-analysis-crash.c
Index: clang/test/Analysis/initializer-list-struct-analysis-crash.c
===================================================================
--- /dev/null
+++ 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);
+}
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ 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 <optional>
@@ -427,9 +428,9 @@
const SubRegion *R);
std::optional<SVal>
getConstantValFromConstArrayInitializer(RegionBindingsConstRef B,
- const ElementRegion *R);
+ const ElementRegion *R, QualType T);
std::optional<SVal>
- getSValFromInitListExpr(const InitListExpr *ILE,
+ getSValFromInitListExpr(const InitListExpr *ILE, QualType VarT,
const SmallVector<uint64_t, 2> &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<ObjCIvarRegion>(R)) {
@@ -1723,7 +1725,7 @@
}
std::optional<SVal> 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<uint64_t, 2> ConcreteOffsets;
- if (std::optional<SVal> V = convertOffsetsFromSvalToUnsigneds(
- SValOffsets, Extents, ConcreteOffsets))
+ std::optional<SVal> 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<InitListExpr>(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<SVal> RegionStoreManager::getSValFromInitListExpr(
- const InitListExpr *ILE, const SmallVector<uint64_t, 2> &Offsets,
- QualType ElemT) {
+ const InitListExpr *ILE, QualType VarT,
+ const SmallVector<uint64_t, 2> &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<InitListExpr>(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<SVal> factory;
+ llvm::ImmutableList<SVal> 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<SVal> 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<SVal> 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<InitListExpr>(ElemExpr);
+ // we need this if here as there can be a function call
+ if (InListExpr) {
+ SmallVector<uint64_t, 2> 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<SVal> &V = B.getDirectBinding(R))
return *V;
@@ -1938,7 +2014,8 @@
return getSValFromStringLiteral(SL, Idx.getZExtValue(), T);
}
} else if (isa<ElementRegion, VarRegion>(superR)) {
- if (std::optional<SVal> V = getConstantValFromConstArrayInitializer(B, R))
+ if (std::optional<SVal> V =
+ getConstantValFromConstArrayInitializer(B, R, targetType))
return *V;
}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ 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<ElementRegion>(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<NonLoc>();
- 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<NonLoc>();
}
// Get the size of the array.
const auto *superReg = cast<SubRegion>(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;
+ }
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
