llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-analysis
@llvm/pr-subscribers-clang-temporal-safety
Author: Utkarsh Saxena (usx95)
<details>
<summary>Changes</summary>
Prevent false positives in lifetime safety analysis when variables are moved
using `std::move`.
When a value is moved using `std::move`, ownership is transferred from the
original variable to another. The lifetime safety analysis was previously
generating false positives by warning about use-after-lifetime when the
original variable was destroyed after being moved. This change prevents those
false positives by tracking moved declarations and exempting them from loan
expiration checks.
- Added tracking for declarations that have been moved via `std::move` in the
`FactsGenerator` class
- Added a `MovedDecls` set to track moved declarations in a flow-insensitive
manner
- Implemented detection of `std::move` calls in `VisitCallExpr`
- Modified `handleLifetimeEnds` to skip loans for declarations that have been
moved
- Added a test case demonstrating that warnings are suppressed for moved
variables
```cpp
void silenced() {
MyObj b;
View v;
{
MyObj a;
v = a;
b = std::move(a); // No warning for 'a' being moved.
}
(void)v;
}
```
Fixes https://github.com/llvm/llvm-project/issues/152520
---
Full diff: https://github.com/llvm/llvm-project/pull/170007.diff
3 Files Affected:
- (modified)
clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h (+5)
- (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+23)
- (modified) clang/test/Sema/warn-lifetime-safety.cpp (+18)
``````````diff
diff --git
a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
index 5b5626020e772..0c0581239ce34 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
@@ -101,6 +101,11 @@ class FactsGenerator : public
ConstStmtVisitor<FactsGenerator> {
// corresponding to the left-hand side is updated to be a "write", thereby
// exempting it from the check.
llvm::DenseMap<const DeclRefExpr *, UseFact *> UseFacts;
+
+ // Tracks declarations that have been moved via std::move. This is used to
+ // prevent false positives when the original owner is destroyed after the
+ // value has been moved. This tracking is flow-insensitive.
+ llvm::DenseSet<const ValueDecl *> MovedDecls;
};
} // namespace clang::lifetimes::internal
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index b27dcb6163449..ba88af2418056 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -163,9 +163,27 @@ void FactsGenerator::VisitCXXMemberCallExpr(const
CXXMemberCallExpr *MCE) {
}
}
+static bool isStdMove(const FunctionDecl *FD) {
+ return FD && FD->isInStdNamespace() && FD->getIdentifier() &&
+ FD->getName() == "move";
+}
+
void FactsGenerator::VisitCallExpr(const CallExpr *CE) {
handleFunctionCall(CE, CE->getDirectCallee(),
{CE->getArgs(), CE->getNumArgs()});
+ // Track declarations that are moved via std::move.
+ // This is a flow-insensitive approximation: once a declaration is moved
+ // anywhere in the function, it's treated as moved everywhere. This can lead
+ // to false negatives on control flow paths where the value is not actually
+ // moved, but these are considered lower priority than the false positives
+ // this tracking prevents.
+ // TODO: The ideal solution would be flow-sensitive ownership tracking that
+ // records where values are moved from and to, but this is more complex.
+ if (isStdMove(CE->getDirectCallee()))
+ if (CE->getNumArgs() == 1)
+ if (auto *DRE =
+ dyn_cast<DeclRefExpr>(CE->getArg(0)->IgnoreParenImpCasts()))
+ MovedDecls.insert(DRE->getDecl());
}
void FactsGenerator::VisitCXXNullPtrLiteralExpr(
@@ -341,6 +359,11 @@ void FactsGenerator::handleLifetimeEnds(const
CFGLifetimeEnds &LifetimeEnds) {
// Iterate through all loans to see if any expire.
for (const auto &Loan : FactMgr.getLoanMgr().getLoans()) {
const AccessPath &LoanPath = Loan.Path;
+ // Skip loans for declarations that have been moved. When a value is moved,
+ // the original owner no longer has ownership and its destruction should
not
+ // cause the loan to expire, preventing false positives.
+ if (MovedDecls.contains(LoanPath.D))
+ continue;
// Check if the loan is for a stack variable and if that variable
// is the one being destructed.
if (LoanPath.D == LifetimeEndsVD)
diff --git a/clang/test/Sema/warn-lifetime-safety.cpp
b/clang/test/Sema/warn-lifetime-safety.cpp
index f22c73cfeb784..97a79cc4ce102 100644
--- a/clang/test/Sema/warn-lifetime-safety.cpp
+++ b/clang/test/Sema/warn-lifetime-safety.cpp
@@ -1,9 +1,14 @@
// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety
-Wexperimental-lifetime-safety -Wno-dangling -verify %s
+#include "Inputs/lifetime-analysis.h"
+
struct View;
struct [[gsl::Owner]] MyObj {
int id;
+ MyObj();
+ MyObj(int);
+ MyObj(const MyObj&);
~MyObj() {} // Non-trivial destructor
MyObj operator+(MyObj);
@@ -1297,3 +1302,16 @@ void add(int c, MyObj* node) {
arr[4] = node;
}
} // namespace CppCoverage
+
+namespace do_not_warn_on_std_move {
+void silenced() {
+ MyObj b;
+ View v;
+ {
+ MyObj a;
+ v = a;
+ b = std::move(a); // No warning for 'a' being moved.
+ }
+ (void)v;
+}
+} // namespace do_not_warn_on_std_move
``````````
</details>
https://github.com/llvm/llvm-project/pull/170007
_______________________________________________
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
