[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb611376e7eb5: [clang][dataflow] Singleton pointer values for
null pointers. (authored by wyt, committed by gribozavr).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128056/new/
https://reviews.llvm.org/D128056
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
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
@@ -2214,6 +2214,93 @@
});
}
+TEST_F(TransferTest, NullToPointerCast) {
+ std::string Code = R"(
+struct Baz {};
+void target() {
+ int *FooX = nullptr;
+ int *FooY = nullptr;
+ bool **Bar = nullptr;
+ Baz *Baz = nullptr;
+ // [[p]]
+}
+ )";
+ runDataflow(Code,
+ [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment = Results[0].second.Env;
+
+const ValueDecl *FooXDecl = findValueDecl(ASTCtx, "FooX");
+ASSERT_THAT(FooXDecl, NotNull());
+
+const ValueDecl *FooYDecl = findValueDecl(ASTCtx, "FooY");
+ASSERT_THAT(FooYDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooXVal =
+cast(Env.getValue(*FooXDecl, SkipPast::None));
+const auto *FooYVal =
+cast(Env.getValue(*FooYDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+const auto *BazVal =
+cast(Env.getValue(*BazDecl, SkipPast::None));
+
+EXPECT_EQ(FooXVal, FooYVal);
+EXPECT_NE(FooXVal, BarVal);
+EXPECT_NE(FooXVal, BazVal);
+EXPECT_NE(BarVal, BazVal);
+
+const StorageLocation = FooXVal->getPointeeLoc();
+EXPECT_TRUE(isa(FooPointeeLoc));
+EXPECT_THAT(Env.getValue(FooPointeeLoc), IsNull());
+
+const StorageLocation = BarVal->getPointeeLoc();
+EXPECT_TRUE(isa(BarPointeeLoc));
+EXPECT_THAT(Env.getValue(BarPointeeLoc), IsNull());
+
+const StorageLocation = BazVal->getPointeeLoc();
+EXPECT_TRUE(isa(BazPointeeLoc));
+EXPECT_THAT(Env.getValue(BazPointeeLoc), IsNull());
+ });
+}
+
+TEST_F(TransferTest, NullToMemberPointerCast) {
+ std::string Code = R"(
+struct Foo {};
+void target(Foo *Foo) {
+ int Foo::*MemberPointer = nullptr;
+ // [[p]]
+}
+ )";
+ runDataflow(
+ Code, [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment = Results[0].second.Env;
+
+const ValueDecl *MemberPointerDecl =
+findValueDecl(ASTCtx, "MemberPointer");
+ASSERT_THAT(MemberPointerDecl, NotNull());
+
+const auto *MemberPointerVal = cast(
+Env.getValue(*MemberPointerDecl, SkipPast::None));
+
+const StorageLocation = MemberPointerVal->getPointeeLoc();
+EXPECT_THAT(Env.getValue(MemberLoc), IsNull());
+ });
+}
+
TEST_F(TransferTest, AddrOfValue) {
std::string Code = R"(
void target() {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -251,6 +251,16 @@
Env.setStorageLocation(*S, *SubExprLoc);
break;
}
+case CK_NullToPointer:
+case CK_NullToMemberPointer: {
+ auto = Env.createStorageLocation(S->getType());
+ Env.setStorageLocation(*S, Loc);
+
+ auto =
+ Env.getOrCreateNullPointerValue(S->getType()->getPointeeType());
+ Env.setValue(Loc, NullPointerVal);
+ break;
+}
default:
break;
}
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.
wyt updated this revision to Diff 440162.
wyt marked 2 inline comments as done.
wyt added a comment.
Address comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128056/new/
https://reviews.llvm.org/D128056
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
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
@@ -2214,6 +2214,93 @@
});
}
+TEST_F(TransferTest, NullToPointerCast) {
+ std::string Code = R"(
+struct Baz {};
+void target() {
+ int *FooX = nullptr;
+ int *FooY = nullptr;
+ bool **Bar = nullptr;
+ Baz *Baz = nullptr;
+ // [[p]]
+}
+ )";
+ runDataflow(Code,
+ [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment = Results[0].second.Env;
+
+const ValueDecl *FooXDecl = findValueDecl(ASTCtx, "FooX");
+ASSERT_THAT(FooXDecl, NotNull());
+
+const ValueDecl *FooYDecl = findValueDecl(ASTCtx, "FooY");
+ASSERT_THAT(FooYDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooXVal =
+cast(Env.getValue(*FooXDecl, SkipPast::None));
+const auto *FooYVal =
+cast(Env.getValue(*FooYDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+const auto *BazVal =
+cast(Env.getValue(*BazDecl, SkipPast::None));
+
+EXPECT_EQ(FooXVal, FooYVal);
+EXPECT_NE(FooXVal, BarVal);
+EXPECT_NE(FooXVal, BazVal);
+EXPECT_NE(BarVal, BazVal);
+
+const StorageLocation = FooXVal->getPointeeLoc();
+EXPECT_TRUE(isa(FooPointeeLoc));
+EXPECT_THAT(Env.getValue(FooPointeeLoc), IsNull());
+
+const StorageLocation = BarVal->getPointeeLoc();
+EXPECT_TRUE(isa(BarPointeeLoc));
+EXPECT_THAT(Env.getValue(BarPointeeLoc), IsNull());
+
+const StorageLocation = BazVal->getPointeeLoc();
+EXPECT_TRUE(isa(BazPointeeLoc));
+EXPECT_THAT(Env.getValue(BazPointeeLoc), IsNull());
+ });
+}
+
+TEST_F(TransferTest, NullToMemberPointerCast) {
+ std::string Code = R"(
+struct Foo {};
+void target(Foo *Foo) {
+ int Foo::*MemberPointer = nullptr;
+ // [[p]]
+}
+ )";
+ runDataflow(
+ Code, [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment = Results[0].second.Env;
+
+const ValueDecl *MemberPointerDecl =
+findValueDecl(ASTCtx, "MemberPointer");
+ASSERT_THAT(MemberPointerDecl, NotNull());
+
+const auto *MemberPointerVal = cast(
+Env.getValue(*MemberPointerDecl, SkipPast::None));
+
+const StorageLocation = MemberPointerVal->getPointeeLoc();
+EXPECT_THAT(Env.getValue(MemberLoc), IsNull());
+ });
+}
+
TEST_F(TransferTest, AddrOfValue) {
std::string Code = R"(
void target() {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -251,6 +251,16 @@
Env.setStorageLocation(*S, *SubExprLoc);
break;
}
+case CK_NullToPointer:
+case CK_NullToMemberPointer: {
+ auto = Env.createStorageLocation(S->getType());
+ Env.setStorageLocation(*S, Loc);
+
+ auto =
+ Env.getOrCreateNullPointerValue(S->getType()->getPointeeType());
+ Env.setValue(Loc, NullPointerVal);
+ break;
+}
default:
break;
}
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++
[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.
gribozavr2 accepted this revision.
gribozavr2 added inline comments.
Comment at:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:240-241
+ // Index used to avoid recreating pointer values for null pointers of the
+ // same canonical pointee type.
+ //
Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:59
+PointerValue &
+DataflowAnalysisContext::getOrCreateNullPointerValue(QualType PointeeType) {
+ auto CanonicalPointeeType = PointeeType.getCanonicalType();
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128056/new/
https://reviews.llvm.org/D128056
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.
wyt added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:252-255 + // FIXME: The pointer values are indexed by the pointee types which are + // required to initialize the `PointeeLoc` field in `PointerValue`. Consider + // creating a type-independent `NullPointerValue` without a `PointeeLoc` + // field. @xazax.hun > Should we have a `PointerValue` without `PointeeLoc` to represent null > pointers? `PointeeLoc` is a reference field in the current `PointerValue` so it is required for now. Creating an independent `NullPointerValue` class without `PointeeLoc` is something we are considering, but in this patch we will be using `PointerValues` with a `PointeeLoc` corresponding to the pointee type to represent null pointers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128056/new/ https://reviews.llvm.org/D128056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.
wyt updated this revision to Diff 439721.
wyt marked 5 inline comments as done.
wyt added a comment.
Address comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128056/new/
https://reviews.llvm.org/D128056
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
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
@@ -2214,6 +2214,93 @@
});
}
+TEST_F(TransferTest, NullToPointerCast) {
+ std::string Code = R"(
+struct Baz {};
+void target() {
+ int *FooX = nullptr;
+ int *FooY = nullptr;
+ bool **Bar = nullptr;
+ Baz *Baz = nullptr;
+ // [[p]]
+}
+ )";
+ runDataflow(Code,
+ [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment = Results[0].second.Env;
+
+const ValueDecl *FooXDecl = findValueDecl(ASTCtx, "FooX");
+ASSERT_THAT(FooXDecl, NotNull());
+
+const ValueDecl *FooYDecl = findValueDecl(ASTCtx, "FooY");
+ASSERT_THAT(FooYDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooXVal =
+cast(Env.getValue(*FooXDecl, SkipPast::None));
+const auto *FooYVal =
+cast(Env.getValue(*FooYDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+const auto *BazVal =
+cast(Env.getValue(*BazDecl, SkipPast::None));
+
+EXPECT_EQ(FooXVal, FooYVal);
+EXPECT_NE(FooXVal, BarVal);
+EXPECT_NE(FooXVal, BazVal);
+EXPECT_NE(BarVal, BazVal);
+
+const StorageLocation = FooXVal->getPointeeLoc();
+EXPECT_TRUE(isa(FooPointeeLoc));
+EXPECT_THAT(Env.getValue(FooPointeeLoc), IsNull());
+
+const StorageLocation = BarVal->getPointeeLoc();
+EXPECT_TRUE(isa(BarPointeeLoc));
+EXPECT_THAT(Env.getValue(BarPointeeLoc), IsNull());
+
+const StorageLocation = BazVal->getPointeeLoc();
+EXPECT_TRUE(isa(BazPointeeLoc));
+EXPECT_THAT(Env.getValue(BazPointeeLoc), IsNull());
+ });
+}
+
+TEST_F(TransferTest, NullToMemberPointerCast) {
+ std::string Code = R"(
+struct Foo {};
+void target(Foo *Foo) {
+ int Foo::*MemberPointer = nullptr;
+ // [[p]]
+}
+ )";
+ runDataflow(
+ Code, [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment = Results[0].second.Env;
+
+const ValueDecl *MemberPointerDecl =
+findValueDecl(ASTCtx, "MemberPointer");
+ASSERT_THAT(MemberPointerDecl, NotNull());
+
+const auto *MemberPointerVal = cast(
+Env.getValue(*MemberPointerDecl, SkipPast::None));
+
+const StorageLocation = MemberPointerVal->getPointeeLoc();
+EXPECT_THAT(Env.getValue(MemberLoc), IsNull());
+ });
+}
+
TEST_F(TransferTest, AddrOfValue) {
std::string Code = R"(
void target() {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -251,6 +251,16 @@
Env.setStorageLocation(*S, *SubExprLoc);
break;
}
+case CK_NullToPointer:
+case CK_NullToMemberPointer: {
+ auto = Env.createStorageLocation(S->getType());
+ Env.setStorageLocation(*S, Loc);
+
+ auto =
+ Env.getOrCreateNullPointerValue(S->getType()->getPointeeType());
+ Env.setValue(Loc, NullPointerVal);
+ break;
+}
default:
break;
}
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++
[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.
sgatev accepted this revision. sgatev added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:164-166 + /// Returns a pointer value that represents a null pointer. Calls + /// with `PointeeType` that are canonically equivalent will return the same + /// result. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:316-318 + /// Returns a pointer value that represents a null pointer. Calls + /// with `PointeeType` that are canonically equivalent will return the same + /// result. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:319 + /// result. + PointerValue (QualType PointeeType); + Let's move this below `getThisPointeeStorageLocation` so that it's closer to related members. Same for the cpp file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128056/new/ https://reviews.llvm.org/D128056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.
xazax.hun accepted this revision. xazax.hun added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp:69 +Res.first->second = +(std::make_unique(PointeeLoc)); + } Should we have a `PointerValue` without `PointeeLoc` to represent null pointers? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128056/new/ https://reviews.llvm.org/D128056 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.
wyt marked an inline comment as done.
wyt added a comment.
@xazax.hun
> Since you always want this function to create a null pointer value, I think
> it would be less error prone to ask for the location instead of an arbitrary
> value. Currently, a confused caller could put a non-null value into a table.
Replaced with a getOrCreate factory to encapsulate pointer value creation.
> I think `getAsString` is considered expensive. Could you use `QualType`
> directly as the key?
Done.
> I am wondering about the future plans regarding how pointers are represented.
> What will be the expected behavior when the analysis discovers that the
> pointer has a null value? E.g.:
>
> if (p == nullptr)
> {
>
> }
>
> Would we expect `p` in this case to have the same singleton value in the then
> block of the if statement?
This is an interesting idea.
IIUC, for non-boolean values, the core framework currently does not dissect the
equality expression - p == nullptr would be represented by a symbolic boolean,
but p and nullptr would not be entangled together.
However, we are working on a pointer nullability analysis on top of the
framework that should add information to interrelate two pointer values and
their nullability information when we do a comparison between pointers.
Comment at:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:146
+ /// `PointeeType`.
+ void setNullPointerVal(QualType PointeeType, PointerValue ) {
+assert(NullPointerVals.find(PointeeType.getAsString()) ==
xazax.hun wrote:
> Since you always want this function to create a null pointer value, I think
> it would be less error prone to ask for the location instead of an arbitrary
> value. Currently, a confused caller could put a non-null value into a table.
> Since you always want this function to create a null pointer value, I think
> it would be less error prone to ask for the location instead of an arbitrary
> value. Currently, a confused caller could put a non-null value into a table.
Comment at:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:149
+ NullPointerVals.end());
+NullPointerVals[PointeeType.getAsString()] =
+ }
xazax.hun wrote:
> I think `getAsString` is considered expensive. Could you use `QualType`
> directly as the key?
> I think `getAsString` is considered expensive. Could you use `QualType`
> directly as the key?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128056/new/
https://reviews.llvm.org/D128056
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.
wyt updated this revision to Diff 439078.
wyt added a comment.
Use QualType as key to singleton map, implement getOrCreate factory function
for retrieving null pointer values.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128056/new/
https://reviews.llvm.org/D128056
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
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
@@ -2214,6 +2214,93 @@
});
}
+TEST_F(TransferTest, NullToPointerCast) {
+ std::string Code = R"(
+struct Baz {};
+void target() {
+ int *FooX = nullptr;
+ int *FooY = nullptr;
+ bool **Bar = nullptr;
+ Baz *Baz = nullptr;
+ // [[p]]
+}
+ )";
+ runDataflow(Code,
+ [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment = Results[0].second.Env;
+
+const ValueDecl *FooXDecl = findValueDecl(ASTCtx, "FooX");
+ASSERT_THAT(FooXDecl, NotNull());
+
+const ValueDecl *FooYDecl = findValueDecl(ASTCtx, "FooY");
+ASSERT_THAT(FooYDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooXVal =
+cast(Env.getValue(*FooXDecl, SkipPast::None));
+const auto *FooYVal =
+cast(Env.getValue(*FooYDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+const auto *BazVal =
+cast(Env.getValue(*BazDecl, SkipPast::None));
+
+EXPECT_EQ(FooXVal, FooYVal);
+EXPECT_NE(FooXVal, BarVal);
+EXPECT_NE(FooXVal, BazVal);
+EXPECT_NE(BarVal, BazVal);
+
+const StorageLocation = FooXVal->getPointeeLoc();
+EXPECT_TRUE(isa(FooPointeeLoc));
+EXPECT_THAT(Env.getValue(FooPointeeLoc), IsNull());
+
+const StorageLocation = BarVal->getPointeeLoc();
+EXPECT_TRUE(isa(BarPointeeLoc));
+EXPECT_THAT(Env.getValue(BarPointeeLoc), IsNull());
+
+const StorageLocation = BazVal->getPointeeLoc();
+EXPECT_TRUE(isa(BazPointeeLoc));
+EXPECT_THAT(Env.getValue(BazPointeeLoc), IsNull());
+ });
+}
+
+TEST_F(TransferTest, NullToMemberPointerCast) {
+ std::string Code = R"(
+struct Foo {};
+void target(Foo *Foo) {
+ int Foo::*MemberPointer = nullptr;
+ // [[p]]
+}
+ )";
+ runDataflow(
+ Code, [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment = Results[0].second.Env;
+
+const ValueDecl *MemberPointerDecl =
+findValueDecl(ASTCtx, "MemberPointer");
+ASSERT_THAT(MemberPointerDecl, NotNull());
+
+const auto *MemberPointerVal = cast(
+Env.getValue(*MemberPointerDecl, SkipPast::None));
+
+const StorageLocation = MemberPointerVal->getPointeeLoc();
+EXPECT_THAT(Env.getValue(MemberLoc), IsNull());
+ });
+}
+
TEST_F(TransferTest, AddrOfValue) {
std::string Code = R"(
void target() {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -251,6 +251,16 @@
Env.setStorageLocation(*S, *SubExprLoc);
break;
}
+case CK_NullToPointer:
+case CK_NullToMemberPointer: {
+ auto = Env.createStorageLocation(S->getType());
+ Env.setStorageLocation(*S, Loc);
+
+ auto =
+ Env.getOrCreateNullPointerValue(S->getType()->getPointeeType());
+ Env.setValue(Loc, NullPointerVal);
+ break;
+}
default:
break;
}
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
---
[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.
xazax.hun added a comment.
I am wondering about the future plans regarding how pointers are represented.
What will be the expected behavior when the analysis discovers that the pointer
has a null value? E.g.:
if (p == nullptr)
{
}
Would we expect `p` in this case to have the same singleton value in the then
block of the if statement?
Comment at:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:146
+ /// `PointeeType`.
+ void setNullPointerVal(QualType PointeeType, PointerValue ) {
+assert(NullPointerVals.find(PointeeType.getAsString()) ==
Since you always want this function to create a null pointer value, I think it
would be less error prone to ask for the location instead of an arbitrary
value. Currently, a confused caller could put a non-null value into a table.
Comment at:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:149
+ NullPointerVals.end());
+NullPointerVals[PointeeType.getAsString()] =
+ }
I think `getAsString` is considered expensive. Could you use `QualType`
directly as the key?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128056/new/
https://reviews.llvm.org/D128056
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D128056: [clang][dataflow] Singleton pointer values for null pointers.
wyt created this revision.
Herald added subscribers: martong, tschuett, xazax.hun.
Herald added a project: All.
wyt requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When a `nullptr` is assigned to a pointer variable, it is wrapped in a
`ImplicitCastExpr` with cast kind `CK_NullTo(Member)Pointer`. This patch
assigns singleton pointer values representing null to these expressions.
For each pointee type, a singleton null `PointerValue` is created and stored in
the `NullPointerVals` map of the `DataflowAnalysisContext` class. The pointee
type is retrieved from the implicit cast expression, and used to initialise the
`PointeeLoc` field of the `PointerValue`. The `PointeeLoc` created is not
mapped to any `Value`, reflecting the absence of value indicated by null
pointers.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D128056
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
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
@@ -2214,6 +2214,93 @@
});
}
+TEST_F(TransferTest, NullToPointerCast) {
+ std::string Code = R"(
+struct Baz {};
+void target() {
+ int *FooX = nullptr;
+ int *FooY = nullptr;
+ bool **Bar = nullptr;
+ Baz *Baz = nullptr;
+ // [[p]]
+}
+ )";
+ runDataflow(Code,
+ [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment = Results[0].second.Env;
+
+const ValueDecl *FooXDecl = findValueDecl(ASTCtx, "FooX");
+ASSERT_THAT(FooXDecl, NotNull());
+
+const ValueDecl *FooYDecl = findValueDecl(ASTCtx, "FooY");
+ASSERT_THAT(FooYDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const auto *FooXVal =
+cast(Env.getValue(*FooXDecl, SkipPast::None));
+const auto *FooYVal =
+cast(Env.getValue(*FooYDecl, SkipPast::None));
+const auto *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+const auto *BazVal =
+cast(Env.getValue(*BazDecl, SkipPast::None));
+
+EXPECT_EQ(FooXVal, FooYVal);
+EXPECT_NE(FooXVal, BarVal);
+EXPECT_NE(FooXVal, BazVal);
+EXPECT_NE(BarVal, BazVal);
+
+const StorageLocation = FooXVal->getPointeeLoc();
+EXPECT_TRUE(isa(FooPointeeLoc));
+EXPECT_THAT(Env.getValue(FooPointeeLoc), IsNull());
+
+const StorageLocation = BarVal->getPointeeLoc();
+EXPECT_TRUE(isa(BarPointeeLoc));
+EXPECT_THAT(Env.getValue(BarPointeeLoc), IsNull());
+
+const StorageLocation = BazVal->getPointeeLoc();
+EXPECT_TRUE(isa(BazPointeeLoc));
+EXPECT_THAT(Env.getValue(BazPointeeLoc), IsNull());
+ });
+}
+
+TEST_F(TransferTest, NullToMemberPointerCast) {
+ std::string Code = R"(
+struct Foo {};
+void target(Foo *Foo) {
+ int Foo::*MemberPointer = nullptr;
+ // [[p]]
+}
+ )";
+ runDataflow(
+ Code, [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext ) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment = Results[0].second.Env;
+
+const ValueDecl *MemberPointerDecl =
+findValueDecl(ASTCtx, "MemberPointer");
+ASSERT_THAT(MemberPointerDecl, NotNull());
+
+const auto *MemberPointerVal = cast(
+Env.getValue(*MemberPointerDecl, SkipPast::None));
+
+const StorageLocation = MemberPointerVal->getPointeeLoc();
+EXPECT_THAT(Env.getValue(MemberLoc), IsNull());
+ });
+}
+
TEST_F(TransferTest, AddrOfValue) {
std::string Code = R"(
void target() {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -246,6 +246,16 @@
Env.setStorageLocation(*S, *SubExprLoc);
