Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -265,7 +265,8 @@ continue; } - if (T->isAnyPointerType() || T->isReferenceType() || T->isBlockPointerType()) { + if (T->isAnyPointerType() || T->isReferenceType() || + T->isBlockPointerType()) { if (isPointerOrReferenceUninit(FR, LocalChain)) ContainsUninitField = true; continue; Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp @@ -95,6 +95,12 @@ /// 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); + //===----------------------------------------------------------------------===// // Methods for FindUninitializedFields. //===----------------------------------------------------------------------===// @@ -126,67 +132,22 @@ return false; } - assert(V.getAs() && - "At this point V must be loc::MemRegionVal!"); - auto L = V.castAs(); - - // 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 (L.getRegion()->getSymbolicBase()) { - IsAnyFieldInitialized = true; - return false; - } - - DynamicTypeInfo DynTInfo = getDynamicTypeInfo(State, L.getRegion()); - if (!DynTInfo.isValid()) { + // 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); + if (!DerefInfo) { IsAnyFieldInitialized = true; return false; } - QualType DynT = DynTInfo.getType(); - - // 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()); - - if (isVoidPointer(DynT)) { - IsAnyFieldInitialized = true; - return false; - } - - // At this point the pointer itself is initialized and points to a valid - // location, we'll now check the pointee. - SVal DerefdV = State->getSVal(V.castAs(), DynT); - - // If DerefdV is still a pointer value, we'll dereference it again (e.g.: - // int** -> int*). - while (auto Tmp = DerefdV.getAs()) { - if (Tmp->getRegion()->getSymbolicBase()) { - IsAnyFieldInitialized = true; - return false; - } - - DynTInfo = getDynamicTypeInfo(State, Tmp->getRegion()); - if (!DynTInfo.isValid()) { - IsAnyFieldInitialized = true; - return false; - } - - DynT = DynTInfo.getType(); - if (isVoidPointer(DynT)) { - IsAnyFieldInitialized = true; - return false; - } - - DerefdV = State->getSVal(*Tmp, DynT); - } + V = std::get<0>(*DerefInfo); + QualType DynT = std::get<1>(*DerefInfo); + bool NeedsCastBack = std::get<2>(*DerefInfo); // If FR is a pointer pointing to a non-primitive type. if (Optional RecordV = - DerefdV.getAs()) { + V.getAs()) { const TypedValueRegion *R = RecordV->getRegion(); @@ -220,7 +181,7 @@ "At this point FR must either have a primitive dynamic type, or it " "must be a null, undefined, unknown or concrete pointer!"); - if (isPrimitiveUninit(DerefdV)) { + if (isPrimitiveUninit(V)) { if (NeedsCastBack) return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT))); return addFieldToUninits(LocalChain.add(LocField(FR))); @@ -242,3 +203,48 @@ } return false; } + +static llvm::Optional> +dereference(ProgramStateRef State, const FieldRegion *FR) { + + DynamicTypeInfo DynTInfo; + QualType DynT; + + // 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; + } + + DynTInfo = getDynamicTypeInfo(State, Tmp->getRegion()); + if (!DynTInfo.isValid()) { + return None; + } + + DynT = DynTInfo.getType(); + + if (isVoidPointer(DynT)) { + return None; + } + + V = State->getSVal(*Tmp, DynT); + } + + return std::make_tuple(V, DynT, NeedsCastBack); +} Index: cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp =================================================================== --- cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp +++ cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp @@ -194,15 +194,28 @@ CharPointerTest(); } -struct CyclicPointerTest { +struct CyclicPointerTest1 { int *ptr; - CyclicPointerTest() : ptr(reinterpret_cast(&ptr)) {} + CyclicPointerTest1() : ptr(reinterpret_cast(&ptr)) {} }; -void fCyclicPointerTest() { - CyclicPointerTest(); +void fCyclicPointerTest1() { + CyclicPointerTest1(); } +// TODO: Currently, the checker ends up in an infinite loop for the following +// test case. +/* +struct CyclicPointerTest2 { + int **pptr; + CyclicPointerTest2() : pptr(reinterpret_cast(&pptr)) {} +}; + +void fCyclicPointerTest2() { + CyclicPointerTest2(); +} +*/ + //===----------------------------------------------------------------------===// // Void pointer tests. //===----------------------------------------------------------------------===//