[clang][dataflow] Handle when this refers to a different location

clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

@@ -18,6 +18,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/Type.h"
 #include "clang/Analysis/FlowSensitive/ASTOps.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
@@ -351,10 +352,34 @@ class Environment {
   /// Returns the storage location assigned to the `this` pointee in the
   /// environment or null if the `this` pointee has no assigned storage location
   /// in the environment.
+  /// If you want to look up the storage location for a specific `CXXThisExpr`,
+  /// use the overload that takes a `CXXThisExpr`.
   RecordStorageLocation *getThisPointeeStorageLocation() const {
     return ThisPointeeLoc;
   }
 
+  /// Returns the storage location assigned to the `this` pointee in the
+  /// environment given a specific `CXXThisExpr`. Returns null if the `this`
+  /// pointee has no assigned storage location in the environment.
+  /// Note that `this` can be used in a non-member context, e.g.:
+  ///
+  /// \code
+  ///   struct S {
+  ///     int x;
+  ///     int y = this->x;
+  ///   };
+  ///   int foo() {
+  ///     return S{10}.y;  // will have a `this` for initializing `S::y`.
+  ///   }
+  /// \endcode
+  RecordStorageLocation *
+  getThisPointeeStorageLocation(const CXXThisExpr &ThisExpr) const {
+    auto It = ThisExprOverrides->find(&ThisExpr);
+    if (It == ThisExprOverrides->end())
+      return ThisPointeeLoc;
+    return It->second;
+  }
+
   /// Sets the storage location assigned to the `this` pointee in the
   /// environment.
   void setThisPointeeStorageLocation(RecordStorageLocation &Loc) {
@@ -684,6 +709,8 @@ class Environment {
 private:
   using PrValueToResultObject =
       llvm::DenseMap<const Expr *, RecordStorageLocation *>;
+  using ThisExprOverridesMap =
+      llvm::DenseMap<const CXXThisExpr *, RecordStorageLocation *>;
 
   // The copy-constructor is for use in fork() only.
   Environment(const Environment &) = default;
@@ -747,6 +774,15 @@ class Environment {
                        RecordStorageLocation *ThisPointeeLoc,
                        RecordStorageLocation *LocForRecordReturnVal);
 
+  static ThisExprOverridesMap
+  buildThisExprOverridesMap(const FunctionDecl *FuncDecl,
+                            RecordStorageLocation *ThisPointeeLoc,
+                            const PrValueToResultObject &ResultObjectMap);
+
+  static ThisExprOverridesMap
+  buildThisExprOverridesMap(Stmt *S, RecordStorageLocation *ThisPointeeLoc,
+                            const PrValueToResultObject &ResultObjectMap);
+
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 
@@ -793,6 +829,11 @@ class Environment {
   // analysis target is not a method.
   RecordStorageLocation *ThisPointeeLoc = nullptr;
 
+  // Maps from `CXXThisExpr`s to their storage locations, if it should be
+  // different from `ThisPointeeLoc` (for example, CXXThisExpr that are
+  // under a CXXDefaultInitExpr under an InitListExpr).
+  std::shared_ptr<ThisExprOverridesMap> ThisExprOverrides;
+
   // Maps from declarations and glvalue expression to storage locations that are
   // assigned to them. Unlike the maps in `DataflowAnalysisContext`, these
   // include only storage locations that are in scope for a particular basic

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

@@ -15,12 +15,14 @@
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/Type.h"
 #include "clang/Analysis/FlowSensitive/ASTOps.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -30,6 +32,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include <cassert>
 #include <memory>
+#include <stack>
 #include <utility>
 
 #define DEBUG_TYPE "dataflow"
@@ -486,6 +489,97 @@ class ResultObjectVisitor : public AnalysisASTVisitor {
   DataflowAnalysisContext &DACtx;
 };
 
+/// A visitor that finds `CXXThisExpr` that can refer to an object other than
+/// the `this` of a member function.
+class ThisExprOverridesVisitor : public AnalysisASTVisitor {
+  using BaseVisitor = AnalysisASTVisitor;
+
+public:
+  ThisExprOverridesVisitor(
+      RecordStorageLocation *ThisPointeeLoc,
+      const llvm::DenseMap<const Expr *, RecordStorageLocation *>
+          &ResultObjectMap,
+      llvm::DenseMap<const CXXThisExpr *, RecordStorageLocation *>
+          &ThisExprOverrides)
+      : DefaultThisPointeeLoc(ThisPointeeLoc), ResultObjectMap(ResultObjectMap),
+        ThisExprOverrides(ThisExprOverrides) {
+    ThisLocations.push(DefaultThisPointeeLoc);
+  }
+
+  void traverseConstructorInits(const CXXConstructorDecl *Ctor) {
+    for (const CXXCtorInitializer *Init : Ctor->inits()) {
+      TraverseStmt(Init->getInit());
+    }
+  }
+
+  bool TraverseInitListExpr(InitListExpr *ILE) override {
+    if (!ILE->isSemanticForm() || ILE->isTransparent()) {
+      BaseVisitor::TraverseInitListExpr(ILE);
+      return true;
+    }
+    bool IsRecordType = ILE->getType()->isRecordType();
+    if (IsRecordType) {
+      auto It = ResultObjectMap.find(ILE);
+      if (It == ResultObjectMap.end()) {
+        llvm_unreachable("InitListExpr not found in ResultObjectMap");
+        return false;
+      }
+      InitListLocations.push(It->second);
+    }
+    BaseVisitor::TraverseInitListExpr(ILE);
+    if (IsRecordType)
+      InitListLocations.pop();
+    return true;
+  }
+
+  bool TraverseCXXParenListInitExpr(CXXParenListInitExpr *PLIE) override {
+    auto It = ResultObjectMap.find(PLIE);
+    if (It == ResultObjectMap.end()) {
+      llvm_unreachable("CXXParenListInitExpr not found in ResultObjectMap");
+      return false;
+    }
+    InitListLocations.push(It->second);
+    BaseVisitor::TraverseCXXParenListInitExpr(PLIE);
+    InitListLocations.pop();
+    return true;
+  }
+
+  bool TraverseCXXDefaultInitExpr(CXXDefaultInitExpr *CDIE) override {
+    bool HasInitListLocations = !InitListLocations.empty();
+    if (HasInitListLocations) {
+      auto *Loc = InitListLocations.top();
+      ThisLocations.push(Loc);
+    }
+    BaseVisitor::TraverseCXXDefaultInitExpr(CDIE);
+    if (HasInitListLocations)
+      ThisLocations.pop();
+    return true;
+  }
+
+  bool TraverseCXXThisExpr(CXXThisExpr *This) override {
+    assert(!ThisLocations.empty());
+    auto *Loc = ThisLocations.top();
+    if (Loc != DefaultThisPointeeLoc)
+      ThisExprOverrides[This] = Loc;
+    return true;
+  }
+
+  // The default `this` pointee location (null if not in a member function).
+  RecordStorageLocation *DefaultThisPointeeLoc;
+  // Locations to use for `this`, with the most recent scope on top.
+  std::stack<RecordStorageLocation *> ThisLocations;
+  // A stack of nested InitListExpr and CXXParenListInitExprs storage
+  // locations that may be used for `this` if we enter a CXXDefaultInitExpr.
+  std::stack<RecordStorageLocation *> InitListLocations;
+  // Map to look up a storage location, e.g., when encountering an
+  // InitListExpr.
+  const llvm::DenseMap<const Expr *, RecordStorageLocation *> &ResultObjectMap;
+  // The visitor will update this map with locations to use for `this`,
+  // if different from `DefaultThisPointeeLoc`.
+  llvm::DenseMap<const CXXThisExpr *, RecordStorageLocation *>
+      &ThisExprOverrides;
+};
+
 } // namespace
 
 void Environment::initialize() {
@@ -498,6 +592,11 @@ void Environment::initialize() {
         std::make_shared<PrValueToResultObject>(buildResultObjectMap(
             DACtx, InitialTargetStmt, getThisPointeeStorageLocation(),
             /*LocForRecordReturnValue=*/nullptr));
+
+    ThisExprOverrides =
+        std::make_shared<ThisExprOverridesMap>(buildThisExprOverridesMap(
+            InitialTargetStmt, getThisPointeeStorageLocation(),
+            *ResultObjectMap));
     return;
   }
 
@@ -559,6 +658,11 @@ void Environment::initialize() {
       std::make_shared<PrValueToResultObject>(buildResultObjectMap(
           DACtx, InitialTargetFunc, getThisPointeeStorageLocation(),
           LocForRecordReturnVal));
+
+  ThisExprOverrides =
+      std::make_shared<ThisExprOverridesMap>(buildThisExprOverridesMap(
+          InitialTargetFunc, getThisPointeeStorageLocation(),
+          *ResultObjectMap));
 }
 
 // FIXME: Add support for resetting globals after function calls to enable the
@@ -659,6 +763,9 @@ void Environment::pushCallInternal(const FunctionDecl *FuncDecl,
   ResultObjectMap = std::make_shared<PrValueToResultObject>(
       buildResultObjectMap(DACtx, FuncDecl, getThisPointeeStorageLocation(),
                            LocForRecordReturnVal));
+  ThisExprOverrides =
+      std::make_shared<ThisExprOverridesMap>(buildThisExprOverridesMap(
+          FuncDecl, getThisPointeeStorageLocation(), *ResultObjectMap));
 }
 
 void Environment::popCall(const CallExpr *Call, const Environment &CalleeEnv) {
@@ -726,6 +833,7 @@ LatticeEffect Environment::widen(const Environment &PrevEnv,
   assert(ReturnLoc == PrevEnv.ReturnLoc);
   assert(LocForRecordReturnVal == PrevEnv.LocForRecordReturnVal);
   assert(ThisPointeeLoc == PrevEnv.ThisPointeeLoc);
+  assert(ThisExprOverrides == PrevEnv.ThisExprOverrides);
   assert(CallStack == PrevEnv.CallStack);
   assert(ResultObjectMap == PrevEnv.ResultObjectMap);
   assert(InitialTargetFunc == PrevEnv.InitialTargetFunc);
@@ -763,6 +871,7 @@ Environment Environment::join(const Environment &EnvA, const Environment &EnvB,
   assert(EnvA.DACtx == EnvB.DACtx);
   assert(EnvA.LocForRecordReturnVal == EnvB.LocForRecordReturnVal);
   assert(EnvA.ThisPointeeLoc == EnvB.ThisPointeeLoc);
+  assert(EnvA.ThisExprOverrides == EnvB.ThisExprOverrides);
   assert(EnvA.CallStack == EnvB.CallStack);
   assert(EnvA.ResultObjectMap == EnvB.ResultObjectMap);
   assert(EnvA.InitialTargetFunc == EnvB.InitialTargetFunc);
@@ -774,6 +883,7 @@ Environment Environment::join(const Environment &EnvA, const Environment &EnvB,
   JoinedEnv.ResultObjectMap = EnvA.ResultObjectMap;
   JoinedEnv.LocForRecordReturnVal = EnvA.LocForRecordReturnVal;
   JoinedEnv.ThisPointeeLoc = EnvA.ThisPointeeLoc;
+  JoinedEnv.ThisExprOverrides = EnvA.ThisExprOverrides;
   JoinedEnv.InitialTargetFunc = EnvA.InitialTargetFunc;
   JoinedEnv.InitialTargetStmt = EnvA.InitialTargetStmt;
 
@@ -1225,6 +1335,30 @@ Environment::PrValueToResultObject Environment::buildResultObjectMap(
   return Map;
 }
 
+Environment::ThisExprOverridesMap Environment::buildThisExprOverridesMap(
+    const FunctionDecl *FuncDecl, RecordStorageLocation *ThisPointeeLoc,
+    const PrValueToResultObject &ResultObjectMap) {
+  assert(FuncDecl->doesThisDeclarationHaveABody());
+
+  ThisExprOverridesMap Map = buildThisExprOverridesMap(
+      FuncDecl->getBody(), ThisPointeeLoc, ResultObjectMap);
+
+  ThisExprOverridesVisitor Visitor(ThisPointeeLoc, ResultObjectMap, Map);
+  if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(FuncDecl)) {
+    Visitor.traverseConstructorInits(Ctor);
+  }
+  return Map;
+}
+
+Environment::ThisExprOverridesMap Environment::buildThisExprOverridesMap(
+    Stmt *S, RecordStorageLocation *ThisPointeeLoc,
+    const PrValueToResultObject &ResultObjectMap) {
+  ThisExprOverridesMap Map;
+  ThisExprOverridesVisitor Visitor(ThisPointeeLoc, ResultObjectMap, Map);
+  Visitor.TraverseStmt(S);
+  return Map;
+}
+
 RecordStorageLocation *getImplicitObjectLocation(const CXXMemberCallExpr &MCE,
                                                  const Environment &Env) {
   Expr *ImplicitObject = MCE.getImplicitObjectArgument();

clang/lib/Analysis/FlowSensitive/Transfer.cpp

@@ -419,7 +419,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
   }
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {
-    auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
+    auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation(*S);
     if (ThisPointeeLoc == nullptr)
       // Unions are not supported yet, and will not have a location for the
       // `this` expression's pointee.