diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index 07af485e786c8..dc82ccea4a27d 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -394,6 +394,32 @@ class Environment { /// `Type` must not be null. Value *createValue(QualType Type); + /// Creates an object (i.e. a storage location with an associated value) of + /// type `Ty`. If `InitExpr` is non-null and has a value associated with it, + /// initializes the object with this value. Otherwise, initializes the object + /// with a value created using `createValue()`. + StorageLocation &createObject(QualType Ty, const Expr *InitExpr = nullptr) { + return createObjectInternal(nullptr, Ty, InitExpr); + } + + /// Creates an object for the variable declaration `D`. If `D` has an + /// initializer and this initializer is associated with a value, initializes + /// the object with this value. Otherwise, initializes the object with a + /// value created using `createValue()`. Uses the storage location returned by + /// `DataflowAnalysisContext::getStableStorageLocation(D)`. + StorageLocation &createObject(const VarDecl &D) { + return createObjectInternal(&D, D.getType(), D.getInit()); + } + + /// Creates an object for the variable declaration `D`. If `InitExpr` is + /// non-null and has a value associated with it, initializes the object with + /// this value. Otherwise, initializes the object with a value created using + /// `createValue()`. Uses the storage location returned by + /// `DataflowAnalysisContext::getStableStorageLocation(D)`. + StorageLocation &createObject(const VarDecl &D, const Expr *InitExpr) { + return createObjectInternal(&D, D.getType(), InitExpr); + } + /// Assigns `Val` as the value of `Loc` in the environment. void setValue(const StorageLocation &Loc, Value &Val); @@ -592,6 +618,11 @@ class Environment { llvm::DenseSet &Visited, int Depth, int &CreatedValuesCount); + /// Shared implementation of `createObject()` overloads. + /// `D` and `InitExpr` may be null. + StorageLocation &createObjectInternal(const VarDecl *D, QualType Ty, + const Expr *InitExpr); + StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const; const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const; diff --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp index 531115efa64f9..bb1c2c7a1de10 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp @@ -72,7 +72,7 @@ StorageLocation & DataflowAnalysisContext::getStableStorageLocation(const VarDecl &D) { if (auto *Loc = getStorageLocation(D)) return *Loc; - auto &Loc = createStorageLocation(D.getType()); + auto &Loc = createStorageLocation(D.getType().getNonReferenceType()); setStorageLocation(D, Loc); return Loc; } diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index a49c529c37a46..fdd6b5959cd72 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -260,10 +260,8 @@ void Environment::initFieldsGlobalsAndFuncs(const FunctionDecl *FuncDecl) { for (const VarDecl *D : Vars) { if (getStorageLocation(*D) != nullptr) continue; - auto &Loc = createStorageLocation(D->getType().getNonReferenceType()); - setStorageLocation(*D, Loc); - if (auto *Val = createValue(D->getType().getNonReferenceType())) - setValue(Loc, *Val); + + setStorageLocation(*D, createObject(*D)); } for (const FunctionDecl *FD : Funcs) { @@ -296,16 +294,7 @@ Environment::Environment(DataflowAnalysisContext &DACtx, for (const auto *ParamDecl : FuncDecl->parameters()) { assert(ParamDecl != nullptr); - // References aren't objects, so the reference itself doesn't have a - // storage location. Instead, the storage location for a reference refers - // directly to an object of the referenced type -- so strip off any - // reference from the type. - auto &ParamLoc = - createStorageLocation(ParamDecl->getType().getNonReferenceType()); - setStorageLocation(*ParamDecl, ParamLoc); - if (Value *ParamVal = - createValue(ParamDecl->getType().getNonReferenceType())) - setValue(ParamLoc, *ParamVal); + setStorageLocation(*ParamDecl, createObject(*ParamDecl, nullptr)); } } @@ -382,27 +371,8 @@ void Environment::pushCallInternal(const FunctionDecl *FuncDecl, // overloaded operators implemented as member functions, and parameter packs. 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; - - QualType ParamType = Param->getType(); - if (ParamType->isReferenceType()) { - setStorageLocation(*Param, *ArgLoc); - } else { - auto &Loc = createStorageLocation(*Param); - setStorageLocation(*Param, Loc); - - if (auto *ArgVal = getValue(*ArgLoc)) { - setValue(Loc, *ArgVal); - } else if (Value *Val = createValue(ParamType)) { - setValue(Loc, *Val); - } - } + setStorageLocation(*Param, createObject(*Param, Args[ArgIndex])); } } @@ -872,6 +842,54 @@ Value *Environment::createValueUnlessSelfReferential( return nullptr; } +StorageLocation &Environment::createObjectInternal(const VarDecl *D, + QualType Ty, + const Expr *InitExpr) { + if (Ty->isReferenceType()) { + // Although variables of reference type always need to be initialized, it + // can happen that we can't see the initializer, so `InitExpr` may still + // be null. + if (InitExpr) { + if (auto *InitExprLoc = + getStorageLocation(*InitExpr, SkipPast::Reference)) + return *InitExprLoc; + } + + // Even though we have an initializer, we might not get an + // InitExprLoc, for example if the InitExpr is a CallExpr for which we + // don't have a function body. In this case, we just invent a storage + // location and value -- it's the best we can do. + return createObjectInternal(D, Ty.getNonReferenceType(), nullptr); + } + + Value *Val = nullptr; + if (InitExpr) + // In the (few) cases where an expression is intentionally + // "uninterpreted", `InitExpr` is not associated with a value. There are + // two ways to handle this situation: propagate the status, so that + // uninterpreted initializers result in uninterpreted variables, or + // provide a default value. We choose the latter so that later refinements + // of the variable can be used for reasoning about the surrounding code. + // For this reason, we let this case be handled by the `createValue()` + // call below. + // + // FIXME. If and when we interpret all language cases, change this to + // assert that `InitExpr` is interpreted, rather than supplying a + // default value (assuming we don't update the environment API to return + // references). + Val = getValueStrict(*InitExpr); + if (!Val) + Val = createValue(Ty); + + StorageLocation &Loc = + D ? createStorageLocation(*D) : createStorageLocation(Ty); + + if (Val) + setValue(Loc, *Val); + + return Loc; +} + StorageLocation &Environment::skip(StorageLocation &Loc, SkipPast SP) const { switch (SP) { case SkipPast::None: diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 4c97e81184bba..f51b9f34d9863 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -208,62 +208,15 @@ class TransferVisitor : public ConstStmtVisitor { if (D.hasGlobalStorage()) return; - if (D.getType()->isReferenceType()) { - // If this is the holding variable for a `BindingDecl`, we may already - // have a storage location set up -- so check. (See also explanation below - // where we process the `BindingDecl`.) - if (Env.getStorageLocation(D) == nullptr) { - const Expr *InitExpr = D.getInit(); - assert(InitExpr != nullptr); - if (auto *InitExprLoc = - Env.getStorageLocation(*InitExpr, SkipPast::Reference)) { - Env.setStorageLocation(D, *InitExprLoc); - } else { - // Even though we have an initializer, we might not get an - // InitExprLoc, for example if the InitExpr is a CallExpr for which we - // don't have a function body. In this case, we just invent a storage - // location and value -- it's the best we can do. - StorageLocation &Loc = - Env.createStorageLocation(D.getType().getNonReferenceType()); - Env.setStorageLocation(D, Loc); - if (Value *Val = Env.createValue(D.getType().getNonReferenceType())) - Env.setValue(Loc, *Val); - } - } - } else { - // Not a reference type. + // If this is the holding variable for a `BindingDecl`, we may already + // have a storage location set up -- so check. (See also explanation below + // where we process the `BindingDecl`.) + if (D.getType()->isReferenceType() && Env.getStorageLocation(D) != nullptr) + return; - assert(Env.getStorageLocation(D) == nullptr); - StorageLocation &Loc = Env.createStorageLocation(D); - Env.setStorageLocation(D, Loc); + assert(Env.getStorageLocation(D) == nullptr); - const Expr *InitExpr = D.getInit(); - if (InitExpr == nullptr) { - // No initializer expression - associate `Loc` with a new value. - if (Value *Val = Env.createValue(D.getType())) - Env.setValue(Loc, *Val); - return; - } - - if (auto *InitExprVal = Env.getValueStrict(*InitExpr)) - Env.setValue(Loc, *InitExprVal); - - if (Env.getValue(Loc) == nullptr) { - // We arrive here in (the few) cases where an expression is - // intentionally "uninterpreted". There are two ways to handle this - // situation: propagate the status, so that uninterpreted initializers - // result in uninterpreted variables, or provide a default value. We - // choose the latter so that later refinements of the variable can be - // used for reasoning about the surrounding code. - // - // FIXME. If and when we interpret all language cases, change this to - // assert that `InitExpr` is interpreted, rather than supplying a - // default value (assuming we don't update the environment API to return - // references). - if (Value *Val = Env.createValue(D.getType())) - Env.setValue(Loc, *Val); - } - } + Env.setStorageLocation(D, Env.createObject(D)); // `DecompositionDecl` must be handled after we've interpreted the loc // itself, because the binding expression refers back to the