Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp @@ -30,7 +30,7 @@ #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include +#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h" using namespace clang; using namespace clang::ento; @@ -210,10 +210,10 @@ /// constructor. bool isCalledByConstructor(const CheckerContext &Context); -/// Returns whether FD can be (transitively) dereferenced to a void pointer type +/// Returns whether T can be (transitively) dereferenced to a void pointer type /// (void*, void**, ...). The type of the region behind a void pointer isn't /// known, and thus FD can not be analyzed. -bool isVoidPointer(const FieldDecl *FD); +bool isVoidPointer(QualType T); /// Returns true if T is a primitive type. We defined this type so that for /// objects that we'd only like analyze as much as checking whether their @@ -452,7 +452,7 @@ SVal V = State->getSVal(FR); - if (V.isUnknown() || V.isZeroConstant()) { + if (V.isUnknown() || V.getAs()) { IsAnyFieldInitialized = true; return false; } @@ -461,48 +461,70 @@ return addFieldToUninits({LocalChain, FR}); } - const FieldDecl *FD = FR->getDecl(); + assert(V.getAs() && + "At this point V must be loc::MemRegionVal!"); + auto L = V.castAs(); - // TODO: The dynamic type of a void pointer may be retrieved with - // `getDynamicTypeInfo`. - if (isVoidPointer(FD)) { + // 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; } - assert(V.getAs() && "V should be Loc at this point!"); + DynamicTypeInfo DynTInfo = getDynamicTypeInfo(State, L.getRegion()); + if (!DynTInfo.isValid()) { + 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()); + QualType DynT = DynTInfo.getType(); - // TODO: Dereferencing should be done according to the dynamic type. - while (Optional L = DerefdV.getAs()) { - DerefdV = State->getSVal(*L); + if (isVoidPointer(DynT)) { + IsAnyFieldInitialized = true; + return false; } - // If V is a pointer pointing to a record type. - if (Optional RecordV = - DerefdV.getAs()) { + // 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); - const TypedValueRegion *R = RecordV->getRegion(); + // 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; + } - // 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 (R->getSymbolicBase()) { + DynTInfo = getDynamicTypeInfo(State, Tmp->getRegion()); + if (!DynTInfo.isValid()) { IsAnyFieldInitialized = true; return false; } - const QualType T = R->getValueType(); + DynT = DynTInfo.getType(); + if (isVoidPointer(DynT)) { + IsAnyFieldInitialized = true; + return false; + } - if (T->isStructureOrClassType()) + DerefdV = State->getSVal(*Tmp, DynT); + } + + // If FR is a pointer pointing to a non-primitive type. + if (Optional RecordV = + DerefdV.getAs()) { + + const TypedValueRegion *R = RecordV->getRegion(); + + if (DynT->getPointeeType()->isStructureOrClassType()) return isNonUnionUninit(R, {LocalChain, FR}); - if (T->isUnionType()) { + if (DynT->getPointeeType()->isUnionType()) { if (isUnionUninit(R)) { return addFieldToUninits({LocalChain, FR, /*IsDereferenced*/ true}); } else { @@ -511,7 +533,7 @@ } } - if (T->isArrayType()) { + if (DynT->getPointeeType()->isArrayType()) { IsAnyFieldInitialized = true; return false; } @@ -519,8 +541,10 @@ llvm_unreachable("All cases are handled!"); } - // TODO: If possible, it should be asserted that the DerefdV at this point is - // primitive. + assert((isPrimitiveType(DynT->getPointeeType()) || DynT->isPointerType() || + DynT->isReferenceType()) && + "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)) return addFieldToUninits({LocalChain, FR, /*IsDereferenced*/ true}); @@ -564,6 +588,25 @@ return (*Chain.begin())->getDecl(); } +// TODO: This function constructs an incorrect string if a void pointer is a +// part of the chain: +// +// struct B { int x; } +// +// struct A { +// void *vptr; +// A(void* vptr) : vptr(vptr) {} +// }; +// +// void f() { +// B b; +// A a(&b); +// } +// +// The note message will be "uninitialized field 'this->vptr->x'", even though +// void pointers can't be dereferenced. This should be changed to "uninitialized +// field 'static_cast(this->vptr)->x'". +// // TODO: This function constructs an incorrect fieldchain string in the // following case: // @@ -622,9 +665,7 @@ namespace { -bool isVoidPointer(const FieldDecl *FD) { - QualType T = FD->getType(); - +bool isVoidPointer(QualType T) { while (!T.isNull()) { if (T->isVoidPointerType()) return true; Index: test/Analysis/cxx-uninitialized-object-inheritance.cpp =================================================================== --- test/Analysis/cxx-uninitialized-object-inheritance.cpp +++ test/Analysis/cxx-uninitialized-object-inheritance.cpp @@ -773,3 +773,26 @@ void fVirtualDiamondInheritanceTest3() { VirtualDiamondInheritanceTest3(); } + +//===----------------------------------------------------------------------===// +// Dynamic type test. +//===----------------------------------------------------------------------===// + +struct DynTBase {}; +struct DynTDerived : DynTBase { + // TODO: we'd expect the note: {{uninitialized field 'this->x'}} + int x; // no-note +}; + +struct DynamicTypeTest { + DynTBase *bptr; + int i = 0; + + // TODO: we'd expect the warning: {{1 uninitialized field}} + DynamicTypeTest(DynTBase *bptr) : bptr(bptr) {} // no-warning +}; + +void f() { + DynTDerived d; + DynamicTypeTest t(&d); +}; 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 @@ -191,7 +191,7 @@ struct CyclicPointerTest { int *ptr; - CyclicPointerTest() : ptr(reinterpret_cast(&ptr)) {} + CyclicPointerTest() : ptr(reinterpret_cast(&ptr)) {} }; void fCyclicPointerTest() { @@ -280,13 +280,62 @@ void *vptr; // no-crash CyclicVoidPointerTest() : vptr(&vptr) {} - }; void fCyclicVoidPointerTest() { CyclicVoidPointerTest(); } +struct IntDynTypedVoidPointerTest1 { + void *vptr; // expected-note{{uninitialized pointee 'this->vptr'}} + int dontGetFilteredByNonPedanticMode = 0; + + IntDynTypedVoidPointerTest1(void *vptr) : vptr(vptr) {} // expected-warning{{1 uninitialized field}} +}; + +void fIntDynTypedVoidPointerTest1() { + int a; + IntDynTypedVoidPointerTest1 tmp(&a); +} + +struct RecordDynTypedVoidPointerTest { + struct RecordType { + int x; // expected-note{{uninitialized field 'this->vptr->x'}} + int y; // expected-note{{uninitialized field 'this->vptr->y'}} + }; + + void *vptr; + int dontGetFilteredByNonPedanticMode = 0; + + RecordDynTypedVoidPointerTest(void *vptr) : vptr(vptr) {} // expected-warning{{2 uninitialized fields}} +}; + +void fRecordDynTypedVoidPointerTest() { + RecordDynTypedVoidPointerTest::RecordType a; + RecordDynTypedVoidPointerTest tmp(&a); +} + +struct NestedNonVoidDynTypedVoidPointerTest { + struct RecordType { + int x; // expected-note{{uninitialized field 'this->vptr->x'}} + int y; // expected-note{{uninitialized field 'this->vptr->y'}} + void *vptr; // expected-note{{uninitialized pointee 'this->vptr->vptr'}} + }; + + void *vptr; + int dontGetFilteredByNonPedanticMode = 0; + + NestedNonVoidDynTypedVoidPointerTest(void *vptr, void *c) : vptr(vptr) { + static_cast(vptr)->vptr = c; // expected-warning{{3 uninitialized fields}} + } +}; + +void fNestedNonVoidDynTypedVoidPointerTest() { + NestedNonVoidDynTypedVoidPointerTest::RecordType a; + char c; + NestedNonVoidDynTypedVoidPointerTest tmp(&a, &c); +} + //===----------------------------------------------------------------------===// // Multipointer tests. //===----------------------------------------------------------------------===//