Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h =================================================================== --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h @@ -255,7 +255,12 @@ /// ease. This also helps ensuring that every special field type is handled /// correctly. inline bool isPrimitiveType(const QualType &T) { - return T->isBuiltinType() || T->isEnumeralType() || T->isMemberPointerType(); + return T->isBuiltinType() || T->isEnumeralType() || + T->isMemberPointerType() || T->isBlockPointerType(); +} + +inline bool isDereferencableType(const QualType &T) { + return T->isAnyPointerType() || T->isReferenceType(); } // Template method definitions. Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -292,8 +292,7 @@ continue; } - if (T->isAnyPointerType() || T->isReferenceType() || - T->isBlockPointerType()) { + if (isDereferencableType(T)) { if (isPointerOrReferenceUninit(FR, LocalChain)) ContainsUninitField = true; continue; Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp @@ -95,11 +95,13 @@ /// known, and thus FD can not be analyzed. static bool isVoidPointer(QualType T); -/// Dereferences \p V and returns the value and dynamic type of the pointee, as -/// well as wether \p FR needs to be casted back to that type. If for whatever -/// reason dereferencing fails, returns with None. -static llvm::Optional> -dereference(ProgramStateRef State, const FieldRegion *FR); +using DereferenceInfo = std::pair; + +/// Dereferences \p FR and returns with the pointee's region, and whether it +/// needs to be casted back to it's location type. If for whatever reason +/// dereferencing fails, returns with None. +static llvm::Optional dereference(ProgramStateRef State, + const FieldRegion *FR); //===----------------------------------------------------------------------===// // Methods for FindUninitializedFields. @@ -110,10 +112,8 @@ bool FindUninitializedFields::isPointerOrReferenceUninit( const FieldRegion *FR, FieldChainInfo LocalChain) { - assert((FR->getDecl()->getType()->isAnyPointerType() || - FR->getDecl()->getType()->isReferenceType() || - FR->getDecl()->getType()->isBlockPointerType()) && - "This method only checks pointer/reference objects!"); + assert(isDereferencableType(FR->getDecl()->getType()) && + "This method only checks dereferencable objects!"); SVal V = State->getSVal(FR); @@ -123,6 +123,8 @@ } if (V.isUndef()) { + assert(!FR->getDecl()->getType()->isReferenceType() && + "References must be initialized!"); return addFieldToUninits( LocalChain.add(LocField(FR, /*IsDereferenced*/ false))); } @@ -134,54 +136,47 @@ // At this point the pointer itself is initialized and points to a valid // location, we'll now check the pointee. - llvm::Optional> DerefInfo = - dereference(State, FR); + llvm::Optional DerefInfo = dereference(State, FR); if (!DerefInfo) { IsAnyFieldInitialized = true; return false; } - V = std::get<0>(*DerefInfo); - QualType DynT = std::get<1>(*DerefInfo); - bool NeedsCastBack = std::get<2>(*DerefInfo); + const TypedValueRegion *R = DerefInfo->first; + const bool NeedsCastBack = DerefInfo->second; - // If FR is a pointer pointing to a non-primitive type. - if (Optional RecordV = - V.getAs()) { + QualType DynT = R->getLocationType(); + QualType PointeeT = DynT->getPointeeType(); - const TypedValueRegion *R = RecordV->getRegion(); + if (PointeeT->isStructureOrClassType()) { + if (NeedsCastBack) + return isNonUnionUninit(R, LocalChain.add(NeedsCastLocField(FR, DynT))); + return isNonUnionUninit(R, LocalChain.add(LocField(FR))); + } - if (DynT->getPointeeType()->isStructureOrClassType()) { + if (PointeeT->isUnionType()) { + if (isUnionUninit(R)) { if (NeedsCastBack) - return isNonUnionUninit(R, LocalChain.add(NeedsCastLocField(FR, DynT))); - return isNonUnionUninit(R, LocalChain.add(LocField(FR))); - } - - if (DynT->getPointeeType()->isUnionType()) { - if (isUnionUninit(R)) { - if (NeedsCastBack) - return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT))); - return addFieldToUninits(LocalChain.add(LocField(FR))); - } else { - IsAnyFieldInitialized = true; - return false; - } - } - - if (DynT->getPointeeType()->isArrayType()) { + return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT))); + return addFieldToUninits(LocalChain.add(LocField(FR))); + } else { IsAnyFieldInitialized = true; return false; } + } - llvm_unreachable("All cases are handled!"); + if (PointeeT->isArrayType()) { + IsAnyFieldInitialized = true; + return false; } - assert((isPrimitiveType(DynT->getPointeeType()) || DynT->isAnyPointerType() || - DynT->isReferenceType()) && + assert((isPrimitiveType(PointeeT) || isDereferencableType(PointeeT)) && "At this point FR must either have a primitive dynamic type, or it " "must be a null, undefined, unknown or concrete pointer!"); - if (isPrimitiveUninit(V)) { + SVal PointeeV = State->getSVal(R); + + if (isPrimitiveUninit(PointeeV)) { if (NeedsCastBack) return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT))); return addFieldToUninits(LocalChain.add(LocField(FR))); @@ -204,47 +199,46 @@ return false; } -static llvm::Optional> -dereference(ProgramStateRef State, const FieldRegion *FR) { +static llvm::Optional dereference(ProgramStateRef State, + const FieldRegion *FR) { - DynamicTypeInfo DynTInfo; - QualType DynT; + llvm::SmallSet VisitedRegions; // If the static type of the field is a void pointer, we need to cast it back // to the dynamic type before dereferencing. bool NeedsCastBack = isVoidPointer(FR->getDecl()->getType()); SVal V = State->getSVal(FR); - assert(V.getAs() && "V must be loc::MemRegionVal!"); - - // If V is multiple pointer value, we'll dereference it again (e.g.: int** -> - // int*). - // TODO: Dereference according to the dynamic type to avoid infinite loop for - // these kind of fields: - // int **ptr = reinterpret_cast(&ptr); - while (auto Tmp = V.getAs()) { - // We can't reason about symbolic regions, assume its initialized. - // Note that this also avoids a potential infinite recursion, because - // constructors for list-like classes are checked without being called, and - // the Static Analyzer will construct a symbolic region for Node *next; or - // similar code snippets. - if (Tmp->getRegion()->getSymbolicBase()) { - return None; - } + assert(V.getAsRegion() && "V must have an underlying region!"); - DynTInfo = getDynamicTypeInfo(State, Tmp->getRegion()); - if (!DynTInfo.isValid()) { - return None; - } + // The region we'd like to acquire. + const auto *R = V.getAsRegion()->getAs(); + if (!R) + return None; + + VisitedRegions.insert(R); - DynT = DynTInfo.getType(); + // We acquire the dynamic type of R, + QualType DynT = R->getLocationType(); - if (isVoidPointer(DynT)) { + while (const MemRegion *Tmp = State->getSVal(R, DynT).getAsRegion()) { + + R = Tmp->getAs(); + + if (!R) + return None; + + // We found a cyclic pointer, like int *ptr = (int *)&ptr. + // TODO: Report these fields too. + if (!VisitedRegions.insert(R).second) return None; - } - V = State->getSVal(*Tmp, DynT); + DynT = R->getLocationType(); + // In order to ensure that this loop terminates, we're also checking the + // dynamic type of R, since type hierarchy is finite. + if (isDereferencableType(DynT->getPointeeType())) + break; } - return std::make_tuple(V, DynT, NeedsCastBack); + return std::make_pair(R, NeedsCastBack); } Index: test/Analysis/cxx-uninitialized-object-ptr-ref.cpp =================================================================== --- test/Analysis/cxx-uninitialized-object-ptr-ref.cpp +++ test/Analysis/cxx-uninitialized-object-ptr-ref.cpp @@ -46,6 +46,50 @@ } //===----------------------------------------------------------------------===// +// Alloca tests. +//===----------------------------------------------------------------------===// + +struct UntypedAllocaTest { + void *allocaPtr; + int dontGetFilteredByNonPedanticMode = 0; + + UntypedAllocaTest() : allocaPtr(__builtin_alloca(sizeof(int))) { + // All good! + } +}; + +void fUntypedAllocaTest() { + UntypedAllocaTest(); +} + +struct TypedAllocaTest1 { + int *allocaPtr; // expected-note{{uninitialized pointee 'this->allocaPtr'}} + int dontGetFilteredByNonPedanticMode = 0; + + TypedAllocaTest1() // expected-warning{{1 uninitialized field}} + : allocaPtr(static_cast(__builtin_alloca(sizeof(int)))) {} +}; + +void fTypedAllocaTest1() { + TypedAllocaTest1(); +} + +struct TypedAllocaTest2 { + int *allocaPtr; + int dontGetFilteredByNonPedanticMode = 0; + + TypedAllocaTest2() + : allocaPtr(static_cast(__builtin_alloca(sizeof(int)))) { + *allocaPtr = 55555; + // All good! + } +}; + +void fTypedAllocaTest2() { + TypedAllocaTest2(); +} + +//===----------------------------------------------------------------------===// // Heap pointer tests. //===----------------------------------------------------------------------===// @@ -203,18 +247,14 @@ CyclicPointerTest1(); } -// TODO: Currently, the checker ends up in an infinite loop for the following -// test case. -/* struct CyclicPointerTest2 { - int **pptr; + int **pptr; // no-crash CyclicPointerTest2() : pptr(reinterpret_cast(&pptr)) {} }; void fCyclicPointerTest2() { CyclicPointerTest2(); } -*/ //===----------------------------------------------------------------------===// // Void pointer tests. @@ -645,6 +685,15 @@ CyclicList(&n1, int()); } +struct RingListTest { + RingListTest *next; // no-crash + RingListTest() : next(this) {} +}; + +void fRingListTest() { + RingListTest(); +} + //===----------------------------------------------------------------------===// // Tests for classes containing references. //===----------------------------------------------------------------------===// Index: test/Analysis/objcpp-uninitialized-object.mm =================================================================== --- test/Analysis/objcpp-uninitialized-object.mm +++ test/Analysis/objcpp-uninitialized-object.mm @@ -4,7 +4,7 @@ struct StructWithBlock { int a; - myBlock z; // expected-note{{uninitialized pointer 'this->z'}} + myBlock z; // expected-note{{uninitialized field 'this->z'}} StructWithBlock() : a(0), z(^{}) {}