Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dangling false positive if the the owner is also moved in the initializer. #114201

Open
hokein opened this issue Oct 30, 2024 · 1 comment
Open
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer clang:memory-safety Issue/FR relating to the lifetime analysis in Clang (-Wdangling, -Wreturn-local-addr)

Comments

@hokein
Copy link
Collaborator

hokein commented Oct 30, 2024

This issue is identified in #112751.

// case1
namespace std {
template<typename T>
struct unique_ptr {
  T &operator*();
  T *get() const [[clang::lifetimebound]];
};
} // namespace std

struct X {
  X(std::unique_ptr<int> up) :
    pointer(up.get()), owner(std::move(up)) {}

  int *pointer;
  std::unique_ptr<int> owner;
};

When we add the clang::lifetimebound annotation to unique_ptr::get(), clang emits a dangling-field warning for the pointer(up.get()) member initializer. This warning is a false positive in this context, as the owner member is moved as part of the initialization, retaining ownership.

Another example occurs in designated-initializer cases:

// case2
struct X {
   int *pointer;
   std::unique_ptr<int> owner;
};

X func(std::unique_ptr<int> up) {
   return {
      .pointer = up.get(),
      .owner = std::move(up)
   };
}

Fixing these false positives is hard because it would require tracking dependencies between expressions, which is beyond the capabilities of the current statement-local analysis.

@hokein hokein added clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer clang:memory-safety Issue/FR relating to the lifetime analysis in Clang (-Wdangling, -Wreturn-local-addr) labels Oct 30, 2024
@hokein
Copy link
Collaborator Author

hokein commented Oct 30, 2024

Interestingly, clang will emit the diagnostic on case 2 even the get() is not annotated with lifetimebound, https://godbolt.org/z/vYeG9Eb3Y.

The GSL analysis implicitly tracks the *this object for some hard-coded GSL owner methods (vector.get()), the case 1 have been filtered out here, but it is just for the GSL code path.

hokein added a commit that referenced this issue Nov 1, 2024
…ber initializer. (#114213)

This patch extends the filtering heuristic to apply for the
Lifetimebound code path.

This will suppress a common false positive:

```
namespace std {
template<typename T>
struct unique_ptr {
  T &operator*();
  T *get() const [[clang::lifetimebound]];
};
} // namespace std

struct X {
  X(std::unique_ptr<int> up) :
    pointer(up.get()), owner(std::move(up)) {}

  int *pointer;
  std::unique_ptr<int> owner;
};
```

See #114201.
smallp-o-p pushed a commit to smallp-o-p/llvm-project that referenced this issue Nov 3, 2024
…ber initializer. (llvm#114213)

This patch extends the filtering heuristic to apply for the
Lifetimebound code path.

This will suppress a common false positive:

```
namespace std {
template<typename T>
struct unique_ptr {
  T &operator*();
  T *get() const [[clang::lifetimebound]];
};
} // namespace std

struct X {
  X(std::unique_ptr<int> up) :
    pointer(up.get()), owner(std::move(up)) {}

  int *pointer;
  std::unique_ptr<int> owner;
};
```

See llvm#114201.
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this issue Nov 4, 2024
…ber initializer. (llvm#114213)

This patch extends the filtering heuristic to apply for the
Lifetimebound code path.

This will suppress a common false positive:

```
namespace std {
template<typename T>
struct unique_ptr {
  T &operator*();
  T *get() const [[clang::lifetimebound]];
};
} // namespace std

struct X {
  X(std::unique_ptr<int> up) :
    pointer(up.get()), owner(std::move(up)) {}

  int *pointer;
  std::unique_ptr<int> owner;
};
```

See llvm#114201.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer clang:memory-safety Issue/FR relating to the lifetime analysis in Clang (-Wdangling, -Wreturn-local-addr)
Projects
None yet
Development

No branches or pull requests

1 participant