[clang] facad21 - [Analyzer] Fix for `ExprEngine::computeObjectUnderConstruction()` for base and delegating consturctor initializers

[Adam Balogh via cfe-commits]

Author: Adam Balogh
Date: 2020-09-25T13:28:22+02:00
New Revision: facad21b29839a08fdf448eb4dd5a4e31e293b9b

URL: https://github.com/llvm/llvm-project/commit/facad21b29839a08fdf448eb4dd5a4e31e293b9b
DIFF: https://github.com/llvm/llvm-project/commit/facad21b29839a08fdf448eb4dd5a4e31e293b9b.diff

LOG: [Analyzer] Fix for `ExprEngine::computeObjectUnderConstruction()` for base and delegating consturctor initializers

For /C++/ constructor initializers `ExprEngine:computeUnderConstruction()`
asserts that they are all member initializers. This is not neccessarily
true when this function is used to get the return value for the
construction context thus attempts to fetch return values of base and
delegating constructor initializers result in assertions. This small
patch fixes this issue.

Differential Revision: https://reviews.llvm.org/D85351

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
    clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 802bc934cfb06..953a8ef58b447 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -132,10 +132,20 @@ SVal ExprEngine::computeObjectUnderConstruction(
     case ConstructionContext::SimpleConstructorInitializerKind: {
       const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC);
       const auto *Init = ICC->getCXXCtorInitializer();
-      assert(Init->isAnyMemberInitializer());
       const CXXMethodDecl *CurCtor = cast<CXXMethodDecl>(LCtx->getDecl());
       Loc ThisPtr = SVB.getCXXThis(CurCtor, LCtx->getStackFrame());
       SVal ThisVal = State->getSVal(ThisPtr);
+      if (Init->isBaseInitializer()) {
+        const auto *ThisReg = cast<SubRegion>(ThisVal.getAsRegion());
+        const CXXRecordDecl *BaseClass =
+          Init->getBaseClass()->getAsCXXRecordDecl();
+        const auto *BaseReg =
+          MRMgr.getCXXBaseObjectRegion(BaseClass, ThisReg,
+                                       Init->isBaseVirtual());
+        return SVB.makeLoc(BaseReg);
+      }
+      if (Init->isDelegatingInitializer())
+        return ThisVal;
 
       const ValueDecl *Field;
       SVal FieldVal;
@@ -364,6 +374,11 @@ ProgramStateRef ExprEngine::updateObjectsUnderConstruction(
     case ConstructionContext::CXX17ElidedCopyConstructorInitializerKind:
     case ConstructionContext::SimpleConstructorInitializerKind: {
       const auto *ICC = cast<ConstructorInitializerConstructionContext>(CC);
+      const auto *Init = ICC->getCXXCtorInitializer();
+      // Base and delegating initializers handled above
+      assert(Init->isAnyMemberInitializer() &&
+             "Base and delegating initializers should have been handled by"
+             "computeObjectUnderConstruction()");
       return addObjectUnderConstruction(State, ICC->getCXXCtorInitializer(),
                                         LCtx, V);
     }

diff  --git a/clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp b/clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
index 5750d5918db32..eb0ee6c1fd8a0 100644
--- a/clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
+++ b/clang/unittests/StaticAnalyzer/TestReturnValueUnderConstruction.cpp
@@ -23,8 +23,8 @@ class TestReturnValueUnderConstructionChecker
   : public Checker<check::PostCall> {
 public:
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const {
-    // Only calls with origin expression are checked. These are `returnC()`
-    // and C::C().
+    // Only calls with origin expression are checked. These are `returnC()`,
+    // `returnD()`, C::C() and D::D().
     if (!Call.getOriginExpr())
       return;
 
@@ -35,6 +35,10 @@ class TestReturnValueUnderConstructionChecker
     Optional<SVal> RetVal = Call.getReturnValueUnderConstruction();
     ASSERT_TRUE(RetVal);
     ASSERT_TRUE(RetVal->getAsRegion());
+
+    const auto *RetReg = cast<TypedValueRegion>(RetVal->getAsRegion());
+    const Expr *OrigExpr = Call.getOriginExpr();
+    ASSERT_EQ(OrigExpr->getType(), RetReg->getValueType());
   }
 };
 
@@ -51,22 +55,65 @@ void addTestReturnValueUnderConstructionChecker(
 TEST(TestReturnValueUnderConstructionChecker,
      ReturnValueUnderConstructionChecker) {
   EXPECT_TRUE(runCheckerOnCode<addTestReturnValueUnderConstructionChecker>(
-                  R"(class C {
-                     public:
-                       C(int nn): n(nn) {}
-                       virtual ~C() {}
-                     private:
-                       int n;
-                     };
-
-                     C returnC(int m) {
-                       C c(m);
-                       return c;
-                     }
-
-                     void foo() {
-                       C c = returnC(1); 
-                     })"));
+      R"(class C {
+         public:
+           C(int nn): n(nn) {}
+           virtual ~C() {}
+         private:
+           int n;
+         };
+
+         C returnC(int m) {
+           C c(m);
+           return c;
+         }
+
+         void foo() {
+           C c = returnC(1);
+         })"));
+
+  EXPECT_TRUE(runCheckerOnCode<addTestReturnValueUnderConstructionChecker>(
+      R"(class C {
+         public:
+           C(int nn): n(nn) {}
+           explicit C(): C(0) {}
+           virtual ~C() {}
+         private:
+           int n;
+         };
+
+         C returnC() {
+           C c;
+           return c;
+         }
+
+         void foo() {
+           C c = returnC();
+         })"));
+
+  EXPECT_TRUE(runCheckerOnCode<addTestReturnValueUnderConstructionChecker>(
+      R"(class C {
+         public:
+           C(int nn): n(nn) {}
+           virtual ~C() {}
+         private:
+           int n;
+         };
+
+         class D: public C {
+         public:
+           D(int nn): C(nn) {}
+           virtual ~D() {}
+         };
+
+         D returnD(int m) {
+           D d(m);
+           return d;
+         }
+
+         void foo() {
+           D d = returnD(1); 
+         })"));
 }
 
 } // namespace