[clang][dataflow] Store DeclContext of block being analysed in Environment if available.

[wyt]

Summary

Allows propagation of information from the DeclContext into child nodes.

The use case which motivated this patch is the verification of the compatibility between the declared return type and the actual return type in a pointer nullability analysis. For example, we should not be returning a nullable pointer in a function which has _Nonnull annotated return type.
To retrieve the declared return type, we need to keep track of the FuncDecl (which is a DeclCtx) of the function body being analysed.

[sgatev]

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

175

387

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

171–175

[samestep]

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

213

I think you need to set the DeclCtx field here (or at least, somewhere in this method).

[wyt]

Update DeclCtx when pushing/popping calls.

[wyt]

Formatting fixes.

[wyt]

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

171–175

Unfortunately doesn't work, I get a warning about "An initializer for a delegating constructor must appear alone".

[sgatev]

clang/lib/Analysis/FlowSensitive/Transfer.cpp

537–539 ↗

(On Diff #449666)

I believe you'll need to rebase because this has changed: https://github.com/llvm/llvm-project/blob/main/clang/lib/Analysis/FlowSensitive/Transfer.cpp#L567

[wyt]

Rebase to head.

[wyt]

Relocate function and fix comment for consistency.

[eugenis]

MSan is not happy:
https://lab.llvm.org/buildbot/#/builders/74/builds/12717

[eugenis]

Reverted.

[wyt]

Initialise DeclCtx field with nullptr.

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

public:
   /// merged using `Model`.
   ///
   /// Requirements:
   ///
   ///  `Other` and `this` must use the same `DataflowAnalysisContext`.
   LatticeJoinEffect join(const Environment &Other,
                          Environment::ValueModel &Model);
 
   // FIXME: Rename `createOrGetStorageLocation` to `getOrCreateStorageLocation`,
   // `getStableStorageLocation`, or something more appropriate.
 
   /// Creates a storage location appropriate for `Type`. Does not assign a value
   /// to the returned storage location in the environment.
   ///
   /// Requirements:
   ///
   ///  `Type` must not be null.
public:
 
   /// Adds `Val` to the set of clauses that constitute the flow condition.
   void addToFlowCondition(BoolValue &Val);
 
   /// Returns true if and only if the clauses that constitute the flow condition
   /// imply that `Val` is true.
   bool flowConditionImplies(BoolValue &Val) const;
 
+  /// Returns the `DeclContext` of the block being analysed, if any. Otherwise,
+  /// returns null.
+  const DeclContext *getDeclCtx() { return DeclCtx; }
+
+  /// Sets the `DeclContext` of the block being analysed.
+  void setDeclCtx(const DeclContext *Ctx) { DeclCtx = Ctx; }
+
   /// Returns the `ControlFlowContext` registered for `F`, if any. Otherwise,
   /// returns null.
   const ControlFlowContext *getControlFlowContext(const FunctionDecl *F) {
     return DACtx->getControlFlowContext(F);
   }
 
   LLVM_DUMP_METHOD void dump() const;
 
Value *createValueUnlessSelfReferential(QualType Type,
                                           int Depth, int &CreatedValuesCount);
 
   StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const;
   const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;
 
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 
+  // `DeclContext` of the block being analysed if provided.
+  const DeclContext *DeclCtx = nullptr;
+
   // In a properly initialized `Environment`, `ReturnLoc` should only be null if
   // its `DeclContext` could not be cast to a `FunctionDecl`.
   StorageLocation *ReturnLoc = nullptr;
   // The storage location of the `this` pointee. Should only be null if the
   // function being analyzed is only a function and not a method.
   StorageLocation *ThisPointeeLoc = nullptr;
 
   // Maps from program declarations and statements to storage locations that are

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

if (auto *DS = dyn_cast<DeclStmt>(&S)) {
     initGlobalVar(*E->getMemberDecl(), Env);
   }
 }
 
 Environment::Environment(DataflowAnalysisContext &DACtx)
     : DACtx(&DACtx), FlowConditionToken(&DACtx.makeFlowConditionToken()) {}
 
 Environment::Environment(const Environment &Other)
-    : DACtx(Other.DACtx), ReturnLoc(Other.ReturnLoc),
+    : DACtx(Other.DACtx), DeclCtx(Other.DeclCtx), ReturnLoc(Other.ReturnLoc),
       ThisPointeeLoc(Other.ThisPointeeLoc), DeclToLoc(Other.DeclToLoc),
       ExprToLoc(Other.ExprToLoc), LocToVal(Other.LocToVal),
       MemberLocToStruct(Other.MemberLocToStruct),
       FlowConditionToken(&DACtx->forkFlowCondition(*Other.FlowConditionToken)) {
 }
 
 Environment &Environment::operator=(const Environment &Other) {
   Environment Copy(Other);
   *this = std::move(Copy);
   return *this;
 }
 
 Environment::Environment(DataflowAnalysisContext &DACtx,
-                         const DeclContext &DeclCtx)
+                         const DeclContext &DeclCtxArg)
     : Environment(DACtx) {
-  if (const auto *FuncDecl = dyn_cast<FunctionDecl>(&DeclCtx)) {
+  setDeclCtx(&DeclCtxArg);
+
+  if (const auto *FuncDecl = dyn_cast<FunctionDecl>(DeclCtx)) {
     assert(FuncDecl->getBody() != nullptr);
     initGlobalVars(*FuncDecl->getBody(), *this);
     for (const auto *ParamDecl : FuncDecl->parameters()) {
       assert(ParamDecl != nullptr);
       auto &ParamLoc = createStorageLocation(*ParamDecl);
       setStorageLocation(*ParamDecl, ParamLoc);
       if (Value *ParamVal = createValue(ParamDecl->getType()))
         setValue(ParamLoc, *ParamVal);
     }
 
     QualType ReturnType = FuncDecl->getReturnType();
     ReturnLoc = &createStorageLocation(ReturnType);
   }
 
-  if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(&DeclCtx)) {
+  if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(DeclCtx)) {
     auto *Parent = MethodDecl->getParent();
     assert(Parent != nullptr);
     if (Parent->isLambda())
       MethodDecl = dyn_cast<CXXMethodDecl>(Parent->getDeclContext());
 
     if (MethodDecl && !MethodDecl->isStatic()) {
       QualType ThisPointeeType = MethodDecl->getThisObjectType();
       // FIXME: Add support for union types.
       if (!ThisPointeeType->isUnionType()) {
         ThisPointeeLoc = &createStorageLocation(ThisPointeeType);
         if (Value *ThisPointeeVal = createValue(ThisPointeeType))
           setValue(*ThisPointeeLoc, *ThisPointeeVal);
       }
     }
   }
 }
 
 Environment Environment::pushCall(const CallExpr *Call) const {
   Environment Env(*this);
   // FIXME: Support references here.
   Env.ReturnLoc = Env.getStorageLocation(*Call, SkipPast::Reference);
 
   const auto *FuncDecl = Call->getDirectCallee();
   assert(FuncDecl != nullptr);
+
+  Env.setDeclCtx(FuncDecl);
+
   // 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()) {
       Env.ThisPointeeLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
     }
   }
for (; ArgIt != ArgEnd; ++ParamIt, ++ArgIt) {
     }
   }
 
   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 `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.
+  // 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);
bool Environment::equivalentTo(const Environment &Other,
   return true;
 }
 
 LatticeJoinEffect Environment::join(const Environment &Other,
                                     Environment::ValueModel &Model) {
   assert(DACtx == Other.DACtx);
   assert(ReturnLoc == Other.ReturnLoc);
   assert(ThisPointeeLoc == Other.ThisPointeeLoc);
+  assert(DeclCtx == Other.DeclCtx);
 
   auto Effect = LatticeJoinEffect::Unchanged;
 
   Environment JoinedEnv(*DACtx);
 
+  JoinedEnv.setDeclCtx(DeclCtx);
   JoinedEnv.ReturnLoc = ReturnLoc;
   JoinedEnv.ThisPointeeLoc = ThisPointeeLoc;
 
   JoinedEnv.DeclToLoc = intersectDenseMaps(DeclToLoc, Other.DeclToLoc);
   if (DeclToLoc.size() != JoinedEnv.DeclToLoc.size())
     Effect = LatticeJoinEffect::Changed;
 
   JoinedEnv.ExprToLoc = intersectDenseMaps(ExprToLoc, Other.ExprToLoc);