[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 168775.
Szelethus added a comment.
This revision is now accepted and ready to land.

Finally managed to run creduce. I added the test that caused a crash on our 
server, but as I said, I couldn't replicate it elsewhere.


https://reviews.llvm.org/D51300

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/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
@@ -1130,3 +1130,29 @@
   // TODO: we'd expect the warning: {{2 uninitializeds field}}
   CXX11MemberInitTest2(); // no-warning
 }
+
+//===----------------------------------------------------------------------===//
+// Test for cases when the constructed region after the constructor call isn't
+// CXXRecordDecl, when it should be.
+//===----------------------------------------------------------------------===//
+
+struct UndefinedStruct;
+
+struct CXXRecordDeclRegionBugTest {
+  CXXRecordDeclRegionBugTest(const UndefinedStruct &t);
+
+  struct UserDefinedDefaultCtorStruct {
+    UserDefinedDefaultCtorStruct() {}
+  };
+};
+
+using size_t = long unsigned;
+void *operator new[](size_t, const UndefinedStruct &) throw() {
+  return nullptr;
+}
+
+CXXRecordDeclRegionBugTest::CXXRecordDeclRegionBugTest(
+    const UndefinedStruct &U) {
+  size_t N;
+  new (U) UserDefinedDefaultCtorStruct[N];
+}
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();
 
@@ -446,25 +444,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(),
-                                                    Context.getStackFrame());
-  // Getting the value for 'this'.
-  SVal This = Context.getState()->getSVal(ThisLoc);
+  Loc ThisLoc =
+      Context.getSValBuilder().getCXXThis(CtorDecl, Context.getStackFrame());
 
-  // 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();
@@ -475,14 +475,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