Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp @@ -225,12 +225,16 @@ /// 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 getObjectVal(const CXXConstructorDecl *CtorDecl, CheckerContext &Context); -/// Checks whether the constructor under checking is called by another -/// constructor. -static 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). +static 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 @@ -273,7 +277,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); @@ -433,8 +437,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 +665,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. -static bool isCalledByConstructor(const CheckerContext &Context) { - const LocationContext *LC = Context.getLocationContext()->getParent(); +static 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 subregion 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 @@ -1040,13 +1040,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() {