Author: szelethus Date: Sun Nov 18 03:34:10 2018 New Revision: 347153 URL: http://llvm.org/viewvc/llvm-project?rev=347153&view=rev Log: [analyzer][UninitializedObjectChecker] Uninit regions are only reported once
Especially with pointees, a lot of meaningless reports came from uninitialized regions that were already reported. This is fixed by storing all reported fields to the GDM. Differential Revision: https://reviews.llvm.org/D51531 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h?rev=347153&r1=347152&r2=347153&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h Sun Nov 18 03:34:10 2018 @@ -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, @@ -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 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp?rev=347153&r1=347152&r2=347153&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp Sun Nov 18 03:34:10 2018 @@ -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 @@ -140,14 +147,20 @@ void UninitializedObjectChecker::checkEn FindUninitializedFields F(Context.getState(), R, Opts); - const UninitFieldMap &UninitFields = F.getUninitFields(); + std::pair<ProgramStateRef, const UninitFieldMap &> UninitInfo = + F.getResults(); - if (UninitFields.empty()) + ProgramStateRef UpdatedState = UninitInfo.first; + const UninitFieldMap &UninitFields = UninitInfo.second; + + 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; @@ -188,6 +201,15 @@ void UninitializedObjectChecker::checkEn 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. //===----------------------------------------------------------------------===// @@ -205,17 +227,34 @@ FindUninitializedFields::FindUninitializ 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 " + "dereferencable 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, Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp?rev=347153&r1=347152&r2=347153&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp Sun Nov 18 03:34:10 2018 @@ -153,7 +153,7 @@ bool FindUninitializedFields::isDerefere if (V.isUndef()) { return addFieldToUninits( - LocalChain.add(LocField(FR, /*IsDereferenced*/ false))); + LocalChain.add(LocField(FR, /*IsDereferenced*/ false)), FR); } if (!Opts.CheckPointeeInitialization) { @@ -170,7 +170,7 @@ bool FindUninitializedFields::isDerefere } if (DerefInfo->IsCyclic) - return addFieldToUninits(LocalChain.add(CyclicLocField(FR))); + return addFieldToUninits(LocalChain.add(CyclicLocField(FR)), FR); const TypedValueRegion *R = DerefInfo->R; const bool NeedsCastBack = DerefInfo->NeedsCastBack; @@ -187,8 +187,9 @@ bool FindUninitializedFields::isDerefere 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; @@ -208,8 +209,8 @@ bool FindUninitializedFields::isDerefere 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; Modified: cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp?rev=347153&r1=347152&r2=347153&view=diff ============================================================================== --- cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp (original) +++ cfe/trunk/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp Sun Nov 18 03:34:10 2018 @@ -865,3 +865,44 @@ void fReferenceTest5() { ReferenceTest4::RecordType c, d{37, 38}; ReferenceTest4(d, c); } + +//===----------------------------------------------------------------------===// +// Tests for objects containing multiple references to the same object. +//===----------------------------------------------------------------------===// + +struct IntMultipleReferenceToSameObjectTest { + int *iptr; // expected-note{{uninitialized pointee 'this->iptr'}} + int &iref; // no-note, pointee of this->iref was already reported + + int dontGetFilteredByNonPedanticMode = 0; + + IntMultipleReferenceToSameObjectTest(int *i) : iptr(i), iref(*i) {} // expected-warning{{1 uninitialized field}} +}; + +void fIntMultipleReferenceToSameObjectTest() { + int a; + IntMultipleReferenceToSameObjectTest Test(&a); +} + +struct IntReferenceWrapper1 { + int &a; // expected-note{{uninitialized pointee 'this->a'}} + + int dontGetFilteredByNonPedanticMode = 0; + + IntReferenceWrapper1(int &a) : a(a) {} // expected-warning{{1 uninitialized field}} +}; + +struct IntReferenceWrapper2 { + int &a; // no-note, pointee of this->a was already reported + + int dontGetFilteredByNonPedanticMode = 0; + + IntReferenceWrapper2(int &a) : a(a) {} // no-warning +}; + +void fMultipleObjectsReferencingTheSameObjectTest() { + int a; + + IntReferenceWrapper1 T1(a); + IntReferenceWrapper2 T2(a); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
