Szelethus updated this revision to Diff 163992. Szelethus added a comment. Fixed an assertation failure where a lambda field's this capture was undefined.
https://reviews.llvm.org/D51057 Files: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp test/Analysis/cxx-uninitialized-object-ptr-ref.cpp test/Analysis/cxx-uninitialized-object.cpp test/Analysis/objcpp-uninitialized-object.mm
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(^{}) {} Index: test/Analysis/cxx-uninitialized-object.cpp =================================================================== --- test/Analysis/cxx-uninitialized-object.cpp +++ test/Analysis/cxx-uninitialized-object.cpp @@ -1,11 +1,11 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ // RUN: -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \ // RUN: -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true \ -// RUN: -std=c++11 -verify %s +// RUN: -std=c++14 -verify %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \ // RUN: -analyzer-config alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true \ -// RUN: -std=c++11 -verify %s +// RUN: -std=c++14 -verify %s //===----------------------------------------------------------------------===// // Default constructor test. @@ -781,7 +781,7 @@ void fLambdaTest2() { int b; - auto equals = [&b](int a) { return a == b; }; // expected-note{{uninitialized pointee 'this->functor.b'}} + auto equals = [&b](int a) { return a == b; }; // expected-note{{uninitialized pointee 'this->functor./*captured variable*/b'}} LambdaTest2<decltype(equals)>(equals, int()); } #else @@ -803,8 +803,8 @@ namespace LT3Detail { struct RecordType { - int x; // expected-note{{uninitialized field 'this->functor.rec1.x'}} - int y; // expected-note{{uninitialized field 'this->functor.rec1.y'}} + int x; // expected-note{{uninitialized field 'this->functor./*captured variable*/rec1.x'}} + int y; // expected-note{{uninitialized field 'this->functor./*captured variable*/rec1.y'}} }; } // namespace LT3Detail @@ -857,8 +857,8 @@ void fMultipleLambdaCapturesTest1() { int b1, b2 = 3, b3; - auto equals = [&b1, &b2, &b3](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized pointee 'this->functor.b1'}} - // expected-note@-1{{uninitialized pointee 'this->functor.b3'}} + auto equals = [&b1, &b2, &b3](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized pointee 'this->functor./*captured variable*/b1'}} + // expected-note@-1{{uninitialized pointee 'this->functor./*captured variable*/b3'}} MultipleLambdaCapturesTest1<decltype(equals)>(equals, int()); } @@ -872,10 +872,35 @@ void fMultipleLambdaCapturesTest2() { int b1, b2 = 3, b3; - auto equals = [b1, &b2, &b3](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized pointee 'this->functor.b3'}} + auto equals = [b1, &b2, &b3](int a) { return a == b1 == b2 == b3; }; // expected-note{{uninitialized pointee 'this->functor./*captured variable*/b3'}} MultipleLambdaCapturesTest2<decltype(equals)>(equals, int()); } +struct LambdaWrapper { + void *func; // no-crash + int dontGetFilteredByNonPedanticMode = 0; + + LambdaWrapper(void *ptr) : func(ptr) {} // expected-warning{{1 uninitialized field}} +}; + +struct ThisCapturingLambdaFactory { + int a; // expected-note{{uninitialized field 'static_cast<decltype(a.ret()) *>(this->func)->/*'this' capture*/->a'}} + + auto ret() { + return [this] { (void)this; }; + } +}; + +void fLambdaFieldWithInvalidThisCapture() { + void *ptr; + { + ThisCapturingLambdaFactory a; + decltype(a.ret()) lambda = a.ret(); + ptr = λ + } + LambdaWrapper t(ptr); +} + //===----------------------------------------------------------------------===// // System header tests. //===----------------------------------------------------------------------===// 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<int *>(__builtin_alloca(sizeof(int)))) {} +}; + +void fTypedAllocaTest1() { + TypedAllocaTest1(); +} + +struct TypedAllocaTest2 { + int *allocaPtr; + int dontGetFilteredByNonPedanticMode = 0; + + TypedAllocaTest2() + : allocaPtr(static_cast<int *>(__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<int **>(&pptr)) {} }; void fCyclicPointerTest2() { CyclicPointerTest2(); } -*/ //===----------------------------------------------------------------------===// // Void pointer tests. @@ -471,6 +511,39 @@ } //===----------------------------------------------------------------------===// +// Incomplete pointee tests. +//===----------------------------------------------------------------------===// + +class IncompleteType; + +struct IncompletePointeeTypeTest { + IncompleteType *pImpl; //no-crash + int dontGetFilteredByNonPedanticMode = 0; + + IncompletePointeeTypeTest(IncompleteType *A) : pImpl(A) {} +}; + +void fIncompletePointeeTypeTest(void *ptr) { + IncompletePointeeTypeTest(reinterpret_cast<IncompleteType *>(ptr)); +} + +//===----------------------------------------------------------------------===// +// Function pointer tests. +//===----------------------------------------------------------------------===// + +struct FunctionPointerWithDifferentDynTypeTest { + using Func1 = void *(*)(); + using Func2 = int *(*)(); + + Func1 f; // no-crash + FunctionPointerWithDifferentDynTypeTest(Func2 f) : f((Func1)f) {} +}; + +// Note that there isn't a function calling the constructor of +// FunctionPointerWithDifferentDynTypeTest, because a crash could only be +// reproduced without it. + +//===----------------------------------------------------------------------===// // Member pointer tests. //===----------------------------------------------------------------------===// @@ -645,6 +718,15 @@ CyclicList(&n1, int()); } +struct RingListTest { + RingListTest *next; // no-crash + RingListTest() : next(this) {} +}; + +void fRingListTest() { + RingListTest(); +} + //===----------------------------------------------------------------------===// // Tests for classes containing references. //===----------------------------------------------------------------------===// 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 whether \p FR needs to be casted back to that type. If for whatever -/// reason dereferencing fails, returns with None. -static llvm::Optional<std::tuple<SVal, QualType, bool>> -dereference(ProgramStateRef State, const FieldRegion *FR); +using DereferenceInfo = std::pair<const TypedValueRegion *, bool>; + +/// 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<DereferenceInfo> 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); @@ -134,54 +134,47 @@ // At this point the pointer itself is initialized and points to a valid // location, we'll now check the pointee. - llvm::Optional<std::tuple<SVal, QualType, bool>> DerefInfo = - dereference(State, FR); + llvm::Optional<DereferenceInfo> 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<nonloc::LazyCompoundVal> RecordV = - V.getAs<nonloc::LazyCompoundVal>()) { + 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 +197,46 @@ return false; } -static llvm::Optional<std::tuple<SVal, QualType, bool>> -dereference(ProgramStateRef State, const FieldRegion *FR) { +static llvm::Optional<DereferenceInfo> dereference(ProgramStateRef State, + const FieldRegion *FR) { - DynamicTypeInfo DynTInfo; - QualType DynT; + llvm::SmallSet<const TypedValueRegion *, 5> 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<loc::MemRegionVal>() && "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<int **>(&ptr); - while (auto Tmp = V.getAs<loc::MemRegionVal>()) { - // 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<TypedValueRegion>(); + 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<TypedValueRegion>(); + + 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: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -252,9 +252,12 @@ !R->getValueType()->isUnionType() && "This method only checks non-union record objects!"); - const RecordDecl *RD = - R->getValueType()->getAs<RecordType>()->getDecl()->getDefinition(); - assert(RD && "Referred record has no definition"); + const RecordDecl *RD = R->getValueType()->getAsRecordDecl()->getDefinition(); + + if (!RD) { + IsAnyFieldInitialized = true; + return true; + } bool ContainsUninitField = false; @@ -292,8 +295,7 @@ continue; } - if (T->isAnyPointerType() || T->isReferenceType() || - T->isBlockPointerType()) { + if (isDereferencableType(T)) { if (isPointerOrReferenceUninit(FR, LocalChain)) ContainsUninitField = true; continue; @@ -487,16 +489,25 @@ return false; } -StringRef clang::ento::getVariableName(const FieldDecl *Field) { +std::string clang::ento::getVariableName(const FieldDecl *Field) { // If Field is a captured lambda variable, Field->getName() will return with // an empty string. We can however acquire it's name from the lambda's // captures. const auto *CXXParent = dyn_cast<CXXRecordDecl>(Field->getParent()); if (CXXParent && CXXParent->isLambda()) { assert(CXXParent->captures_begin()); auto It = CXXParent->captures_begin() + Field->getFieldIndex(); - return It->getCapturedVar()->getName(); + + if (It->capturesVariable()) + return llvm::Twine("/*captured variable*/" + + It->getCapturedVar()->getName()) + .str(); + + if (It->capturesThis()) + return "/*'this' capture*/"; + + llvm_unreachable("No other capture type is expected!"); } return Field->getName(); Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h =================================================================== --- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h +++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h @@ -87,7 +87,7 @@ /// Returns with Field's name. This is a helper function to get the correct name /// even if Field is a captured lambda variable. -StringRef getVariableName(const FieldDecl *Field); +std::string getVariableName(const FieldDecl *Field); /// Represents a field chain. A field chain is a vector of fields where the /// first element of the chain is the object under checking (not stored), and @@ -255,7 +255,13 @@ /// 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() || + T->isFunctionType(); +} + +inline bool isDereferencableType(const QualType &T) { + return T->isAnyPointerType() || T->isReferenceType(); } // Template method definitions.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits