Author: Stanislav Gatev Date: 2022-01-14T14:58:01Z New Revision: 7d941d6d21e91e8466bf200da094d027338b92fa
URL: https://github.com/llvm/llvm-project/commit/7d941d6d21e91e8466bf200da094d027338b92fa DIFF: https://github.com/llvm/llvm-project/commit/7d941d6d21e91e8466bf200da094d027338b92fa.diff LOG: [clang][dataflow] Add transfer functions for constructors 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/D117218 Added: Modified: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp clang/lib/Analysis/FlowSensitive/Transfer.cpp clang/unittests/Analysis/FlowSensitive/TransferTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 5824bc13ce7b6..7d0cbf3f656bd 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -154,8 +154,20 @@ StorageLocation *Environment::getThisPointeeStorageLocation() const { return DACtx->getThisPointeeStorageLocation(); } -void Environment::setValue(const StorageLocation &Loc, Value &Value) { - LocToVal[&Loc] = &Value; +void Environment::setValue(const StorageLocation &Loc, Value &Val) { + LocToVal[&Loc] = &Val; + + if (auto *StructVal = dyn_cast<StructValue>(&Val)) { + auto &AggregateLoc = *cast<AggregateStorageLocation>(&Loc); + + const QualType Type = AggregateLoc.getType(); + assert(Type->isStructureOrClassType()); + + for (const FieldDecl *Field : Type->getAsRecordDecl()->fields()) { + assert(Field != nullptr); + setValue(AggregateLoc.getChild(*Field), StructVal->getChild(*Field)); + } + } } Value *Environment::getValue(const StorageLocation &Loc) const { diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 7681b729d2b8e..8b35ec023ecc1 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -14,6 +14,7 @@ #include "clang/Analysis/FlowSensitive/Transfer.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" +#include "clang/AST/DeclCXX.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/OperationKinds.h" @@ -28,6 +29,12 @@ namespace clang { namespace dataflow { +static const Expr *skipExprWithCleanups(const Expr *E) { + if (auto *C = dyn_cast_or_null<ExprWithCleanups>(E)) + return C->getSubExpr(); + return E; +} + class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { public: TransferVisitor(Environment &Env) : Env(Env) {} @@ -89,23 +96,39 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { } void VisitImplicitCastExpr(const ImplicitCastExpr *S) { - if (S->getCastKind() == CK_LValueToRValue) { - // The CFG does not contain `ParenExpr` as top-level statements in basic - // blocks, however sub-expressions can still be of that type. - assert(S->getSubExpr() != nullptr); - const Expr *SubExpr = S->getSubExpr()->IgnoreParens(); - - assert(SubExpr != nullptr); + // The CFG does not contain `ParenExpr` as top-level statements in basic + // blocks, however sub-expressions can still be of that type. + assert(S->getSubExpr() != nullptr); + const Expr *SubExpr = S->getSubExpr()->IgnoreParens(); + assert(SubExpr != nullptr); + + switch (S->getCastKind()) { + case CK_LValueToRValue: { auto *SubExprVal = Env.getValue(*SubExpr, SkipPast::Reference); if (SubExprVal == nullptr) - return; + break; auto &ExprLoc = Env.createStorageLocation(*S); Env.setStorageLocation(*S, ExprLoc); Env.setValue(ExprLoc, *SubExprVal); + break; + } + case CK_NoOp: { + // FIXME: Consider making `Environment::getStorageLocation` skip noop + // expressions (this and other similar expressions in the file) instead of + // assigning them storage locations. + auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None); + if (SubExprLoc == nullptr) + break; + + Env.setStorageLocation(*S, *SubExprLoc); + break; + } + default: + // FIXME: Add support for CK_UserDefinedConversion, + // CK_ConstructorConversion, CK_UncheckedDerivedToBase. + break; } - // FIXME: Add support for CK_NoOp, CK_UserDefinedConversion, - // CK_ConstructorConversion, CK_UncheckedDerivedToBase. } void VisitUnaryOperator(const UnaryOperator *S) { @@ -181,15 +204,115 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { Env.setValue(FieldLoc, *InitExprVal); } + void VisitCXXConstructExpr(const CXXConstructExpr *S) { + const CXXConstructorDecl *ConstructorDecl = S->getConstructor(); + assert(ConstructorDecl != nullptr); + + if (ConstructorDecl->isCopyOrMoveConstructor()) { + assert(S->getNumArgs() == 1); + + const Expr *Arg = S->getArg(0); + assert(Arg != nullptr); + + if (S->isElidable()) { + auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::Reference); + if (ArgLoc == nullptr) + return; + + Env.setStorageLocation(*S, *ArgLoc); + } else if (auto *ArgVal = Env.getValue(*Arg, SkipPast::Reference)) { + auto &Loc = Env.createStorageLocation(*S); + Env.setStorageLocation(*S, Loc); + Env.setValue(Loc, *ArgVal); + } + return; + } + + auto &Loc = Env.createStorageLocation(*S); + Env.setStorageLocation(*S, Loc); + Env.initValueInStorageLocation(Loc, S->getType()); + } + + void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *S) { + if (S->getOperator() == OO_Equal) { + assert(S->getNumArgs() == 2); + + const Expr *Arg0 = S->getArg(0); + assert(Arg0 != nullptr); + + const Expr *Arg1 = S->getArg(1); + assert(Arg1 != nullptr); + + // Evaluate only copy and move assignment operators. + auto *Arg0Type = Arg0->getType()->getUnqualifiedDesugaredType(); + auto *Arg1Type = Arg1->getType()->getUnqualifiedDesugaredType(); + if (Arg0Type != Arg1Type) + return; + + auto *ObjectLoc = Env.getStorageLocation(*Arg0, SkipPast::Reference); + if (ObjectLoc == nullptr) + return; + + auto *Val = Env.getValue(*Arg1, SkipPast::Reference); + if (Val == nullptr) + return; + + Env.setValue(*ObjectLoc, *Val); + } + } + + void VisitCXXFunctionalCastExpr(const CXXFunctionalCastExpr *S) { + if (S->getCastKind() == CK_ConstructorConversion) { + // The CFG does not contain `ParenExpr` as top-level statements in basic + // blocks, however sub-expressions can still be of that type. + assert(S->getSubExpr() != nullptr); + const Expr *SubExpr = S->getSubExpr(); + assert(SubExpr != nullptr); + + auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None); + if (SubExprLoc == nullptr) + return; + + Env.setStorageLocation(*S, *SubExprLoc); + } + } + + void VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *S) { + auto &Loc = Env.createStorageLocation(*S); + Env.setStorageLocation(*S, Loc); + Env.initValueInStorageLocation(Loc, S->getType()); + } + + void VisitCallExpr(const CallExpr *S) { + if (S->isCallToStdMove()) { + assert(S->getNumArgs() == 1); + + const Expr *Arg = S->getArg(0); + assert(Arg != nullptr); + + auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::None); + if (ArgLoc == nullptr) + return; + + Env.setStorageLocation(*S, *ArgLoc); + } + } + + void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *S) { + const Expr *SubExpr = S->getSubExpr(); + assert(SubExpr != nullptr); + + auto *SubExprLoc = Env.getStorageLocation(*SubExpr, SkipPast::None); + if (SubExprLoc == nullptr) + return; + + Env.setStorageLocation(*S, *SubExprLoc); + } + // FIXME: Add support for: - // - CallExpr // - CXXBindTemporaryExpr // - CXXBoolLiteralExpr - // - CXXConstructExpr - // - CXXFunctionalCastExpr - // - CXXOperatorCallExpr // - CXXStaticCastExpr - // - MaterializeTemporaryExpr private: void visitVarDecl(const VarDecl &D) { @@ -203,6 +326,11 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { return; } + // The CFG does not contain `ParenExpr` as top-level statements in basic + // blocks, however sub-expressions can still be of that type. + InitExpr = skipExprWithCleanups(D.getInit()->IgnoreParens()); + assert(InitExpr != nullptr); + if (D.getType()->isReferenceType()) { // Initializing a reference variable - do not create a reference to // reference. @@ -222,11 +350,13 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) { Env.setValue(Loc, *InitExprVal); - } else { + } else if (!D.getType()->isStructureOrClassType()) { // 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()); + } else { + llvm_unreachable("structs and classes must always be assigned values"); } } diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index 1c5e49df8da9a..ac97cce8a9567 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -15,6 +15,7 @@ #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/StorageLocation.h" #include "clang/Analysis/FlowSensitive/Value.h" +#include "clang/Basic/LangStandard.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" @@ -37,7 +38,8 @@ using ::testing::Pair; class TransferTest : public ::testing::Test { protected: template <typename Matcher> - void runDataflow(llvm::StringRef Code, Matcher Match) { + void runDataflow(llvm::StringRef Code, Matcher Match, + LangStandard::Kind Std = LangStandard::lang_cxx17) { test::checkDataflow<NoopAnalysis>( Code, "target", [](ASTContext &C, Environment &) { return NoopAnalysis(C); }, @@ -45,7 +47,9 @@ class TransferTest : public ::testing::Test { std::pair<std::string, DataflowAnalysisState<NoopLattice>>> Results, ASTContext &ASTCtx) { Match(Results, ASTCtx); }, - {"-fsyntax-only", "-std=c++17"}); + {"-fsyntax-only", + "-std=" + + std::string(LangStandard::getLangStandardForKind(Std).getName())}); } }; @@ -1281,4 +1285,308 @@ TEST_F(TransferTest, DefaultInitializerReference) { }); } +TEST_F(TransferTest, TemporaryObject) { + std::string Code = R"( + struct A { + int Bar; + }; + + void target() { + A Foo = A(); + // [[p]] + } + )"; + runDataflow( + Code, [](llvm::ArrayRef< + std::pair<std::string, DataflowAnalysisState<NoopLattice>>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, ElementsAre(Pair("p", _))); + const Environment &Env = Results[0].second.Env; + + const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + const auto *FooLoc = cast<AggregateStorageLocation>( + Env.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc = + cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl)); + + const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc)); + const auto *BarVal = cast<IntegerValue>(&FooVal->getChild(*BarDecl)); + EXPECT_EQ(Env.getValue(*BarLoc), BarVal); + }); +} + +TEST_F(TransferTest, ElidableConstructor) { + // This test is effectively the same as TransferTest.TemporaryObject, but + // the code is compiled as C++ 14. + std::string Code = R"( + struct A { + int Bar; + }; + + void target() { + A Foo = A(); + // [[p]] + } + )"; + runDataflow( + Code, + [](llvm::ArrayRef< + std::pair<std::string, DataflowAnalysisState<NoopLattice>>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, ElementsAre(Pair("p", _))); + const Environment &Env = Results[0].second.Env; + + const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + const auto *FooLoc = cast<AggregateStorageLocation>( + Env.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc = + cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl)); + + const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc)); + const auto *BarVal = cast<IntegerValue>(&FooVal->getChild(*BarDecl)); + EXPECT_EQ(Env.getValue(*BarLoc), BarVal); + }, + LangStandard::lang_cxx14); +} + +TEST_F(TransferTest, AssignmentOperator) { + std::string Code = R"( + struct A { + int Baz; + }; + + void target() { + A Foo; + A Bar; + // [[p1]] + Foo = Bar; + // [[p2]] + } + )"; + runDataflow( + Code, [](llvm::ArrayRef< + std::pair<std::string, DataflowAnalysisState<NoopLattice>>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _))); + const Environment &Env1 = Results[0].second.Env; + const Environment &Env2 = Results[1].second.Env; + + const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz"); + ASSERT_THAT(BazDecl, NotNull()); + + const auto *FooLoc1 = cast<AggregateStorageLocation>( + Env1.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc1 = cast<AggregateStorageLocation>( + Env1.getStorageLocation(*BarDecl, SkipPast::None)); + + const auto *FooVal1 = cast<StructValue>(Env1.getValue(*FooLoc1)); + const auto *BarVal1 = cast<StructValue>(Env1.getValue(*BarLoc1)); + EXPECT_NE(FooVal1, BarVal1); + + const auto *FooBazVal1 = + cast<IntegerValue>(Env1.getValue(FooLoc1->getChild(*BazDecl))); + const auto *BarBazVal1 = + cast<IntegerValue>(Env1.getValue(BarLoc1->getChild(*BazDecl))); + EXPECT_NE(FooBazVal1, BarBazVal1); + + const auto *FooLoc2 = cast<AggregateStorageLocation>( + Env2.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc2 = cast<AggregateStorageLocation>( + Env2.getStorageLocation(*BarDecl, SkipPast::None)); + + const auto *FooVal2 = cast<StructValue>(Env2.getValue(*FooLoc2)); + const auto *BarVal2 = cast<StructValue>(Env2.getValue(*BarLoc2)); + EXPECT_EQ(FooVal2, BarVal2); + + const auto *FooBazVal2 = + cast<IntegerValue>(Env2.getValue(FooLoc1->getChild(*BazDecl))); + const auto *BarBazVal2 = + cast<IntegerValue>(Env2.getValue(BarLoc1->getChild(*BazDecl))); + EXPECT_EQ(FooBazVal2, BarBazVal2); + }); +} + +TEST_F(TransferTest, CopyConstructor) { + std::string Code = R"( + struct A { + int Baz; + }; + + void target() { + A Foo; + A Bar = Foo; + // [[p]] + } + )"; + runDataflow( + Code, [](llvm::ArrayRef< + std::pair<std::string, DataflowAnalysisState<NoopLattice>>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, ElementsAre(Pair("p", _))); + const Environment &Env = Results[0].second.Env; + + const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz"); + ASSERT_THAT(BazDecl, NotNull()); + + const auto *FooLoc = cast<AggregateStorageLocation>( + Env.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc = cast<AggregateStorageLocation>( + Env.getStorageLocation(*BarDecl, SkipPast::None)); + + const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc)); + const auto *BarVal = cast<StructValue>(Env.getValue(*BarLoc)); + EXPECT_EQ(FooVal, BarVal); + + const auto *FooBazVal = + cast<IntegerValue>(Env.getValue(FooLoc->getChild(*BazDecl))); + const auto *BarBazVal = + cast<IntegerValue>(Env.getValue(BarLoc->getChild(*BazDecl))); + EXPECT_EQ(FooBazVal, BarBazVal); + }); +} + +TEST_F(TransferTest, CopyConstructorWithParens) { + std::string Code = R"( + struct A { + int Baz; + }; + + void target() { + A Foo; + A Bar((A(Foo))); + // [[p]] + } + )"; + runDataflow( + Code, [](llvm::ArrayRef< + std::pair<std::string, DataflowAnalysisState<NoopLattice>>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, ElementsAre(Pair("p", _))); + const Environment &Env = Results[0].second.Env; + + const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz"); + ASSERT_THAT(BazDecl, NotNull()); + + const auto *FooLoc = cast<AggregateStorageLocation>( + Env.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc = cast<AggregateStorageLocation>( + Env.getStorageLocation(*BarDecl, SkipPast::None)); + + const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc)); + const auto *BarVal = cast<StructValue>(Env.getValue(*BarLoc)); + EXPECT_EQ(FooVal, BarVal); + + const auto *FooBazVal = + cast<IntegerValue>(Env.getValue(FooLoc->getChild(*BazDecl))); + const auto *BarBazVal = + cast<IntegerValue>(Env.getValue(BarLoc->getChild(*BazDecl))); + EXPECT_EQ(FooBazVal, BarBazVal); + }); +} + +TEST_F(TransferTest, MoveConstructor) { + std::string Code = R"( + namespace std { + + template <typename T> struct remove_reference { using type = T; }; + template <typename T> struct remove_reference<T&> { using type = T; }; + template <typename T> struct remove_reference<T&&> { using type = T; }; + + template <typename T> + using remove_reference_t = typename remove_reference<T>::type; + + template <typename T> + std::remove_reference_t<T>&& move(T&& x); + + } // namespace std + + struct A { + int Baz; + }; + + void target() { + A Foo; + A Bar; + // [[p1]] + Foo = std::move(Bar); + // [[p2]] + } + )"; + runDataflow( + Code, [](llvm::ArrayRef< + std::pair<std::string, DataflowAnalysisState<NoopLattice>>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _))); + const Environment &Env1 = Results[0].second.Env; + const Environment &Env2 = Results[1].second.Env; + + const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz"); + ASSERT_THAT(BazDecl, NotNull()); + + const auto *FooLoc1 = cast<AggregateStorageLocation>( + Env1.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc1 = cast<AggregateStorageLocation>( + Env1.getStorageLocation(*BarDecl, SkipPast::None)); + + const auto *FooVal1 = cast<StructValue>(Env1.getValue(*FooLoc1)); + const auto *BarVal1 = cast<StructValue>(Env1.getValue(*BarLoc1)); + EXPECT_NE(FooVal1, BarVal1); + + const auto *FooBazVal1 = + cast<IntegerValue>(Env1.getValue(FooLoc1->getChild(*BazDecl))); + const auto *BarBazVal1 = + cast<IntegerValue>(Env1.getValue(BarLoc1->getChild(*BazDecl))); + EXPECT_NE(FooBazVal1, BarBazVal1); + + const auto *FooLoc2 = cast<AggregateStorageLocation>( + Env2.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *FooVal2 = cast<StructValue>(Env2.getValue(*FooLoc2)); + EXPECT_EQ(FooVal2, BarVal1); + + const auto *FooBazVal2 = + cast<IntegerValue>(Env2.getValue(FooLoc1->getChild(*BazDecl))); + EXPECT_EQ(FooBazVal2, BarBazVal1); + }); +} + } // namespace _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
