Only overwrite destination points-to-set if we have a single object on the LHS.
Adds multiple tests; of these, Overwrite_MultipleDestinations fails before this
change.
PiperOrigin-RevId: 453662886
diff --git a/lifetime_analysis/lifetime_analysis.cc b/lifetime_analysis/lifetime_analysis.cc
index 212f9ce..9a9f002 100644
--- a/lifetime_analysis/lifetime_analysis.cc
+++ b/lifetime_analysis/lifetime_analysis.cc
@@ -417,11 +417,18 @@
if (!op->getLHS()->getType()->isPointerType()) break;
ObjectSet rhs_points_to = points_to_map_.GetExprObjectSet(op->getRHS());
for (Object pointer : lhs_points_to) {
- if (object_repository_.GetObjectValueType(pointer) ==
- ObjectRepository::ObjectValueType::kMultiValued) {
- points_to_map_.ExtendPointerPointsToSet(pointer, rhs_points_to);
- } else {
+ // We can overwrite (instead of extend) the destination points-to-set
+ // only in very specific circumstances:
+ // - We need to know unambiguously what the LHS refers to, so that we
+ // know we're definitely writing to a particular object, and
+ // - That destination object needs to be "single-valued" (it can't be
+ // an array, for example).
+ if (lhs_points_to.size() == 1 &&
+ object_repository_.GetObjectValueType(pointer) ==
+ ObjectRepository::ObjectValueType::kSingleValued) {
points_to_map_.SetPointerPointsToSet(pointer, rhs_points_to);
+ } else {
+ points_to_map_.ExtendPointerPointsToSet(pointer, rhs_points_to);
}
}
break;
diff --git a/lifetime_analysis/test/basic.cc b/lifetime_analysis/test/basic.cc
index d442510..d9f2f84 100644
--- a/lifetime_analysis/test/basic.cc
+++ b/lifetime_analysis/test/basic.cc
@@ -456,6 +456,54 @@
LifetimesAre({{"f", "a -> a"}, {"target", "a -> a"}}));
}
+TEST_F(LifetimeAnalysisTest, Overwrite_SingleDestination) {
+ EXPECT_THAT(GetLifetimes(R"(
+ int* target(int* a, int* b) {
+ int** pp = &b;
+ // There is only one thing that `pp` can be pointing at, so the analysis
+ // should conclude that `b` is being overwritten with `a`.
+ *pp = a;
+ return b;
+ }
+ )"),
+ LifetimesAre({{"target", "a, b -> a"}}));
+}
+
+TEST_F(LifetimeAnalysisTest, Overwrite_SingleDestinationVariant) {
+ EXPECT_THAT(GetLifetimes(R"(
+ // Similar to above, but potentially leave `pp` uninitialized.
+ int* target(int* a, int* b) {
+ int** pp;
+ if (*a > 0) {
+ pp = &b;
+ }
+ // If `pp` is uninitialized, the following is UB, so the analysis can
+ // assume that `pp` was initialized to point to `b`.
+ // This particular test function is pretty terrible style, but it seems
+ // plausible that similar situations can come up in more reasonable code.
+ *pp = a;
+ return b;
+ }
+ )"),
+ LifetimesAre({{"target", "a, b -> a"}}));
+}
+
+TEST_F(LifetimeAnalysisTest, Overwrite_MultipleDestinations) {
+ EXPECT_THAT(GetLifetimes(R"(
+ // This is a regression test. The analysis used to conclude falsely that `b`
+ // was unconditionally being overwritten with `a` in the assignment and was
+ // therefore producing the wrong lifetimes "a, b -> a".
+ int* target(int* a, int* b) {
+ int** pp = *a > 0? &a : &b;
+ // The analysis should understand that the following assignment _might_
+ // overwrite `b` with `a` but does not necessarily do so.
+ *pp = a;
+ return b;
+ }
+ )"),
+ LifetimesAre({{"target", "a, a -> a"}}));
+}
+
} // namespace
} // namespace lifetimes
} // namespace tidy
