vendor/llvm-project/llvmorg-16-init-18548-gb0daacf58f41

1 files changed, 368 insertions, 104 deletions

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index f6f71e34b892..cc3992805cc7 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -20,6 +20,7 @@
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cassert>
@@ -48,43 +49,66 @@ llvm::DenseMap<K, V> intersectDenseMaps(const llvm::DenseMap<K, V> &Map1,
   return Result;
 }
 
-static bool areEquivalentIndirectionValues(Value *Val1, Value *Val2) {
-  if (auto *IndVal1 = dyn_cast<ReferenceValue>(Val1)) {
-    auto *IndVal2 = cast<ReferenceValue>(Val2);
-    return &IndVal1->getReferentLoc() == &IndVal2->getReferentLoc();
-  }
-  if (auto *IndVal1 = dyn_cast<PointerValue>(Val1)) {
-    auto *IndVal2 = cast<PointerValue>(Val2);
-    return &IndVal1->getPointeeLoc() == &IndVal2->getPointeeLoc();
+static bool compareDistinctValues(QualType Type, Value &Val1,
+                                  const Environment &Env1, Value &Val2,
+                                  const Environment &Env2,
+                                  Environment::ValueModel &Model) {
+  // Note: Potentially costly, but, for booleans, we could check whether both
+  // can be proven equivalent in their respective environments.
+
+  // FIXME: move the reference/pointers logic from `areEquivalentValues` to here
+  // and implement separate, join/widen specific handling for
+  // reference/pointers.
+  switch (Model.compare(Type, Val1, Env1, Val2, Env2)) {
+  case ComparisonResult::Same:
+    return true;
+  case ComparisonResult::Different:
+    return false;
+  case ComparisonResult::Unknown:
+    switch (Val1.getKind()) {
+    case Value::Kind::Integer:
+    case Value::Kind::Reference:
+    case Value::Kind::Pointer:
+    case Value::Kind::Struct:
+      // FIXME: this choice intentionally introduces unsoundness to allow
+      // for convergence. Once we have widening support for the
+      // reference/pointer and struct built-in models, this should be
+      // `false`.
+      return true;
+    default:
+      return false;
+    }
   }
-  return false;
-}
-
-/// Returns true if and only if `Val1` is equivalent to `Val2`.
-static bool equivalentValues(QualType Type, Value *Val1,
-                             const Environment &Env1, Value *Val2,
-                             const Environment &Env2,
-                             Environment::ValueModel &Model) {
-  return Val1 == Val2 || areEquivalentIndirectionValues(Val1, Val2) ||
-         Model.compareEquivalent(Type, *Val1, Env1, *Val2, Env2);
+  llvm_unreachable("All cases covered in switch");
 }
 
 /// Attempts to merge distinct values `Val1` and `Val2` in `Env1` and `Env2`,
 /// respectively, of the same type `Type`. Merging generally produces a single
 /// value that (soundly) approximates the two inputs, although the actual
 /// meaning depends on `Model`.
-static Value *mergeDistinctValues(QualType Type, Value *Val1,
-                                  const Environment &Env1, Value *Val2,
+static Value *mergeDistinctValues(QualType Type, Value &Val1,
+                                  const Environment &Env1, Value &Val2,
                                   const Environment &Env2,
                                   Environment &MergedEnv,
                                   Environment::ValueModel &Model) {
   // Join distinct boolean values preserving information about the constraints
   // in the respective path conditions.
-  //
-  // FIXME: Does not work for backedges, since the two (or more) paths will not
-  // have mutually exclusive conditions.
-  if (auto *Expr1 = dyn_cast<BoolValue>(Val1)) {
-    auto *Expr2 = cast<BoolValue>(Val2);
+  if (isa<BoolValue>(&Val1) && isa<BoolValue>(&Val2)) {
+    // FIXME: Checking both values should be unnecessary, since they should have
+    // a consistent shape.  However, right now we can end up with BoolValue's in
+    // integer-typed variables due to our incorrect handling of
+    // boolean-to-integer casts (we just propagate the BoolValue to the result
+    // of the cast). So, a join can encounter an integer in one branch but a
+    // bool in the other.
+    // For example:
+    // ```
+    // std::optional<bool> o;
+    // int x;
+    // if (o.has_value())
+    //   x = o.value();
+    // ```
+    auto *Expr1 = cast<BoolValue>(&Val1);
+    auto *Expr2 = cast<BoolValue>(&Val2);
     auto &MergedVal = MergedEnv.makeAtomicBoolValue();
     MergedEnv.addToFlowCondition(MergedEnv.makeOr(
         MergedEnv.makeAnd(Env1.getFlowConditionToken(),
@@ -94,59 +118,99 @@ static Value *mergeDistinctValues(QualType Type, Value *Val1,
     return &MergedVal;
   }
 
-  // FIXME: add unit tests that cover this statement.
-  if (areEquivalentIndirectionValues(Val1, Val2)) {
-    return Val1;
-  }
-
   // FIXME: Consider destroying `MergedValue` immediately if `ValueModel::merge`
   // returns false to avoid storing unneeded values in `DACtx`.
+  // FIXME: Creating the value based on the type alone creates misshapen values
+  // for lvalues, since the type does not reflect the need for `ReferenceValue`.
   if (Value *MergedVal = MergedEnv.createValue(Type))
-    if (Model.merge(Type, *Val1, Env1, *Val2, Env2, *MergedVal, MergedEnv))
+    if (Model.merge(Type, Val1, Env1, Val2, Env2, *MergedVal, MergedEnv))
       return MergedVal;
 
   return nullptr;
 }
 
-/// Initializes a global storage value.
-static void initGlobalVar(const VarDecl &D, Environment &Env) {
-  if (!D.hasGlobalStorage() ||
-      Env.getStorageLocation(D, SkipPast::None) != nullptr)
-    return;
+// When widening does not change `Current`, return value will equal `&Prev`.
+static Value &widenDistinctValues(QualType Type, Value &Prev,
+                                  const Environment &PrevEnv, Value &Current,
+                                  Environment &CurrentEnv,
+                                  Environment::ValueModel &Model) {
+  // Boolean-model widening.
+  if (isa<BoolValue>(&Prev)) {
+    assert(isa<BoolValue>(Current));
+    // Widen to Top, because we know they are different values. If previous was
+    // already Top, re-use that to (implicitly) indicate that no change occured.
+    if (isa<TopBoolValue>(Prev))
+      return Prev;
+    return CurrentEnv.makeTopBoolValue();
+  }
+
+  // FIXME: Add other built-in model widening.
 
-  auto &Loc = Env.createStorageLocation(D);
-  Env.setStorageLocation(D, Loc);
-  if (auto *Val = Env.createValue(D.getType()))
-    Env.setValue(Loc, *Val);
+  // Custom-model widening.
+  if (auto *W = Model.widen(Type, Prev, PrevEnv, Current, CurrentEnv))
+    return *W;
+
+  // Default of widening is a no-op: leave the current value unchanged.
+  return Current;
 }
 
 /// Initializes a global storage value.
-static void initGlobalVar(const Decl &D, Environment &Env) {
+static void insertIfGlobal(const Decl &D,
+                           llvm::DenseSet<const FieldDecl *> &Fields,
+                           llvm::DenseSet<const VarDecl *> &Vars) {
   if (auto *V = dyn_cast<VarDecl>(&D))
-    initGlobalVar(*V, Env);
-}
-
-/// Initializes global storage values that are declared or referenced from
-/// sub-statements of `S`.
-// FIXME: Add support for resetting globals after function calls to enable
-// the implementation of sound analyses.
-static void initGlobalVars(const Stmt &S, Environment &Env) {
-  for (auto *Child : S.children()) {
+    if (V->hasGlobalStorage())
+      Vars.insert(V);
+}
+
+static void getFieldsAndGlobalVars(const Decl &D,
+                                   llvm::DenseSet<const FieldDecl *> &Fields,
+                                   llvm::DenseSet<const VarDecl *> &Vars) {
+  insertIfGlobal(D, Fields, Vars);
+  if (const auto *Decomp = dyn_cast<DecompositionDecl>(&D))
+    for (const auto *B : Decomp->bindings())
+      if (auto *ME = dyn_cast_or_null<MemberExpr>(B->getBinding()))
+        // FIXME: should we be using `E->getFoundDecl()`?
+        if (const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()))
+          Fields.insert(FD);
+}
+
+/// Traverses `S` and inserts into `Vars` any global storage values that are
+/// declared in or referenced from sub-statements.
+static void getFieldsAndGlobalVars(const Stmt &S,
+                                   llvm::DenseSet<const FieldDecl *> &Fields,
+                                   llvm::DenseSet<const VarDecl *> &Vars) {
+  for (auto *Child : S.children())
     if (Child != nullptr)
-      initGlobalVars(*Child, Env);
-  }
+      getFieldsAndGlobalVars(*Child, Fields, Vars);
 
   if (auto *DS = dyn_cast<DeclStmt>(&S)) {
-    if (DS->isSingleDecl()) {
-      initGlobalVar(*DS->getSingleDecl(), Env);
-    } else {
+    if (DS->isSingleDecl())
+      getFieldsAndGlobalVars(*DS->getSingleDecl(), Fields, Vars);
+    else
       for (auto *D : DS->getDeclGroup())
-        initGlobalVar(*D, Env);
-    }
+          getFieldsAndGlobalVars(*D, Fields, Vars);
   } else if (auto *E = dyn_cast<DeclRefExpr>(&S)) {
-    initGlobalVar(*E->getDecl(), Env);
+    insertIfGlobal(*E->getDecl(), Fields, Vars);
   } else if (auto *E = dyn_cast<MemberExpr>(&S)) {
-    initGlobalVar(*E->getMemberDecl(), Env);
+    // FIXME: should we be using `E->getFoundDecl()`?
+    const ValueDecl *VD = E->getMemberDecl();
+    insertIfGlobal(*VD, Fields, Vars);
+    if (const auto *FD = dyn_cast<FieldDecl>(VD))
+      Fields.insert(FD);
+  }
+}
+
+// FIXME: Add support for resetting globals after function calls to enable
+// the implementation of sound analyses.
+void Environment::initVars(llvm::DenseSet<const VarDecl *> Vars) {
+  for (const VarDecl *D : Vars) {
+    if (getStorageLocation(*D, SkipPast::None) != nullptr)
+      continue;
+    auto &Loc = createStorageLocation(*D);
+    setStorageLocation(*D, Loc);
+    if (auto *Val = createValue(D->getType()))
+      setValue(Loc, *Val);
   }
 }
 
@@ -154,9 +218,10 @@ Environment::Environment(DataflowAnalysisContext &DACtx)
     : DACtx(&DACtx), FlowConditionToken(&DACtx.makeFlowConditionToken()) {}
 
 Environment::Environment(const Environment &Other)
-    : DACtx(Other.DACtx), DeclToLoc(Other.DeclToLoc),
-      ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
-      MemberLocToStruct(Other.MemberLocToStruct),
+    : DACtx(Other.DACtx), CallStack(Other.CallStack),
+      ReturnLoc(Other.ReturnLoc), ThisPointeeLoc(Other.ThisPointeeLoc),
+      DeclToLoc(Other.DeclToLoc), ExprToLoc(Other.ExprToLoc),
+      LocToVal(Other.LocToVal), MemberLocToStruct(Other.MemberLocToStruct),
       FlowConditionToken(&DACtx->forkFlowCondition(*Other.FlowConditionToken)) {
 }
 
@@ -169,9 +234,32 @@ Environment &Environment::operator=(const Environment &Other) {
 Environment::Environment(DataflowAnalysisContext &DACtx,
                          const DeclContext &DeclCtx)
     : Environment(DACtx) {
+  CallStack.push_back(&DeclCtx);
+
   if (const auto *FuncDecl = dyn_cast<FunctionDecl>(&DeclCtx)) {
     assert(FuncDecl->getBody() != nullptr);
-    initGlobalVars(*FuncDecl->getBody(), *this);
+
+    llvm::DenseSet<const FieldDecl *> Fields;
+    llvm::DenseSet<const VarDecl *> Vars;
+
+    // Look for global variable references in the constructor-initializers.
+    if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(&DeclCtx)) {
+      for (const auto *Init : CtorDecl->inits()) {
+        if (const auto *M = Init->getAnyMember())
+          Fields.insert(M);
+        const Expr *E = Init->getInit();
+        assert(E != nullptr);
+        getFieldsAndGlobalVars(*E, Fields, Vars);
+      }
+    }
+    getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
+
+    // These have to be added before the lines that follow to ensure that
+    // `create*` work correctly for structs.
+    DACtx.addModeledFields(Fields);
+
+    initVars(Vars);
+
     for (const auto *ParamDecl : FuncDecl->parameters()) {
       assert(ParamDecl != nullptr);
       auto &ParamLoc = createStorageLocation(*ParamDecl);
@@ -179,6 +267,9 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
       if (Value *ParamVal = createValue(ParamDecl->getType()))
         setValue(ParamLoc, *ParamVal);
     }
+
+    QualType ReturnType = FuncDecl->getReturnType();
+    ReturnLoc = &createStorageLocation(ReturnType);
   }
 
   if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(&DeclCtx)) {
@@ -187,59 +278,132 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
     if (Parent->isLambda())
       MethodDecl = dyn_cast<CXXMethodDecl>(Parent->getDeclContext());
 
+    // FIXME: Initialize the ThisPointeeLoc of lambdas too.
     if (MethodDecl && !MethodDecl->isStatic()) {
       QualType ThisPointeeType = MethodDecl->getThisObjectType();
-      // FIXME: Add support for union types.
-      if (!ThisPointeeType->isUnionType()) {
-        auto &ThisPointeeLoc = createStorageLocation(ThisPointeeType);
-        DACtx.setThisPointeeStorageLocation(ThisPointeeLoc);
-        if (Value *ThisPointeeVal = createValue(ThisPointeeType))
-          setValue(ThisPointeeLoc, *ThisPointeeVal);
-      }
+      ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
+      if (Value *ThisPointeeVal = createValue(ThisPointeeType))
+        setValue(*ThisPointeeLoc, *ThisPointeeVal);
     }
   }
 }
 
+bool Environment::canDescend(unsigned MaxDepth,
+                             const DeclContext *Callee) const {
+  return CallStack.size() <= MaxDepth && !llvm::is_contained(CallStack, Callee);
+}
+
 Environment Environment::pushCall(const CallExpr *Call) const {
   Environment Env(*this);
 
-  // FIXME: Currently this only works if the callee is never a method and the
-  // same callee is never analyzed from multiple separate callsites. To
-  // generalize this, we'll need to store a "context" field (probably a stack of
-  // `const CallExpr *`s) in the `Environment`, and then change the
-  // `DataflowAnalysisContext` class to hold a map from contexts to "frames",
-  // where each frame stores its own version of what are currently the
-  // `DeclToLoc`, `ExprToLoc`, and `ThisPointeeLoc` fields.
+  // FIXME: Support references here.
+  Env.ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);
 
-  const auto *FuncDecl = Call->getDirectCallee();
-  assert(FuncDecl != nullptr);
-  assert(FuncDecl->getBody() != nullptr);
-  // FIXME: In order to allow the callee to reference globals, we probably need
-  // to call `initGlobalVars` here in some way.
+  if (const auto *MethodCall = dyn_cast<CXXMemberCallExpr>(Call)) {
+    if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
+      if (!isa<CXXThisExpr>(Arg))
+        Env.ThisPointeeLoc = getStorageLocation(*Arg, SkipPast::Reference);
+      // Otherwise (when the argument is `this`), retain the current
+      // environment's `ThisPointeeLoc`.
+    }
+  }
+
+  Env.pushCallInternal(Call->getDirectCallee(),
+                       llvm::ArrayRef(Call->getArgs(), Call->getNumArgs()));
+
+  return Env;
+}
 
-  auto ParamIt = FuncDecl->param_begin();
-  auto ArgIt = Call->arg_begin();
-  auto ArgEnd = Call->arg_end();
+Environment Environment::pushCall(const CXXConstructExpr *Call) const {
+  Environment Env(*this);
+
+  // FIXME: Support references here.
+  Env.ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);
+
+  Env.ThisPointeeLoc = Env.ReturnLoc;
+
+  Env.pushCallInternal(Call->getConstructor(),
+                       llvm::ArrayRef(Call->getArgs(), Call->getNumArgs()));
+
+  return Env;
+}
+
+void Environment::pushCallInternal(const FunctionDecl *FuncDecl,
+                                   ArrayRef<const Expr *> Args) {
+  CallStack.push_back(FuncDecl);
+
+  // FIXME: Share this code with the constructor, rather than duplicating it.
+  llvm::DenseSet<const FieldDecl *> Fields;
+  llvm::DenseSet<const VarDecl *> Vars;
+  // Look for global variable references in the constructor-initializers.
+  if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(FuncDecl)) {
+    for (const auto *Init : CtorDecl->inits()) {
+      if (const auto *M = Init->getAnyMember())
+        Fields.insert(M);
+      const Expr *E = Init->getInit();
+      assert(E != nullptr);
+      getFieldsAndGlobalVars(*E, Fields, Vars);
+    }
+  }
+  getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
+
+  // These have to be added before the lines that follow to ensure that
+  // `create*` work correctly for structs.
+  DACtx->addModeledFields(Fields);
+
+  initVars(Vars);
+
+  const auto *ParamIt = FuncDecl->param_begin();
 
   // FIXME: Parameters don't always map to arguments 1:1; examples include
   // overloaded operators implemented as member functions, and parameter packs.
-  for (; ArgIt != ArgEnd; ++ParamIt, ++ArgIt) {
+  for (unsigned ArgIndex = 0; ArgIndex < Args.size(); ++ParamIt, ++ArgIndex) {
     assert(ParamIt != FuncDecl->param_end());
 
+    const Expr *Arg = Args[ArgIndex];
+    auto *ArgLoc = getStorageLocation(*Arg, SkipPast::Reference);
+    if (ArgLoc == nullptr)
+      continue;
+
     const VarDecl *Param = *ParamIt;
-    const Expr *Arg = *ArgIt;
-    auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
-    assert(ArgLoc != nullptr);
-    Env.setStorageLocation(*Param, *ArgLoc);
+    auto &Loc = createStorageLocation(*Param);
+    setStorageLocation(*Param, Loc);
+
+    QualType ParamType = Param->getType();
+    if (ParamType->isReferenceType()) {
+      auto &Val = takeOwnership(std::make_unique<ReferenceValue>(*ArgLoc));
+      setValue(Loc, Val);
+    } else if (auto *ArgVal = getValue(*ArgLoc)) {
+      setValue(Loc, *ArgVal);
+    } else if (Value *Val = createValue(ParamType)) {
+      setValue(Loc, *Val);
+    }
   }
+}
 
-  return Env;
+void Environment::popCall(const Environment &CalleeEnv) {
+  // We ignore `DACtx` because it's already the same in both. We don't want the
+  // callee's `DeclCtx`, `ReturnLoc` or `ThisPointeeLoc`. We don't bring back
+  // `DeclToLoc` and `ExprToLoc` because we want to be able to later analyze the
+  // same callee in a different context, and `setStorageLocation` requires there
+  // to not already be a storage location assigned. Conceptually, these maps
+  // capture information from the local scope, so when popping that scope, we do
+  // not propagate the maps.
+  this->LocToVal = std::move(CalleeEnv.LocToVal);
+  this->MemberLocToStruct = std::move(CalleeEnv.MemberLocToStruct);
+  this->FlowConditionToken = std::move(CalleeEnv.FlowConditionToken);
 }
 
 bool Environment::equivalentTo(const Environment &Other,
                                Environment::ValueModel &Model) const {
   assert(DACtx == Other.DACtx);
 
+  if (ReturnLoc != Other.ReturnLoc)
+    return false;
+
+  if (ThisPointeeLoc != Other.ThisPointeeLoc)
+    return false;
+
   if (DeclToLoc != Other.DeclToLoc)
     return false;
 
@@ -259,21 +423,91 @@ bool Environment::equivalentTo(const Environment &Other,
       continue;
     assert(It->second != nullptr);
 
-    if (!equivalentValues(Loc->getType(), Val, *this, It->second, Other, Model))
+    if (!areEquivalentValues(*Val, *It->second) &&
+        !compareDistinctValues(Loc->getType(), *Val, *this, *It->second, Other,
+                               Model))
       return false;
   }
 
   return true;
 }
 
+LatticeJoinEffect Environment::widen(const Environment &PrevEnv,
+                                     Environment::ValueModel &Model) {
+  assert(DACtx == PrevEnv.DACtx);
+  assert(ReturnLoc == PrevEnv.ReturnLoc);
+  assert(ThisPointeeLoc == PrevEnv.ThisPointeeLoc);
+  assert(CallStack == PrevEnv.CallStack);
+
+  auto Effect = LatticeJoinEffect::Unchanged;
+
+  // By the API, `PrevEnv` is a previous version of the environment for the same
+  // block, so we have some guarantees about its shape. In particular, it will
+  // be the result of a join or widen operation on previous values for this
+  // block. For `DeclToLoc` and `ExprToLoc`, join guarantees that these maps are
+  // subsets of the maps in `PrevEnv`. So, as long as we maintain this property
+  // here, we don't need change their current values to widen.
+  //
+  // FIXME: `MemberLocToStruct` does not share the above property, because
+  // `join` can cause the map size to increase (when we add fresh data in places
+  // of conflict). Once this issue with join is resolved, re-enable the
+  // assertion below or replace with something that captures the desired
+  // invariant.
+  assert(DeclToLoc.size() <= PrevEnv.DeclToLoc.size());
+  assert(ExprToLoc.size() <= PrevEnv.ExprToLoc.size());
+  // assert(MemberLocToStruct.size() <= PrevEnv.MemberLocToStruct.size());
+
+  llvm::DenseMap<const StorageLocation *, Value *> WidenedLocToVal;
+  for (auto &Entry : LocToVal) {
+    const StorageLocation *Loc = Entry.first;
+    assert(Loc != nullptr);
+
+    Value *Val = Entry.second;
+    assert(Val != nullptr);
+
+    auto PrevIt = PrevEnv.LocToVal.find(Loc);
+    if (PrevIt == PrevEnv.LocToVal.end())
+      continue;
+    assert(PrevIt->second != nullptr);
+
+    if (areEquivalentValues(*Val, *PrevIt->second)) {
+      WidenedLocToVal.insert({Loc, Val});
+      continue;
+    }
+
+    Value &WidenedVal = widenDistinctValues(Loc->getType(), *PrevIt->second,
+                                            PrevEnv, *Val, *this, Model);
+    WidenedLocToVal.insert({Loc, &WidenedVal});
+    if (&WidenedVal != PrevIt->second)
+      Effect = LatticeJoinEffect::Changed;
+  }
+  LocToVal = std::move(WidenedLocToVal);
+  // FIXME: update the equivalence calculation for `MemberLocToStruct`, once we
+  // have a systematic way of soundly comparing this map.
+  if (DeclToLoc.size() != PrevEnv.DeclToLoc.size() ||
+      ExprToLoc.size() != PrevEnv.ExprToLoc.size() ||
+      LocToVal.size() != PrevEnv.LocToVal.size() ||
+      MemberLocToStruct.size() != PrevEnv.MemberLocToStruct.size())
+    Effect = LatticeJoinEffect::Changed;
+
+  return Effect;
+}
+
 LatticeJoinEffect Environment::join(const Environment &Other,
                                     Environment::ValueModel &Model) {
   assert(DACtx == Other.DACtx);
+  assert(ReturnLoc == Other.ReturnLoc);
+  assert(ThisPointeeLoc == Other.ThisPointeeLoc);
+  assert(CallStack == Other.CallStack);
 
   auto Effect = LatticeJoinEffect::Unchanged;
 
   Environment JoinedEnv(*DACtx);
 
+  JoinedEnv.CallStack = CallStack;
+  JoinedEnv.ReturnLoc = ReturnLoc;
+  JoinedEnv.ThisPointeeLoc = ThisPointeeLoc;
+
   JoinedEnv.DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
   if (DeclToLoc.size() != JoinedEnv.DeclToLoc.size())
     Effect = LatticeJoinEffect::Changed;
@@ -288,6 +522,8 @@ LatticeJoinEffect Environment::join(const Environment &Other,
     Effect = LatticeJoinEffect::Changed;
 
   // FIXME: set `Effect` as needed.
+  // FIXME: update join to detect backedges and simplify the flow condition
+  // accordingly.
   JoinedEnv.FlowConditionToken = &DACtx->joinFlowConditions(
       *FlowConditionToken, *Other.FlowConditionToken);
 
@@ -303,14 +539,17 @@ LatticeJoinEffect Environment::join(const Environment &Other,
       continue;
     assert(It->second != nullptr);
 
-    if (Val == It->second) {
+    if (areEquivalentValues(*Val, *It->second)) {
       JoinedEnv.LocToVal.insert({Loc, Val});
       continue;
     }
 
-    if (Value *MergedVal = mergeDistinctValues(
-            Loc->getType(), Val, *this, It->second, Other, JoinedEnv, Model))
+    if (Value *MergedVal =
+            mergeDistinctValues(Loc->getType(), *Val, *this, *It->second, Other,
+                                JoinedEnv, Model)) {
       JoinedEnv.LocToVal.insert({Loc, MergedVal});
+      Effect = LatticeJoinEffect::Changed;
+    }
   }
   if (LocToVal.size() != JoinedEnv.LocToVal.size())
     Effect = LatticeJoinEffect::Changed;
@@ -321,7 +560,7 @@ LatticeJoinEffect Environment::join(const Environment &Other,
 }
 
 StorageLocation &Environment::createStorageLocation(QualType Type) {
-  return DACtx->getStableStorageLocation(Type);
+  return DACtx->createStorageLocation(Type);
 }
 
 StorageLocation &Environment::createStorageLocation(const VarDecl &D) {
@@ -363,7 +602,11 @@ StorageLocation *Environment::getStorageLocation(const Expr &E,
 }
 
 StorageLocation *Environment::getThisPointeeStorageLocation() const {
-  return DACtx->getThisPointeeStorageLocation();
+  return ThisPointeeLoc;
+}
+
+StorageLocation *Environment::getReturnStorageLocation() const {
+  return ReturnLoc;
 }
 
 PointerValue &Environment::getOrCreateNullPointerValue(QualType PointeeType) {
@@ -377,9 +620,9 @@ void Environment::setValue(const StorageLocation &Loc, Value &Val) {
     auto &AggregateLoc = *cast<AggregateStorageLocation>(&Loc);
 
     const QualType Type = AggregateLoc.getType();
-    assert(Type->isStructureOrClassType());
+    assert(Type->isStructureOrClassType() || Type->isUnionType());
 
-    for (const FieldDecl *Field : getObjectFields(Type)) {
+    for (const FieldDecl *Field : DACtx->getReferencedFields(Type)) {
       assert(Field != nullptr);
       StorageLocation &FieldLoc = AggregateLoc.getChild(*Field);
       MemberLocToStruct[&FieldLoc] = std::make_pair(StructVal, Field);
@@ -450,6 +693,9 @@ Value *Environment::createValueUnlessSelfReferential(
   }
 
   if (Type->isIntegerType()) {
+    // FIXME: consider instead `return nullptr`, given that we do nothing useful
+    // with integers, and so distinguishing them serves no purpose, but could
+    // prevent convergence.
     CreatedValuesCount++;
     return &takeOwnership(std::make_unique<IntegerValue>());
   }
@@ -488,12 +734,10 @@ Value *Environment::createValueUnlessSelfReferential(
     return &takeOwnership(std::make_unique<PointerValue>(PointeeLoc));
   }
 
-  if (Type->isStructureOrClassType()) {
+  if (Type->isStructureOrClassType() || Type->isUnionType()) {
     CreatedValuesCount++;
-    // FIXME: Initialize only fields that are accessed in the context that is
-    // being analyzed.
     llvm::DenseMap<const ValueDecl *, Value *> FieldValues;
-    for (const FieldDecl *Field : getObjectFields(Type)) {
+    for (const FieldDecl *Field : DACtx->getReferencedFields(Type)) {
       assert(Field != nullptr);
 
       QualType FieldType = Field->getType();
@@ -546,9 +790,29 @@ bool Environment::flowConditionImplies(BoolValue &Val) const {
   return DACtx->flowConditionImplies(*FlowConditionToken, Val);
 }
 
-void Environment::dump() const {
+void Environment::dump(raw_ostream &OS) const {
+  // FIXME: add printing for remaining fields and allow caller to decide what
+  // fields are printed.
+  OS << "DeclToLoc:\n";
+  for (auto [D, L] : DeclToLoc)
+    OS << "  [" << D->getName() << ", " << L << "]\n";
+
+  OS << "ExprToLoc:\n";
+  for (auto [E, L] : ExprToLoc)
+    OS << "  [" << E << ", " << L << "]\n";
+
+  OS << "LocToVal:\n";
+  for (auto [L, V] : LocToVal) {
+    OS << "  [" << L << ", " << V << ": " << *V << "]\n";
+  }
+
+  OS << "FlowConditionToken:\n";
   DACtx->dumpFlowCondition(*FlowConditionToken);
 }
 
+void Environment::dump() const {
+  dump(llvm::dbgs());
+}
+
 } // namespace dataflow
 } // namespace clang