Szelethus updated this revision to Diff 152343.
Szelethus added a comment.
Moved `LC = LC ->getParent()` to the `while` statement's argument to avoid a
potential infinite loop. Whoops :)
https://reviews.llvm.org/D48436
Files:
lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
test/Analysis/cxx-uninitialized-object.cpp
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() {
Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
@@ -212,9 +212,11 @@
Optional<nonloc::LazyCompoundVal>
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<nonloc::LazyCompoundVal> 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<nonloc::LazyCompoundVal>();
}
-// 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<CXXConstructorDecl>(LC->getDecl()))
- return true;
+ Optional<nonloc::LazyCompoundVal> 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<CXXConstructorDecl>(LC->getDecl());
+ if (!OtherCtor)
+ continue;
- LC = LC->getParent();
+ Optional<nonloc::LazyCompoundVal> 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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
