From 782eced561492c74f7b4409d6ee7eee84a1647c7 Mon Sep 17 00:00:00 2001 From: Stanislav Gatev Date: Mon, 17 Jan 2022 15:17:05 +0000 Subject: [PATCH] [clang][dataflow] Replace initValueInStorageLocation with createValue Since Environment's setValue method already does part of the work that initValueInStorageLocation does, we can factor out a new createValue method to reduce the duplication. This is part of the implementation of the dataflow analysis framework. See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev. Reviewed-by: ymandel, xazax.hun Differential Revision: https://reviews.llvm.org/D117493 --- .../Analysis/FlowSensitive/DataflowEnvironment.h | 19 ++++---- .../Analysis/FlowSensitive/DataflowEnvironment.cpp | 53 ++++++++++------------ clang/lib/Analysis/FlowSensitive/Transfer.cpp | 15 ++++-- 3 files changed, 43 insertions(+), 44 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index 9613b292..5082181 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -116,15 +116,15 @@ public: /// in the environment. StorageLocation *getThisPointeeStorageLocation() const; - /// Creates a value appropriate for `Type`, assigns it to `Loc`, and returns - /// it, if `Type` is supported, otherwise return null. If `Type` is a pointer - /// or reference type, creates all the necessary storage locations and values - /// for indirections until it finds a non-pointer/non-reference type. + /// Creates a value appropriate for `Type`, if `Type` is supported, otherwise + /// return null. If `Type` is a pointer or reference type, creates all the + /// necessary storage locations and values for indirections until it finds a + /// non-pointer/non-reference type. /// /// Requirements: /// /// `Type` must not be null. - Value *initValueInStorageLocation(const StorageLocation &Loc, QualType Type); + Value *createValue(QualType Type); /// Assigns `Val` as the value of `Loc` in the environment. void setValue(const StorageLocation &Loc, Value &Val); @@ -150,8 +150,8 @@ public: Value &takeOwnership(std::unique_ptr Val); private: - /// Returns the value assigned to `Loc` in the environment or null if `Type` - /// isn't supported. + /// Creates a value appropriate for `Type`, if `Type` is supported, otherwise + /// return null. /// /// Recursively initializes storage locations and values until it sees a /// self-referential pointer or reference type. `Visited` is used to track @@ -161,9 +161,8 @@ private: /// Requirements: /// /// `Type` must not be null. - Value *initValueInStorageLocationUnlessSelfReferential( - const StorageLocation &Loc, QualType Type, - llvm::DenseSet &Visited); + Value *createValueUnlessSelfReferential(QualType Type, + llvm::DenseSet &Visited); StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const; const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const; diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 7d0cbf3..e1d420f 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -49,7 +49,8 @@ Environment::Environment(DataflowAnalysisContext &DACtx, assert(ParamDecl != nullptr); auto &ParamLoc = createStorageLocation(*ParamDecl); setStorageLocation(*ParamDecl, ParamLoc); - initValueInStorageLocation(ParamLoc, ParamDecl->getType()); + if (Value *ParamVal = createValue(ParamDecl->getType())) + setValue(ParamLoc, *ParamVal); } } @@ -60,7 +61,8 @@ Environment::Environment(DataflowAnalysisContext &DACtx, if (!ThisPointeeType->isUnionType()) { auto &ThisPointeeLoc = createStorageLocation(ThisPointeeType); DACtx.setThisPointeeStorageLocation(ThisPointeeLoc); - initValueInStorageLocation(ThisPointeeLoc, ThisPointeeType); + if (Value *ThisPointeeVal = createValue(ThisPointeeType)) + setValue(ThisPointeeLoc, *ThisPointeeVal); } } } @@ -189,21 +191,17 @@ Value *Environment::getValue(const Expr &E, SkipPast SP) const { return getValue(*Loc); } -Value *Environment::initValueInStorageLocation(const StorageLocation &Loc, - QualType Type) { +Value *Environment::createValue(QualType Type) { llvm::DenseSet Visited; - return initValueInStorageLocationUnlessSelfReferential(Loc, Type, Visited); + return createValueUnlessSelfReferential(Type, Visited); } -Value *Environment::initValueInStorageLocationUnlessSelfReferential( - const StorageLocation &Loc, QualType Type, - llvm::DenseSet &Visited) { +Value *Environment::createValueUnlessSelfReferential( + QualType Type, llvm::DenseSet &Visited) { assert(!Type.isNull()); if (Type->isIntegerType()) { - auto &Val = takeOwnership(std::make_unique()); - setValue(Loc, Val); - return &Val; + return &takeOwnership(std::make_unique()); } if (Type->isReferenceType()) { @@ -212,14 +210,15 @@ Value *Environment::initValueInStorageLocationUnlessSelfReferential( if (!Visited.contains(PointeeType.getCanonicalType())) { Visited.insert(PointeeType.getCanonicalType()); - initValueInStorageLocationUnlessSelfReferential(PointeeLoc, PointeeType, - Visited); + Value *PointeeVal = + createValueUnlessSelfReferential(PointeeType, Visited); Visited.erase(PointeeType.getCanonicalType()); + + if (PointeeVal != nullptr) + setValue(PointeeLoc, *PointeeVal); } - auto &Val = takeOwnership(std::make_unique(PointeeLoc)); - setValue(Loc, Val); - return &Val; + return &takeOwnership(std::make_unique(PointeeLoc)); } if (Type->isPointerType()) { @@ -228,19 +227,18 @@ Value *Environment::initValueInStorageLocationUnlessSelfReferential( if (!Visited.contains(PointeeType.getCanonicalType())) { Visited.insert(PointeeType.getCanonicalType()); - initValueInStorageLocationUnlessSelfReferential(PointeeLoc, PointeeType, - Visited); + Value *PointeeVal = + createValueUnlessSelfReferential(PointeeType, Visited); Visited.erase(PointeeType.getCanonicalType()); + + if (PointeeVal != nullptr) + setValue(PointeeLoc, *PointeeVal); } - auto &Val = takeOwnership(std::make_unique(PointeeLoc)); - setValue(Loc, Val); - return &Val; + return &takeOwnership(std::make_unique(PointeeLoc)); } if (Type->isStructureOrClassType()) { - auto *AggregateLoc = cast(&Loc); - // FIXME: Initialize only fields that are accessed in the context that is // being analyzed. llvm::DenseMap FieldValues; @@ -253,15 +251,12 @@ Value *Environment::initValueInStorageLocationUnlessSelfReferential( Visited.insert(FieldType.getCanonicalType()); FieldValues.insert( - {Field, initValueInStorageLocationUnlessSelfReferential( - AggregateLoc->getChild(*Field), FieldType, Visited)}); + {Field, createValueUnlessSelfReferential(FieldType, Visited)}); Visited.erase(FieldType.getCanonicalType()); } - auto &Val = - takeOwnership(std::make_unique(std::move(FieldValues))); - setValue(Loc, Val); - return &Val; + return &takeOwnership( + std::make_unique(std::move(FieldValues))); } return nullptr; diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index cc73fb0..6ae4573 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -96,7 +96,8 @@ public: const Expr *InitExpr = D.getInit(); if (InitExpr == nullptr) { // No initializer expression - associate `Loc` with a new value. - Env.initValueInStorageLocation(Loc, D.getType()); + if (Value *Val = Env.createValue(D.getType())) + Env.setValue(Loc, *Val); return; } @@ -117,7 +118,8 @@ public: // FIXME: The initializer expression must always be assigned a value. // Replace this with an assert when we have sufficient coverage of // language features. - Env.initValueInStorageLocation(Loc, D.getType()); + if (Value *Val = Env.createValue(D.getType())) + Env.setValue(Loc, *Val); } return; } @@ -128,7 +130,8 @@ public: // FIXME: The initializer expression must always be assigned a value. // Replace this with an assert when we have sufficient coverage of // language features. - Env.initValueInStorageLocation(Loc, D.getType()); + if (Value *Val = Env.createValue(D.getType())) + Env.setValue(Loc, *Val); } else { llvm_unreachable("structs and classes must always be assigned values"); } @@ -269,7 +272,8 @@ public: auto &Loc = Env.createStorageLocation(*S); Env.setStorageLocation(*S, Loc); - Env.initValueInStorageLocation(Loc, S->getType()); + if (Value *Val = Env.createValue(S->getType())) + Env.setValue(Loc, *Val); } void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *S) { @@ -319,7 +323,8 @@ public: void VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *S) { auto &Loc = Env.createStorageLocation(*S); Env.setStorageLocation(*S, Loc); - Env.initValueInStorageLocation(Loc, S->getType()); + if (Value *Val = Env.createValue(S->getType())) + Env.setValue(Loc, *Val); } void VisitCallExpr(const CallExpr *S) { -- 2.7.4