diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -154,8 +154,20 @@ 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(&Val)) { + auto &AggregateLoc = *cast(&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 --- 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(E)) + return C->getSubExpr(); + return E; +} + class TransferVisitor : public ConstStmtVisitor { public: TransferVisitor(Environment &Env) : Env(Env) {} @@ -89,13 +96,13 @@ } 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(); + // 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); - assert(SubExpr != nullptr); + if (S->getCastKind() == CK_LValueToRValue) { auto *SubExprVal = Env.getValue(*SubExpr, SkipPast::Reference); if (SubExprVal == nullptr) return; @@ -103,6 +110,15 @@ auto &ExprLoc = Env.createStorageLocation(*S); Env.setStorageLocation(*S, ExprLoc); Env.setValue(ExprLoc, *SubExprVal); + } else if (S->getCastKind() == 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) + return; + + Env.setStorageLocation(*S, *SubExprLoc); } // FIXME: Add support for CK_NoOp, CK_UserDefinedConversion, // CK_ConstructorConversion, CK_UncheckedDerivedToBase. @@ -181,15 +197,115 @@ 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 +319,11 @@ 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 +343,13 @@ 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 --- 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 @@ class TransferTest : public ::testing::Test { protected: template - void runDataflow(llvm::StringRef Code, Matcher Match) { + void runDataflow(llvm::StringRef Code, Matcher Match, + LangStandard::Kind Std = LangStandard::lang_cxx17) { test::checkDataflow( Code, "target", [](ASTContext &C, Environment &) { return NoopAnalysis(C); }, @@ -45,7 +47,9 @@ std::pair>> 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, TemporaryObject) { + std::string Code = R"( + struct A { + int Bar; + }; + + void target() { + A Foo = A(); + // [[p]] + } + )"; + runDataflow( + Code, [](llvm::ArrayRef< + std::pair>> + 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( + Env.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc = + cast(&FooLoc->getChild(*BarDecl)); + + const auto *FooVal = cast(Env.getValue(*FooLoc)); + const auto *BarVal = cast(&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>> + 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( + Env.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc = + cast(&FooLoc->getChild(*BarDecl)); + + const auto *FooVal = cast(Env.getValue(*FooLoc)); + const auto *BarVal = cast(&FooVal->getChild(*BarDecl)); + EXPECT_EQ(Env.getValue(*BarLoc), BarVal); + }, + LangStandard::lang_cxx17); +} + +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>> + 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( + Env1.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc1 = cast( + Env1.getStorageLocation(*BarDecl, SkipPast::None)); + + const auto *FooVal1 = cast(Env1.getValue(*FooLoc1)); + const auto *BarVal1 = cast(Env1.getValue(*BarLoc1)); + EXPECT_NE(FooVal1, BarVal1); + + const auto *FooBazVal1 = + cast(Env1.getValue(FooLoc1->getChild(*BazDecl))); + const auto *BarBazVal1 = + cast(Env1.getValue(BarLoc1->getChild(*BazDecl))); + EXPECT_NE(FooBazVal1, BarBazVal1); + + const auto *FooLoc2 = cast( + Env2.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc2 = cast( + Env2.getStorageLocation(*BarDecl, SkipPast::None)); + + const auto *FooVal2 = cast(Env2.getValue(*FooLoc2)); + const auto *BarVal2 = cast(Env2.getValue(*BarLoc2)); + EXPECT_EQ(FooVal2, BarVal2); + + const auto *FooBazVal2 = + cast(Env2.getValue(FooLoc1->getChild(*BazDecl))); + const auto *BarBazVal2 = + cast(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>> + 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( + Env.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc = cast( + Env.getStorageLocation(*BarDecl, SkipPast::None)); + + const auto *FooVal = cast(Env.getValue(*FooLoc)); + const auto *BarVal = cast(Env.getValue(*BarLoc)); + EXPECT_EQ(FooVal, BarVal); + + const auto *FooBazVal = + cast(Env.getValue(FooLoc->getChild(*BazDecl))); + const auto *BarBazVal = + cast(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>> + 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( + Env.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc = cast( + Env.getStorageLocation(*BarDecl, SkipPast::None)); + + const auto *FooVal = cast(Env.getValue(*FooLoc)); + const auto *BarVal = cast(Env.getValue(*BarLoc)); + EXPECT_EQ(FooVal, BarVal); + + const auto *FooBazVal = + cast(Env.getValue(FooLoc->getChild(*BazDecl))); + const auto *BarBazVal = + cast(Env.getValue(BarLoc->getChild(*BazDecl))); + EXPECT_EQ(FooBazVal, BarBazVal); + }); +} + +TEST_F(TransferTest, MoveConstructor) { + std::string Code = R"( + namespace std { + + template struct remove_reference { using type = T; }; + template struct remove_reference { using type = T; }; + template struct remove_reference { using type = T; }; + + template + using remove_reference_t = typename remove_reference::type; + + template + std::remove_reference_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>> + 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( + Env1.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *BarLoc1 = cast( + Env1.getStorageLocation(*BarDecl, SkipPast::None)); + + const auto *FooVal1 = cast(Env1.getValue(*FooLoc1)); + const auto *BarVal1 = cast(Env1.getValue(*BarLoc1)); + EXPECT_NE(FooVal1, BarVal1); + + const auto *FooBazVal1 = + cast(Env1.getValue(FooLoc1->getChild(*BazDecl))); + const auto *BarBazVal1 = + cast(Env1.getValue(BarLoc1->getChild(*BazDecl))); + EXPECT_NE(FooBazVal1, BarBazVal1); + + const auto *FooLoc2 = cast( + Env2.getStorageLocation(*FooDecl, SkipPast::None)); + const auto *FooVal2 = cast(Env2.getValue(*FooLoc2)); + EXPECT_EQ(FooVal2, BarVal1); + + const auto *FooBazVal2 = + cast(Env2.getValue(FooLoc1->getChild(*BazDecl))); + EXPECT_EQ(FooBazVal2, BarBazVal1); + }); +} + } // namespace