ymandel updated this revision to Diff 491806.
ymandel added a comment.
use `isa_and_nonnull`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142468/new/
https://reviews.llvm.org/D142468
Files:
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -156,6 +156,89 @@
});
}
+TEST(TransferTest, StructIncomplete) {
+ std::string Code = R"(
+ struct A;
+
+ void target() {
+ A* Foo;
+ // [[p]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+ const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
+ auto *FooValue = dyn_cast_or_null<PointerValue>(
+ Env.getValue(*FooDecl, SkipPast::None));
+ ASSERT_THAT(FooValue, NotNull());
+
+ EXPECT_TRUE(isa<AggregateStorageLocation>(FooValue->getPointeeLoc()));
+ auto *FooPointeeValue = Env.getValue(FooValue->getPointeeLoc());
+ ASSERT_THAT(FooPointeeValue, NotNull());
+ EXPECT_TRUE(isa<StructValue>(FooPointeeValue));
+ });
+}
+
+// As a memory optimization, we prevent modeling fields nested below a certain
+// level (currently, depth 3). This test verifies this lack of modeling. We also
+// include a regression test for the case that the unmodeled field is a
+// reference to a struct; previously, we crashed when accessing such a field.
+TEST(TransferTest, StructFieldUnmodeled) {
+ std::string Code = R"(
+ struct S { int X; };
+ S GlobalS;
+ struct A { S &Unmodeled = GlobalS; };
+ struct B { A F3; };
+ struct C { B F2; };
+ struct D { C F1; };
+
+ void target() {
+ D Bar;
+ A Foo = Bar.F1.F2.F3;
+ int Zab = Foo.Unmodeled.X;
+ // [[p]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+ const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
+ ASSERT_TRUE(FooDecl->getType()->isStructureType());
+ auto FooFields = FooDecl->getType()->getAsRecordDecl()->fields();
+
+ FieldDecl *UnmodeledDecl = nullptr;
+ for (FieldDecl *Field : FooFields) {
+ if (Field->getNameAsString() == "Unmodeled") {
+ UnmodeledDecl = Field;
+ } else {
+ FAIL() << "Unexpected field: " << Field->getNameAsString();
+ }
+ }
+ ASSERT_THAT(UnmodeledDecl, NotNull());
+
+ const auto *FooLoc = cast<AggregateStorageLocation>(
+ Env.getStorageLocation(*FooDecl, SkipPast::None));
+ const auto *UnmodeledLoc = &FooLoc->getChild(*UnmodeledDecl);
+ ASSERT_TRUE(isa<ScalarStorageLocation>(UnmodeledLoc));
+ ASSERT_THAT(Env.getValue(*UnmodeledLoc), IsNull());
+
+ const ValueDecl *ZabDecl = findValueDecl(ASTCtx, "Zab");
+ ASSERT_THAT(ZabDecl, NotNull());
+ EXPECT_THAT(Env.getValue(*ZabDecl, SkipPast::None), NotNull());
+ });
+}
+
TEST(TransferTest, StructVarDecl) {
std::string Code = R"(
struct A {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -197,6 +197,8 @@
return;
if (VD->getType()->isReferenceType()) {
+ assert(isa_and_nonnull<ReferenceValue>(Env.getValue((*DeclLoc))) &&
+ "reference-typed declarations map to `ReferenceValue`s");
Env.setStorageLocation(*S, *DeclLoc);
} else {
auto &Loc = Env.createStorageLocation(*S);
@@ -474,6 +476,8 @@
return;
if (VarDeclLoc->getType()->isReferenceType()) {
+ assert(isa_and_nonnull<ReferenceValue>(Env.getValue((*VarDeclLoc))) &&
+ "reference-typed declarations map to `ReferenceValue`s");
Env.setStorageLocation(*S, *VarDeclLoc);
} else {
auto &Loc = Env.createStorageLocation(*S);
@@ -494,7 +498,22 @@
auto &MemberLoc = BaseLoc->getChild(*Member);
if (MemberLoc.getType()->isReferenceType()) {
- Env.setStorageLocation(*S, MemberLoc);
+ // Based on its type, `MemberLoc` must be mapped either to nothing or to a
+ // `ReferenceValue`. For the former, we won't set a storage location for
+ // this expression, so as to maintain an invariant lvalue expressions;
+ // namely, that their location maps to a `ReferenceValue`. In this,
+ // lvalues are unlike other expressions, where it is valid for their
+ // location to map to nothing (because they are not modeled).
+ //
+ // Note: we need this invariant for lvalues so that, when accessing a
+ // value, we can distinguish an rvalue from an lvalue. An alternative
+ // design, which takes the expression's value category into account, would
+ // avoid the need for this invariant.
+ if (auto *V = Env.getValue(MemberLoc)) {
+ assert(isa<ReferenceValue>(V) &&
+ "reference-typed declarations map to `ReferenceValue`s");
+ Env.setStorageLocation(*S, MemberLoc);
+ }
} else {
auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
