C++23 mandates that temporaries used in range-based for loops are
lifetime-extended
to cover the full loop. This patch adds a check for loop variables and
compiler-
generated `__range` bindings to apply the correct extension.
Includes test cases based on examples from CWG900/P2644R1.
Fixes https://github.com/llvm/llvm-project/issues/109793
Microsoft helpfully defines `THIS` to `void` in two different platform
SDK headers, at least one of which is reachable via <Windows.h>. We have
a user who ran into a build because of `THIS` unfortunate macro name
collision.
Rename the members to better match our naming conventions.
Fixes#142186
This fixes a false positive caused by #114044.
For `GSLPointer*` types, it's less clear whether the lifetime issue is
about the GSLPointer object itself or the owner it points to. To avoid
false positives, we take a conservative approach in our heuristic.
Fixes#127195
(This will be backported to release 20).
We currently have ad-hoc filtering logic for temporary object member
access in `VisitGSLPointerArg`. This logic filters out more cases than
it should, leading to false negatives. Furthermore, this location lacks
sufficient context to implement a more accurate solution.
This patch refines the filtering logic by moving it to the central
filtering location, `analyzePathForGSLPointer`, consolidating the logic
and avoiding scattered filtering across multiple places. As a result,
the special handling for conditional operators (#120233) is no longer
necessary.
This change also resolves#120543.
This partially fixes#62072 by making sure that re-declarations of a
function do not have the effect of removing lifetimebound from the
canonical declaration.
It doesn't handle the implicit 'this' parameter, but that can be
addressed in a separate fix.
When a vector is instantiated with a pointer type (`T` being `const
Foo*`), the inferred annotation becomes `push_back(const Foo*& value
[[clang::lifetime_capture_by(this)]])`.
For reference parameters, the `lifetime_capture_by` attribute treats the
lifetime as referring to the referenced object -- in this case, the
**pointer** itself, not the pointee object. In the `push_back`, we copy
the pointer's value, which does not establish a reference to the
pointer. This behavior is safe and does not capture the pointer's
lifetime.
The annotation should not be inferred for cases where `T` is a pointer
type, as the intended semantics do not align with the annotation.
Fixes#121391
When analyzing a dangling gsl pointer, we currently filter out all field
access `MemberExpr` to avoid common false positives (`string_view sv =
Temp().sv`), However, this filter only applies to direct MemberExpr
instances, leaving the conditional operator as an escaping example
(`GSLPointer pointer(Cond ? Owner().ptr : GSLPointer());`).
This patch extends the MemberExpr logic to handle the conditional
operator. The heuristic is intentionally simple, which may result in
some false negatives. However, it effectively covers common cases like
`std::string_view sv = cond ? "123" : std::string();`, which is a
reasonable trade-off.
Fixes https://github.com/llvm/llvm-project/issues/120206
The lifetime analyzer processes GSL pointers:
- when encountering a constructor for a `gsl::pointer`, the analyzer
continues traversing the constructor argument, regardless of whether the
parameter has a `lifetimebound` annotation. This aims to catch cases
where a GSL pointer is constructed from a GSL owner, either directly
(e.g., `FooPointer(FooOwner)`) or through a chain of GSL pointers (e.g.,
`FooPointer(FooPointer(FooOwner))`);
- When a temporary object is reported in the callback, the analyzer has
heuristics to exclude non-owner types, aiming to avoid false positives
(like `FooPointer(FooPointer())`).
In the problematic case (discovered in
https://github.com/llvm/llvm-project/pull/112751#issuecomment-2441055471)
of `return foo.get();`:
- When the analyzer reports the local object `foo`, the `Path` is
`[GslPointerInit, Lifetimebound]`.
- The `Path` goes through
[`pathOnlyHandlesGslPointer`](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/CheckExprLifetime.cpp#L1136)
and isn’t filtered out by the [[heuristics]](because `foo` is an owner
type), the analyzer treats it as the `FooPointer(FooOwner())` scenario,
thus triggering a diagnostic.
Filtering out base on the object 'foo' is wrong, because the GSLPointer
is constructed from the return result of the `foo.get()`. The patch
fixes this by teaching the heuristic to use the return result (only
`const GSLOwner&` is considered) of the lifetimebound annotated
function.
A specialization declaration can have an attribute even if the primary
template does not, particularly when the specialization is instantiated
from an annotated using-alias declaration.
Fix#118064
Unlike brace initialization, the parenthesized aggregate initialization
in C++20 does not extend the lifetime of a temporary object bound to a
reference in an aggreate. This can lead to dangling references:
```
struct A { const int& r; };
A a1(1); // well-formed, but results in a dangling reference.
```
With this patch, clang will diagnose this common dangling issues.
Fixes#101957
With this change, the lifetime_capture_by code path will not handle the
constructor decl to avoid bogus diagnostics (see the testcase).
Instead, we reuse the lifetimebound code as the
lifetime_capture_by(this) has the same semantic as lifetimebound in
constructor. The downside is that the lifetimebound diagnostic is reused
for the capture case (I think it is not a big issue).
Fixes#117680
This patch correctes the handling of non-member functions in the
`isNormalAssignmentOperator` function within `CheckExprLifetime.cpp`.
The previous implementation incorrectly assumed that `FunctionDecl` is
always a `CXXMethodDecl`, leading to potential null pointer
dereferencing.
Change: - Correctly handle the case where `FD` is not a `CXXMethodDecl`
by using `FD->getParamDecl(0)->getType()`.
This fix ensures that the function correctly handles non-member
assignment operators, such as:
`struct S {}; void operator|=(S, S) {}`
This change improves the robustness of the `isNormalAssignmentOperator`
function by correctly identifying and handling different types of
function declarations.
This PR uses the existing lifetime analysis for the `capture_by`
attribute.
The analysis is behind `-Wdangling-capture` warning and is disabled by
default for now. Once it is found to be stable, it will be default
enabled.
Planned followup:
- add implicit inference of this attribute on STL container methods like
`std::vector::push_back`.
- (consider) warning if capturing `X` cannot capture anything. It should
be a reference, pointer or a view type.
- refactoring temporary visitors and other related handlers.
- start discussing `__global` vs `global` in the annotation in a
separate PR.
---------
Co-authored-by: Boaz Brickner <brickner@google.com>
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.
This is a follow-up to https://github.com/llvm/llvm-project/pull/108344.
The original bailout check was overly strict, causing it to miss cases
like the vector(initializer_list, allocator) constructor. This patch
relaxes the check to address that issue.
Fix#111680
The lifetimes of local variables and function parameters must end before
the call to a [[clang::musttail]] function, instead of before the
return, because we will not have a stack frame to hold them when doing
the call.
This documents this limitation, and adds diagnostics to warn about some
code which is invalid because of it.
In the GSL analysis, we don't track the `this` object if the conversion
is not from gsl::owner to gsl pointer, we want to be conservative here
to avoid triggering false positives.
Fixes#108272
The PR reapply https://github.com/llvm/llvm-project/pull/97308.
- Implement [CWG1815](https://wg21.link/CWG1815): Support lifetime
extension of temporary created by aggregate initialization using a
default member initializer.
- Fix crash that introduced in
https://github.com/llvm/llvm-project/pull/97308. In
`InitListChecker::FillInEmptyInitForField`, when we enter
rebuild-default-init context, we copy all the contents of the parent
context to the current context, which will cause the `MaybeODRUseExprs`
to be lost. But we don't need to copy the entire context, only the
`DelayedDefaultInitializationContext` was required, which is used to
build `SourceLocExpr`, etc.
---------
Signed-off-by: yronglin <yronglin777@gmail.com>
This pull request enhances the GSL lifetime analysis to detect
situations where a dangling `Container<GSLPointer>` object is
constructed:
```cpp
std::vector<std::string_view> bad = {std::string()}; // dangling
```
The assignment case is not yet supported, but they will be addressed in
a follow-up.
Fixes#100526 (excluding the `push_back` case).
This reverts commit 45c8766973bb3bb73dd8d996231e114dcf45df9f
and 049512e39d96995cb373a76cf2d009a86eaf3aab.
This change triggers failed asserts on inputs like this:
struct a {
} constexpr b;
class c {
public:
c(a);
};
class B {
public:
using d = int;
struct e {
enum { f } g;
int h;
c i;
d j{};
};
};
B::e k{B::e::f, int(), b};
Compiled like this:
clang -target x86_64-linux-gnu -c repro.cpp
clang: ../../clang/lib/CodeGen/CGExpr.cpp:3105: clang::CodeGen::LValue
clang::CodeGen::CodeGenFunction::EmitDeclRefLValue(const clang::DeclRefExpr*):
Assertion `(ND->isUsed(false) || !isa<VarDecl>(ND) || E->isNonOdrUse() ||
!E->getLocation().isValid()) && "Should not use decl without marking it used!"' failed.
The warning is not emitted for the case `string_view c =
std::vector<std::string>({""}).at(0);` because we bail out during the
visit of the LHS at [this
point](5d2c324fea/clang/lib/Sema/CheckExprLifetime.cpp (L341-L343))
in the code.
This bailout was introduced in [this
commit](bcd0798c47)
to address a false positive with
`vector<vector::iterator>({""}).at(0);`. This PR refines that fix by
ensuring that, for initialization involving a gsl-pointer, we only
consider constructor calls that take the gsl-owner object.
Fixes#100384.
This allows clang to detect more use-after-free bugs (shown in the
#100549).
This relands the remaining change (removing the EnableLifetimeWarnings
flag) in https://github.com/llvm/llvm-project/pull/104906, with a proper
fix for the regression.
Fixes#100549
Reland without the `EnableLifetimeWarnings` removal. I will remove the
EnableLifetimeWarnings in a follow-up patch.
I have added a test to prevent regression.
In the current lifetime analysis, we have two parallel code paths: one
for lifetimebound and another for GSL. These paths perform the same
logic, both determining whether to continue visiting subexpressions.
This PR merges the two paths into a single code path. As a result, we'll
reduce the overhead by eliminating a redundant visit to subexpressions.
The change is mostly NFC (No Functional Change). The only notable
difference is that when a subexpression is visited due to either
lifetimebound or GSL, we will prioritize the lifetimebound path. This
means the final diagnostic will be -Wdangling (rather than both
`-Wdangling` and `-Wdangling-gsl`)
This might cause a slight change in behavior if the -Wdangling
diagnostic is disabled, but I think this is not a major concern since
both diagnostics are enabled by default.
Fixes#93386
This is a follow-up patch to #96475 to detect dangling assignments for
C++ pointer-like objects (classes annotated with the
`[[gsl::Pointer]]`). Fixes#63310.
Similar to the behavior for built-in pointer types, if a temporary owner
(`[[gsl::Owner]]`) object is assigned to a pointer-like class object,
and this temporary object is destroyed at the end of the full assignment
expression, the assignee pointer is considered dangling. In such cases,
clang will emit a warning:
```
/tmp/t.cpp:7:20: warning: object backing the pointer my_string_view will be destroyed at the end of the full-expression [-Wdangling-assignment-gsl]
7 | my_string_view = CreateString();
| ^~~~~~~~~~~~~~
1 warning generated.
```
This new warning is `-Wdangling-assignment-gsl`. It is initially
disabled, but I intend to enable it by default in clang 20.
I have initially tested this patch on our internal codebase, and it has
identified many use-after-free bugs, primarily related to `string_view`.
The current implementation for the assignment case uses a combination of
the `LK_Extended` lifetime kind and the validity of `AEntity`, which is
somewhat messy and doesn't align well with the intended mental model.
This patch introduces a dedicated lifetime kind to handle the assignment
case, simplifying the implementation and improving clarity.
The `lifetime_pointer` case is handled before the assignment case. In
scenarios where we have the `gsl::Pointer` attribute, we may emit the
`-Wdangling-gsl` warning for assignment cases. This means we cannot use
`-Wno-dangling-assignment` to suppress the newly-added warning, this
patch fixes it.
The lifetime bound warning in Clang currently only considers
initializations. This patch extends the warning to include assignments.
- **Support for assignments of built-in pointer types**: this is done is
by reusing the existing statement-local implementation. Clang now warns
if the pointer is assigned to a temporary object that being destoryed at
the end of the full assignment expression.
With this patch, we will detect more cases under the on-by-default
diagnostic `-Wdangling`. I have added a new category for this specific
diagnostic so that people can temporarily disable it if their codebase
is not yet clean.
This is the first step to address #63310, focusing only on pointer
types. Support for C++ assignment operators will come in a follow-up
patch.
Fixes#54492
