[clang] [llvm] [clang] Implement lifetime analysis for lifetime_capture_by(X) (PR #115921)

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 18 00:41:58 PST 2024


================
@@ -793,3 +793,108 @@ void test13() {
 }
 
 } // namespace GH100526
+
+namespace lifetime_capture_by {
+struct S {
+  const int *x;
+  void captureInt(const int&x [[clang::lifetime_capture_by(this)]]) { this->x = &x; }
+  void captureSV(std::string_view sv [[clang::lifetime_capture_by(this)]]);
+};
+///////////////////////////
+// Detect dangling cases.
+///////////////////////////
+void captureInt(const int&x [[clang::lifetime_capture_by(s)]], S&s);
+void captureRValInt(int&&x [[clang::lifetime_capture_by(s)]], S&s);
+void noCaptureInt(int x [[clang::lifetime_capture_by(s)]], S&s);
+std::string_view substr(const std::string& s [[clang::lifetimebound]]);
+std::string_view strcopy(const std::string& s);
+void captureSV(std::string_view x [[clang::lifetime_capture_by(s)]], S&s);
+void captureRValSV(std::string_view&& x [[clang::lifetime_capture_by(s)]], S&s);
+void noCaptureSV(std::string_view x, S&s);
+void captureS(const std::string& x [[clang::lifetime_capture_by(s)]], S&s);
+void captureRValS(std::string&& x [[clang::lifetime_capture_by(s)]], S&s);
+const std::string* getPointerLB(const std::string& s[[clang::lifetimebound]]);
+const std::string* getPointerNoLB(const std::string& s);
+void capturePointer(const std::string* x [[clang::lifetime_capture_by(s)]], S&s);
+struct ThisIsCaptured {
+  void capture(S& s) [[clang::lifetime_capture_by(s)]];
+  void bar(S& s) [[clang::lifetime_capture_by(abcd)]]; // expected-error {{'lifetime_capture_by' attribute argument 'abcd' is not a known function parameter}}
+  void baz(S& s) [[clang::lifetime_capture_by(this)]]; // expected-error {{'lifetime_capture_by' argument references itself}}
+};
+void use() {
+  std::string_view local_sv;
+  std::string local_s;
+  S s;
+  // Capture an 'int'.
+  int local;
+  captureInt(1, // expected-warning {{object captured by 's' will be destroyed at the end of the full-expression}}
+            s);
+  captureRValInt(1, s); // expected-warning {{object captured by 's'}}
+  captureInt(local, s);
+  noCaptureInt(1, s);
+  noCaptureInt(local, s);
+
+  // Capture lifetimebound pointer.
+  capturePointer(getPointerLB(std::string()), s); // expected-warning {{object captured by 's'}}
+  capturePointer(getPointerLB(*getPointerLB(std::string())), s); // expected-warning {{object captured by 's'}}
+  capturePointer(getPointerNoLB(std::string()), s);
+
+  // Capture using std::string_view.
+  captureSV(local_sv, s);
+  captureSV(std::string(), // expected-warning {{object captured by 's'}}
+            s);
+  captureSV(substr(
+      std::string() // expected-warning {{object captured by 's'}}
+      ), s);
+  captureSV(substr(local_s), s);
+  captureSV(strcopy(std::string()), s);
+  captureRValSV(std::move(local_sv), s);
+  captureRValSV(std::string(), s); // expected-warning {{object captured by 's'}}
+  captureRValSV(std::string_view{"abcd"}, s);
+  captureRValSV(substr(local_s), s);
+  captureRValSV(substr(std::string()), s); // expected-warning {{object captured by 's'}}
+  captureRValSV(strcopy(std::string()), s);
+  noCaptureSV(local_sv, s);
+  noCaptureSV(std::string(), s);
+  noCaptureSV(substr(std::string()), s);
+
+  // Capture using std::string.
+  captureS(std::string(), s); // expected-warning {{object captured by 's'}}
+  captureS(local_s, s);
+  captureRValS(std::move(local_s), s);
+  captureRValS(std::string(), s); // expected-warning {{object captured by 's'}}
+
+  // Member functions.
+  s.captureInt(1); // expected-warning {{object captured by 's'}}
+  s.captureSV(std::string()); // expected-warning {{object captured by 's'}}
+  s.captureSV(substr(std::string())); // expected-warning {{object captured by 's'}}
+  s.captureSV(strcopy(std::string()));
+
+  // 'this' is captured.
+  ThisIsCaptured{}.capture(s); // expected-warning {{object captured by 's'}}
+  ThisIsCaptured TIS;
+  TIS.capture(s);
+}
+class [[gsl::Pointer()]] my_string_view : public std::string_view {};
+class my_string_view_not_pointer : public std::string_view {};
+std::optional<std::string_view> getOptionalSV();
+std::optional<std::string> getOptionalS();
+std::optional<my_string_view> getOptionalMySV();
+std::optional<my_string_view_not_pointer> getOptionalMySVNotP();
+my_string_view getMySV();
+my_string_view_not_pointer getMySVNotP();
+
+template<class T>
+struct MySet {
+void insert(T&& t [[clang::lifetime_capture_by(this)]]);
+void insert(const T& t [[clang::lifetime_capture_by(this)]]);
+};
+void user_defined_containers() {
+  MySet<int> set_of_int;
+  set_of_int.insert(1); // expected-warning {{object captured by 'set_of_int' will be destroyed}}
----------------
hokein wrote:

I believe the current behavior is working as intended, although it doesn't align with user expectations.

This issue arises when involving a `gsl::pointer` type with a templated constructor that takes a const reference:

```cpp
#include <vector>

template <typename T>
struct [[gsl::Pointer]] MySpan2 {
   template <typename Container>
   MySpan2(const Container& container);
};

template <typename T>
struct [[gsl::Pointer]] MySpan {
   template <typename Container>
   MySpan(const Container& container [[clang::lifetimebound]]);
};

void test() {
   MySpan<int> t1(std::vector{1}); // 1) warn
   
   std::vector<int> v;
   MySpan<int> t2 = MySpan2<int>(v); // 2) warn on MySpan2, but we expect no warning
   MySpan<int> t3 = MySpan2<int>(std::vector<int>{}); // 3) warn on MySpan2, but we expect warn on the std::vector<int>
}
```

Per the `lifetimebound` documentation, for a reference type, Clang considers it referenced type -- in case 2), this is `MySpan2` itself. However, this mismatch user expectations. We want `t2` to be `lifetimebound` to the underlying storage of `MySpan2` (which is `v`). And specifically in case 3), we intend to diagnose the temporary `std::vector<int>`, rather than flagging the temporary `MySpan2` object.

Some options:

1. **Add specific overloads** for pointer types that are not annotated with `lifetimebound`. (This is an approach we've used internally);
2. **Adjust `lifetimebound` analysis** to accommodate `gsl::pointer` types. Specifically, for a reference to a GSL pointer type (e.g., `const MyPointer&`), we could consider its pointee instead. However, this has trade-offs: it would prevent diagnostics when the GSL pointer itself is dangling, I think this is probably fine, this is not an important case.
3. **Avoid using `lifetimebound` in the constructor** and use `GSLOwner` instead. When a `GSLPointer` is constructed from a `GSLOwner` object, Clang will diagnose if the `GSLOwner` is a temporary. In the above, removing `clang::lifetimebound` would address these issues correctly https://godbolt.org/z/Mq6rKhz99 (`std::vector` is implicitly annotated as `gsl::Owner` by default).

CC @higher-performance, who might have some thoughts on this.

https://github.com/llvm/llvm-project/pull/115921


More information about the cfe-commits mailing list