Author: Yitzhak Mandelbaum Date: 2022-04-19T20:52:29Z New Revision: eb2131bdbad3f74be2fcf236b4d6921612af47a9
URL: https://github.com/llvm/llvm-project/commit/eb2131bdbad3f74be2fcf236b4d6921612af47a9 DIFF: https://github.com/llvm/llvm-project/commit/eb2131bdbad3f74be2fcf236b4d6921612af47a9.diff LOG: [clang][dataflow] Do not crash on missing `Value` for struct-typed variable init. Remove constraint that an initializing expression of struct type must have an associated `Value`. This invariant is not and will not be guaranteed by the framework, because of potentially uninitialized fields. Differential Revision: https://reviews.llvm.org/D123961 Added: Modified: clang/lib/Analysis/FlowSensitive/Transfer.cpp clang/unittests/Analysis/FlowSensitive/TransferTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index a1a6025312db9..52a8a4f0a8764 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -168,27 +168,25 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { auto &Val = Env.takeOwnership(std::make_unique<ReferenceValue>(*InitExprLoc)); Env.setValue(Loc, Val); - } else { - // FIXME: The initializer expression must always be assigned a value. - // Replace this with an assert when we have sufficient coverage of - // language features. - if (Value *Val = Env.createValue(D.getType())) - Env.setValue(Loc, *Val); + return; } + } else if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) { + Env.setValue(Loc, *InitExprVal); return; } - if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) { - Env.setValue(Loc, *InitExprVal); - } 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. - if (Value *Val = Env.createValue(D.getType())) - Env.setValue(Loc, *Val); - } else { - llvm_unreachable("structs and classes must always be assigned values"); - } + // 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); } void VisitImplicitCastExpr(const ImplicitCastExpr *S) { diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index a0b018c3c5724..db1f1cff311c7 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -187,6 +187,54 @@ TEST_F(TransferTest, StructVarDecl) { }); } +TEST_F(TransferTest, StructVarDeclWithInit) { + std::string Code = R"( + struct A { + int Bar; + }; + + A Gen(); + + void target() { + A Foo = Gen(); + // [[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()); + + ASSERT_TRUE(FooDecl->getType()->isStructureType()); + auto FooFields = FooDecl->getType()->getAsRecordDecl()->fields(); + + FieldDecl *BarDecl = nullptr; + for (FieldDecl *Field : FooFields) { + if (Field->getNameAsString() == "Bar") { + BarDecl = Field; + } else { + FAIL() << "Unexpected field: " << Field->getNameAsString(); + } + } + 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, ClassVarDecl) { std::string Code = R"( class A { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
