Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h =================================================================== --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h @@ -258,6 +258,11 @@ return T->isBuiltinType() || T->isEnumeralType() || T->isMemberPointerType(); } +inline bool isLocType(const QualType &T) { + return T->isAnyPointerType() || T->isReferenceType() || + T->isBlockPointerType(); +} + // Template method definitions. template 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 (isLocType(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,9 +112,7 @@ bool FindUninitializedFields::isPointerOrReferenceUninit( const FieldRegion *FR, FieldChainInfo LocalChain) { - assert((FR->getDecl()->getType()->isAnyPointerType() || - FR->getDecl()->getType()->isReferenceType() || - FR->getDecl()->getType()->isBlockPointerType()) && + assert(isLocType(FR->getDecl()->getType()) && "This method only checks pointer/reference 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,46 @@ // 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) || isLocType(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)) { + const SVal &PointeeV = State->getSVal(loc::MemRegionVal(R)); + if (isPrimitiveUninit(PointeeV)) { if (NeedsCastBack) return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT))); return addFieldToUninits(LocalChain.add(LocField(FR))); @@ -204,11 +198,8 @@ return false; } -static llvm::Optional> -dereference(ProgramStateRef State, const FieldRegion *FR) { - - DynamicTypeInfo DynTInfo; - QualType DynT; +static llvm::Optional dereference(ProgramStateRef State, + const FieldRegion *FR) { // If the static type of the field is a void pointer, we need to cast it back // to the dynamic type before dereferencing. @@ -216,35 +207,45 @@ SVal V = State->getSVal(FR); assert(V.getAs() && "V must be loc::MemRegionVal!"); + 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; + } + + // The region we'd like to acquire. + const auto *R = Tmp->getRegionAs(); + assert(R); + + DynamicTypeInfo DynTInfo = getDynamicTypeInfo(State, R); + if (!DynTInfo.isValid()) { + return None; + } + + QualType DynT = DynTInfo.getType(); // 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; - } + Tmp = State->getSVal(*Tmp, DynT).getAs(); - DynTInfo = getDynamicTypeInfo(State, Tmp->getRegion()); - if (!DynTInfo.isValid()) { - return None; - } - - DynT = DynTInfo.getType(); + // In order to ensure that this loop terminates, we're also checking the + // dynamic type of FR, since type hierarchy is finite. + while (Tmp && isLocType(DynT->getPointeeType())) { - if (isVoidPointer(DynT)) { + if (Tmp->getRegion()->getSymbolicBase()) return None; - } - V = State->getSVal(*Tmp, DynT); + DynT = DynT->getPointeeType(); + R = Tmp->getRegionAs(); + assert(R && "R must be a TypedValueRegion!"); + + Tmp = State->getSVal(*Tmp, DynT).getAs(); } - 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 @@ -203,18 +203,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.