Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp @@ -212,9 +212,11 @@ Optional getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext &Context); -/// Checks whether the constructor under checking is called by another -/// constructor. -bool isCalledByConstructor(const 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 +/// it multiple times). +bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor, + CheckerContext &Context); /// Returns whether FD can be (transitively) dereferenced to a void pointer type /// (void*, void**, ...). The type of the region behind a void pointer isn't @@ -255,7 +257,7 @@ return; // This avoids essentially the same error being reported multiple times. - if (isCalledByConstructor(Context)) + if (willObjectBeAnalyzedLater(CtorDecl, Context)) return; Optional Object = getObjectVal(CtorDecl, Context); @@ -419,8 +421,8 @@ } // Checking bases. - // FIXME: As of now, because of `isCalledByConstructor`, objects whose type - // is a descendant of another type will emit warnings for uninitalized + // FIXME: As of now, because of `willObjectBeAnalyzedLater`, objects whose + // type is a descendant of another type will emit warnings for uninitalized // inherited members. // This is not the only way to analyze bases of an object -- if we didn't // filter them out, and didn't analyze the bases, this checker would run for @@ -661,18 +663,32 @@ return Object.getAs(); } -// TODO: We should also check that if the constructor was called by another -// constructor, whether those two are in any relation to one another. In it's -// current state, this introduces some false negatives. -bool isCalledByConstructor(const CheckerContext &Context) { - const LocationContext *LC = Context.getLocationContext()->getParent(); +bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor, + CheckerContext &Context) { - while (LC) { - if (isa(LC->getDecl())) - return true; + Optional CurrentObject = getObjectVal(Ctor, Context); + if (!CurrentObject) + return false; + + const LocationContext *LC = Context.getLocationContext(); + while ((LC = LC->getParent())) { + + // If \p Ctor was called by another constructor. + const auto *OtherCtor = dyn_cast(LC->getDecl()); + if (!OtherCtor) + continue; - LC = LC->getParent(); + Optional OtherObject = + getObjectVal(OtherCtor, Context); + if (!OtherObject) + continue; + + // If the CurrentObject is a field of OtherObject, it will be analyzed + // during the analysis of OtherObject. + if (CurrentObject->getRegion()->isSubRegionOf(OtherObject->getRegion())) + return true; } + return false; } Index: test/Analysis/cxx-uninitialized-object.cpp =================================================================== --- test/Analysis/cxx-uninitialized-object.cpp +++ test/Analysis/cxx-uninitialized-object.cpp @@ -1035,13 +1035,12 @@ // While a singleton would make more sense as a static variable, that would zero // initialize all of its fields, hence the not too practical implementation. struct Singleton { - // TODO: we'd expect the note: {{uninitialized field 'this->i'}} - int i; // no-note + int i; // expected-note{{uninitialized field 'this->i'}} + int dontGetFilteredByNonPedanticMode = 0; Singleton() { assert(!isInstantiated); - // TODO: we'd expect the warning: {{1 uninitialized field}} - isInstantiated = true; // no-warning + isInstantiated = true; // expected-warning{{1 uninitialized field}} } ~Singleton() {