[clang] [clang] Don't consider the lifetimeboundCall when analyzing the gsl pointer construction. (PR #114044)

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 29 09:56:20 PDT 2024


================
@@ -1094,6 +1094,24 @@ static bool pathOnlyHandlesGslPointer(IndirectLocalPath &Path) {
   return false;
 }
 
+static bool
+isLifetimeboundInterleaveInGSL(llvm::ArrayRef<IndirectLocalPathEntry> PathRef) {
----------------
hokein wrote:


> I am just wondering if this is the right approach to filter this problem out.
> 
> I think in the problematic code example `[[clang::lifetimebound]]` describes the lifetime of the returned reference, not the lifetime of the `gsl::pointer` annotated type. So, I think the mismatch is trying to apply the lifetimebound logic to the `gsl::pointer` in this particular example.

>From an implementation perspective, we use the `lifetimebound` attribute to decide whether to continue traversing inner AST nodes (the return type doesn’t matter; it only affects the emitted diagnostic message). This process of each step is stateless -- it does not account for whether the existing path has been initialized for GSL.

Back to the problematic case,

```
class [[gsl::Pointer]] FooPointer {};

template<typename T>
class [[gsl::Owner]] Container {
public:
  const T& get() const [[clang::lifetimebound]];
};

FooPointer abc2() {
  Container<FooPointer> foo;
  return foo.get(); // Clang emits a return-stack-address warning on `foo`.
}
```
When we report the local `foo` entity in the TemporaryVisitor callback:

- The Path is [GslPointerInit, Lifetimebound]
- foo is of type `Container<FooPointer>`

The code incorrectly interprets that the `FooPointer` is constructed from `foo`, which is causing the issue here. I think it is essentially a bug in the code that analyzes the gsl::pointer from gsl::owner construction case (addressing this in the existing gsl::pointer filtering logic seems like a natural approach, though it introduces new false negatives).

> I think my point is, probably not all interleavings are bad, but here the `gsl::pointer` and the `lifetimebound` apply to different levels of indirection and I am wondering it that needs to be part of our filtering.

I completely agree that not all interleavings are bad, and there are some useful examples of where they work well. Better ideas are welcome.

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


More information about the cfe-commits mailing list