Szelethus updated this revision to Diff 163813.
Szelethus added a comment.
Fixed a crash where the returned region's type wasn't `CXXRecordDecl`. I'm not
even sure how this is possible -- and unfortunately I've been unable to create
a minimal (not) working example for this, and I wasn't even able to recreate
the error locally. However, on the server where I could repeatedly cause a
crash with the analysis of `lib/AST/Expr.cpp`, this fix resolved the issue.
Note that this assert failure didn't happen inside `willBeAnalyzedLater`, where
`getConstructedRegion` is used with a different `CXXConstructorDecl` then the
one on top of the stack frame.
https://reviews.llvm.org/D51300
Files:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
Index:
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===================================================================
---
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -124,12 +124,11 @@
// 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
@@ -159,12 +158,11 @@
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(),
- CheckPointeeInitialization);
+ FindUninitializedFields F(Context.getState(), R, CheckPointeeInitialization);
const UninitFieldMap &UninitFields = F.getUninitFields();
@@ -443,25 +441,27 @@
// 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);
- return Object.getAs<nonloc::LazyCompoundVal>();
+ auto *R = ObjectV.getAsRegion()->getAs<TypedValueRegion>();
+ if (R && !R->getValueType()->getAsCXXRecordDecl())
+ return nullptr;
+
+ 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();
@@ -472,14 +472,14 @@
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;
}
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -124,12 +124,11 @@
// 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
@@ -159,12 +158,11 @@
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(),
- CheckPointeeInitialization);
+ FindUninitializedFields F(Context.getState(), R, CheckPointeeInitialization);
const UninitFieldMap &UninitFields = F.getUninitFields();
@@ -443,25 +441,27 @@
// 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);
- return Object.getAs<nonloc::LazyCompoundVal>();
+ auto *R = ObjectV.getAsRegion()->getAs<TypedValueRegion>();
+ if (R && !R->getValueType()->getAsCXXRecordDecl())
+ return nullptr;
+
+ 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();
@@ -472,14 +472,14 @@
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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
