diff options
Diffstat (limited to 'lib/StaticAnalyzer/Checkers/UninitializedObject')
3 files changed, 146 insertions, 69 deletions
diff --git a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h index d10b862ea0..c3291a21c1 100644 --- a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h +++ b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h @@ -21,7 +21,7 @@ // `-analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true`. // // - "NotesAsWarnings" (boolean). If set to true, the checker will emit a -// warning for each uninitalized field, as opposed to emitting one warning +// warning for each uninitialized field, as opposed to emitting one warning // per constructor call, and listing the uninitialized fields that belongs // to it in notes. Defaults to false. // @@ -215,7 +215,11 @@ public: const TypedValueRegion *const R, const UninitObjCheckerOptions &Opts); - const UninitFieldMap &getUninitFields() { return UninitFields; } + /// Returns with the modified state and a map of (uninitialized region, + /// note message) pairs. + std::pair<ProgramStateRef, const UninitFieldMap &> getResults() { + return {State, UninitFields}; + } /// Returns whether the analyzed region contains at least one initialized /// field. Note that this includes subfields as well, not just direct ones, @@ -230,7 +234,7 @@ private: // * every node is an object that is // - a union // - a non-union record - // - dereferencable (see isDereferencableType()) + // - dereferenceable (see isDereferencableType()) // - an array // - of a primitive type (see isPrimitiveType()) // * the parent of each node is the object that contains it @@ -271,7 +275,7 @@ private: // this->iptr (pointee uninit) // this->bptr (pointer uninit) // - // We'll traverse each node of the above graph with the appropiate one of + // We'll traverse each node of the above graph with the appropriate one of // these methods: /// Checks the region of a union object, and returns true if no field is @@ -296,14 +300,16 @@ private: // TODO: Add a support for nonloc::LocAsInteger. /// Processes LocalChain and attempts to insert it into UninitFields. Returns - /// true on success. + /// true on success. Also adds the head of the list and \p PointeeR (if + /// supplied) to the GDM as already analyzed objects. /// /// Since this class analyzes regions with recursion, we'll only store /// references to temporary FieldNode objects created on the stack. This means /// that after analyzing a leaf of the directed tree described above, the /// elements LocalChain references will be destructed, so we can't store it /// directly. - bool addFieldToUninits(FieldChainInfo LocalChain); + bool addFieldToUninits(FieldChainInfo LocalChain, + const MemRegion *PointeeR = nullptr); }; /// Returns true if T is a primitive type. An object of a primitive type only diff --git a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp index 50ab7c0a0e..94f664ab93 100644 --- a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp @@ -28,9 +28,14 @@ using namespace clang; using namespace clang::ento; +/// We'll mark fields (and pointee of fields) that are confirmed to be +/// uninitialized as already analyzed. +REGISTER_SET_WITH_PROGRAMSTATE(AnalyzedRegions, const MemRegion *) + namespace { -class UninitializedObjectChecker : public Checker<check::EndFunction> { +class UninitializedObjectChecker + : public Checker<check::EndFunction, check::DeadSymbols> { std::unique_ptr<BuiltinBug> BT_uninitField; public: @@ -39,7 +44,9 @@ public: UninitializedObjectChecker() : BT_uninitField(new BuiltinBug(this, "Uninitialized fields")) {} + void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const; + void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const; }; /// A basic field type, that is not a pointer or a reference, it's dynamic and @@ -96,12 +103,11 @@ public: // Utility function declarations. -/// Returns the object that was constructed by CtorDecl, or None if that isn't -/// possible. -// TODO: Refactor this function so that it returns the constructed object's -// region. -static Optional<nonloc::LazyCompoundVal> -getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext &Context); +/// Returns the region that was constructed by CtorDecl, or nullptr if that +/// isn't possible. +static const TypedValueRegion * +getConstructedRegion(const CXXConstructorDecl *CtorDecl, + CheckerContext &Context); /// Checks whether the object constructed by \p Ctor will be analyzed later /// (e.g. if the object is a field of another object, in which case we'd check @@ -135,20 +141,26 @@ void UninitializedObjectChecker::checkEndFunction( if (willObjectBeAnalyzedLater(CtorDecl, Context)) return; - Optional<nonloc::LazyCompoundVal> Object = getObjectVal(CtorDecl, Context); - if (!Object) + const TypedValueRegion *R = getConstructedRegion(CtorDecl, Context); + if (!R) return; - FindUninitializedFields F(Context.getState(), Object->getRegion(), Opts); + FindUninitializedFields F(Context.getState(), R, Opts); + + std::pair<ProgramStateRef, const UninitFieldMap &> UninitInfo = + F.getResults(); - const UninitFieldMap &UninitFields = F.getUninitFields(); + ProgramStateRef UpdatedState = UninitInfo.first; + const UninitFieldMap &UninitFields = UninitInfo.second; - if (UninitFields.empty()) + if (UninitFields.empty()) { + Context.addTransition(UpdatedState); return; + } // There are uninitialized fields in the record. - ExplodedNode *Node = Context.generateNonFatalErrorNode(Context.getState()); + ExplodedNode *Node = Context.generateNonFatalErrorNode(UpdatedState); if (!Node) return; @@ -189,6 +201,15 @@ void UninitializedObjectChecker::checkEndFunction( Context.emitReport(std::move(Report)); } +void UninitializedObjectChecker::checkDeadSymbols(SymbolReaper &SR, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + for (const MemRegion *R : State->get<AnalyzedRegions>()) { + if (!SR.isLiveRegion(R)) + State = State->remove<AnalyzedRegions>(R); + } +} + //===----------------------------------------------------------------------===// // Methods for FindUninitializedFields. //===----------------------------------------------------------------------===// @@ -206,17 +227,34 @@ FindUninitializedFields::FindUninitializedFields( UninitFields.clear(); } -bool FindUninitializedFields::addFieldToUninits(FieldChainInfo Chain) { +bool FindUninitializedFields::addFieldToUninits(FieldChainInfo Chain, + const MemRegion *PointeeR) { + const FieldRegion *FR = Chain.getUninitRegion(); + + assert((PointeeR || !isDereferencableType(FR->getDecl()->getType())) && + "One must also pass the pointee region as a parameter for " + "dereferenceable fields!"); + + if (State->contains<AnalyzedRegions>(FR)) + return false; + + if (PointeeR) { + if (State->contains<AnalyzedRegions>(PointeeR)) { + return false; + } + State = State->add<AnalyzedRegions>(PointeeR); + } + + State = State->add<AnalyzedRegions>(FR); + if (State->getStateManager().getContext().getSourceManager().isInSystemHeader( - Chain.getUninitRegion()->getDecl()->getLocation())) + FR->getDecl()->getLocation())) return false; UninitFieldMap::mapped_type NoteMsgBuf; llvm::raw_svector_ostream OS(NoteMsgBuf); Chain.printNoteMsg(OS); - return UninitFields - .insert(std::make_pair(Chain.getUninitRegion(), std::move(NoteMsgBuf))) - .second; + return UninitFields.insert({FR, std::move(NoteMsgBuf)}).second; } bool FindUninitializedFields::isNonUnionUninit(const TypedValueRegion *R, @@ -400,25 +438,27 @@ static void printTail(llvm::raw_ostream &Out, // Utility functions. //===----------------------------------------------------------------------===// -static Optional<nonloc::LazyCompoundVal> -getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext &Context) { +static const TypedValueRegion * +getConstructedRegion(const CXXConstructorDecl *CtorDecl, + CheckerContext &Context) { - Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(), + Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl, Context.getStackFrame()); - // Getting the value for 'this'. - SVal This = Context.getState()->getSVal(ThisLoc); - // Getting the value for '*this'. - SVal Object = Context.getState()->getSVal(This.castAs<Loc>()); + SVal ObjectV = Context.getState()->getSVal(ThisLoc); + + auto *R = ObjectV.getAsRegion()->getAs<TypedValueRegion>(); + if (R && !R->getValueType()->getAsCXXRecordDecl()) + return nullptr; - return Object.getAs<nonloc::LazyCompoundVal>(); + return R; } static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor, CheckerContext &Context) { - Optional<nonloc::LazyCompoundVal> CurrentObject = getObjectVal(Ctor, Context); - if (!CurrentObject) + const TypedValueRegion *CurrRegion = getConstructedRegion(Ctor, Context); + if (!CurrRegion) return false; const LocationContext *LC = Context.getLocationContext(); @@ -429,14 +469,14 @@ static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor, if (!OtherCtor) continue; - Optional<nonloc::LazyCompoundVal> OtherObject = - getObjectVal(OtherCtor, Context); - if (!OtherObject) + const TypedValueRegion *OtherRegion = + getConstructedRegion(OtherCtor, Context); + if (!OtherRegion) continue; - // If the CurrentObject is a subregion of OtherObject, it will be analyzed - // during the analysis of OtherObject. - if (CurrentObject->getRegion()->isSubRegionOf(OtherObject->getRegion())) + // If the CurrRegion is a subregion of OtherRegion, it will be analyzed + // during the analysis of OtherRegion. + if (CurrRegion->isSubRegionOf(OtherRegion)) return true; } @@ -487,12 +527,12 @@ void ento::registerUninitializedObjectChecker(CheckerManager &Mgr) { UninitObjCheckerOptions &ChOpts = Chk->Opts; ChOpts.IsPedantic = - AnOpts.getBooleanOption("Pedantic", /*DefaultVal*/ false, Chk); + AnOpts.getCheckerBooleanOption("Pedantic", /*DefaultVal*/ false, Chk); ChOpts.ShouldConvertNotesToWarnings = - AnOpts.getBooleanOption("NotesAsWarnings", /*DefaultVal*/ false, Chk); - ChOpts.CheckPointeeInitialization = AnOpts.getBooleanOption( + AnOpts.getCheckerBooleanOption("NotesAsWarnings", /*DefaultVal*/ false, Chk); + ChOpts.CheckPointeeInitialization = AnOpts.getCheckerBooleanOption( "CheckPointeeInitialization", /*DefaultVal*/ false, Chk); ChOpts.IgnoredRecordsWithFieldPattern = - AnOpts.getOptionAsString("IgnoreRecordsWithField", + AnOpts.getCheckerStringOption("IgnoreRecordsWithField", /*DefaultVal*/ "", Chk); } diff --git a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp index 623ba6b3ff..ae53f00b0b 100644 --- a/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp +++ b/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp @@ -89,15 +89,39 @@ public: } }; +/// Represents a Loc field that points to itself. +class CyclicLocField final : public FieldNode { + +public: + CyclicLocField(const FieldRegion *FR) : FieldNode(FR) {} + + virtual void printNoteMsg(llvm::raw_ostream &Out) const override { + Out << "object references itself "; + } + + virtual void printPrefix(llvm::raw_ostream &Out) const override {} + + virtual void printNode(llvm::raw_ostream &Out) const override { + Out << getVariableName(getDecl()); + } + + virtual void printSeparator(llvm::raw_ostream &Out) const override { + llvm_unreachable("CyclicLocField objects must be the last node of the " + "fieldchain!"); + } +}; + } // end of anonymous namespace // Utility function declarations. -/// Returns whether \p T can be (transitively) dereferenced to a void pointer -/// type (void*, void**, ...). -static bool isVoidPointer(QualType T); - -using DereferenceInfo = std::pair<const TypedValueRegion *, bool>; +struct DereferenceInfo { + const TypedValueRegion *R; + const bool NeedsCastBack; + const bool IsCyclic; + DereferenceInfo(const TypedValueRegion *R, bool NCB, bool IC) + : R(R), NeedsCastBack(NCB), IsCyclic(IC) {} +}; /// 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 @@ -105,6 +129,10 @@ using DereferenceInfo = std::pair<const TypedValueRegion *, bool>; static llvm::Optional<DereferenceInfo> dereference(ProgramStateRef State, const FieldRegion *FR); +/// Returns whether \p T can be (transitively) dereferenced to a void pointer +/// type (void*, void**, ...). +static bool isVoidPointer(QualType T); + //===----------------------------------------------------------------------===// // Methods for FindUninitializedFields. //===----------------------------------------------------------------------===// @@ -116,7 +144,7 @@ bool FindUninitializedFields::isDereferencableUninit( assert((isDereferencableType(FR->getDecl()->getType()) || V.getAs<nonloc::LocAsInteger>()) && - "This method only checks dereferencable objects!"); + "This method only checks dereferenceable objects!"); if (V.isUnknown() || V.getAs<loc::ConcreteInt>()) { IsAnyFieldInitialized = true; @@ -125,7 +153,7 @@ bool FindUninitializedFields::isDereferencableUninit( if (V.isUndef()) { return addFieldToUninits( - LocalChain.add(LocField(FR, /*IsDereferenced*/ false))); + LocalChain.add(LocField(FR, /*IsDereferenced*/ false)), FR); } if (!Opts.CheckPointeeInitialization) { @@ -141,8 +169,11 @@ bool FindUninitializedFields::isDereferencableUninit( return false; } - const TypedValueRegion *R = DerefInfo->first; - const bool NeedsCastBack = DerefInfo->second; + if (DerefInfo->IsCyclic) + return addFieldToUninits(LocalChain.add(CyclicLocField(FR)), FR); + + const TypedValueRegion *R = DerefInfo->R; + const bool NeedsCastBack = DerefInfo->NeedsCastBack; QualType DynT = R->getLocationType(); QualType PointeeT = DynT->getPointeeType(); @@ -156,8 +187,9 @@ bool FindUninitializedFields::isDereferencableUninit( if (PointeeT->isUnionType()) { if (isUnionUninit(R)) { if (NeedsCastBack) - return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT))); - return addFieldToUninits(LocalChain.add(LocField(FR))); + return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT)), + R); + return addFieldToUninits(LocalChain.add(LocField(FR)), R); } else { IsAnyFieldInitialized = true; return false; @@ -177,8 +209,8 @@ bool FindUninitializedFields::isDereferencableUninit( if (isPrimitiveUninit(PointeeV)) { if (NeedsCastBack) - return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT))); - return addFieldToUninits(LocalChain.add(LocField(FR))); + return addFieldToUninits(LocalChain.add(NeedsCastLocField(FR, DynT)), R); + return addFieldToUninits(LocalChain.add(LocField(FR)), R); } IsAnyFieldInitialized = true; @@ -189,15 +221,6 @@ bool FindUninitializedFields::isDereferencableUninit( // Utility functions. //===----------------------------------------------------------------------===// -static bool isVoidPointer(QualType T) { - while (!T.isNull()) { - if (T->isVoidPointerType()) - return true; - T = T->getPointeeType(); - } - return false; -} - static llvm::Optional<DereferenceInfo> dereference(ProgramStateRef State, const FieldRegion *FR) { @@ -229,9 +252,8 @@ static llvm::Optional<DereferenceInfo> dereference(ProgramStateRef State, return None; // We found a cyclic pointer, like int *ptr = (int *)&ptr. - // TODO: Should we report these fields too? if (!VisitedRegions.insert(R).second) - return None; + return DereferenceInfo{R, NeedsCastBack, /*IsCyclic*/ true}; DynT = R->getLocationType(); // In order to ensure that this loop terminates, we're also checking the @@ -248,5 +270,14 @@ static llvm::Optional<DereferenceInfo> dereference(ProgramStateRef State, R = R->getSuperRegion()->getAs<TypedValueRegion>(); } - return std::make_pair(R, NeedsCastBack); + return DereferenceInfo{R, NeedsCastBack, /*IsCyclic*/ false}; +} + +static bool isVoidPointer(QualType T) { + while (!T.isNull()) { + if (T->isVoidPointerType()) + return true; + T = T->getPointeeType(); + } + return false; } |