https://github.com/hokein created
https://github.com/llvm/llvm-project/pull/104906
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
>From 773a03b25a94d991206f4b517eefdf3693e6a287 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein...@gmail.com>
Date: Tue, 20 Aug 2024 10:22:44 +0200
Subject: [PATCH] [clang] Merge the lifetimebound and GSL code paths for
lifetime analysis.
---
clang/docs/ReleaseNotes.rst | 2 +
clang/lib/Sema/CheckExprLifetime.cpp | 129 ++++++++----------
.../Sema/warn-lifetime-analysis-nocfg.cpp | 13 ++
3 files changed, 72 insertions(+), 72 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 249249971dec7c..924968452819d1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -225,6 +225,8 @@ Improvements to Clang's diagnostics
- Clang now diagnoses when the result of a [[nodiscard]] function is discarded
after being cast in C. Fixes #GH104391.
+- Don't emit duplicated dangling diagnostics. (#GH93386).
+
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp
b/clang/lib/Sema/CheckExprLifetime.cpp
index 7389046eaddde1..06eb909659d05d 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -326,66 +326,6 @@ static bool shouldTrackFirstArgument(const FunctionDecl
*FD) {
return false;
}
-static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
- LocalVisitor Visit) {
- auto VisitPointerArg = [&](const Decl *D, Expr *Arg, bool Value) {
- // We are not interested in the temporary base objects of gsl Pointers:
- // Temp().ptr; // Here ptr might not dangle.
- if (isa<MemberExpr>(Arg->IgnoreImpCasts()))
- return;
- // Once we initialized a value with a reference, it can no longer dangle.
- if (!Value) {
- for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) {
- if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit)
- continue;
- if (PE.Kind == IndirectLocalPathEntry::GslPointerInit ||
- PE.Kind == IndirectLocalPathEntry::GslPointerAssignment)
- return;
- break;
- }
- }
- Path.push_back({Value ? IndirectLocalPathEntry::GslPointerInit
- : IndirectLocalPathEntry::GslReferenceInit,
- Arg, D});
- if (Arg->isGLValue())
- visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
- Visit,
- /*EnableLifetimeWarnings=*/true);
- else
- visitLocalsRetainedByInitializer(Path, Arg, Visit, true,
- /*EnableLifetimeWarnings=*/true);
- Path.pop_back();
- };
-
- if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Call)) {
- const auto *MD = cast_or_null<CXXMethodDecl>(MCE->getDirectCallee());
- if (MD && shouldTrackImplicitObjectArg(MD))
- VisitPointerArg(MD, MCE->getImplicitObjectArgument(),
- !MD->getReturnType()->isReferenceType());
- return;
- } else if (auto *OCE = dyn_cast<CXXOperatorCallExpr>(Call)) {
- FunctionDecl *Callee = OCE->getDirectCallee();
- if (Callee && Callee->isCXXInstanceMember() &&
- shouldTrackImplicitObjectArg(cast<CXXMethodDecl>(Callee)))
- VisitPointerArg(Callee, OCE->getArg(0),
- !Callee->getReturnType()->isReferenceType());
- return;
- } else if (auto *CE = dyn_cast<CallExpr>(Call)) {
- FunctionDecl *Callee = CE->getDirectCallee();
- if (Callee && shouldTrackFirstArgument(Callee))
- VisitPointerArg(Callee, CE->getArg(0),
- !Callee->getReturnType()->isReferenceType());
- return;
- }
-
- if (auto *CCE = dyn_cast<CXXConstructExpr>(Call)) {
- const auto *Ctor = CCE->getConstructor();
- const CXXRecordDecl *RD = Ctor->getParent();
- if (CCE->getNumArgs() > 0 && RD->hasAttr<PointerAttr>())
- VisitPointerArg(Ctor->getParamDecl(0), CCE->getArgs()[0], true);
- }
-}
-
static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
if (!TSI)
@@ -423,8 +363,10 @@ static bool implicitObjectParamIsLifetimeBound(const
FunctionDecl *FD) {
return false;
}
-static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
- LocalVisitor Visit) {
+// Visit lifetimebound or gsl-pointer arguments.
+static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
+ LocalVisitor Visit,
+ bool EnableLifetimeWarnings) {
const FunctionDecl *Callee;
ArrayRef<Expr *> Args;
@@ -458,6 +400,35 @@ static void visitLifetimeBoundArguments(IndirectLocalPath
&Path, Expr *Call,
/*EnableLifetimeWarnings=*/false);
Path.pop_back();
};
+ auto VisitGSLPointerArg = [&](const Decl *D, Expr *Arg, bool Value) {
+ // We are not interested in the temporary base objects of gsl Pointers:
+ // Temp().ptr; // Here ptr might not dangle.
+ if (isa<MemberExpr>(Arg->IgnoreImpCasts()))
+ return;
+ // Once we initialized a value with a reference, it can no longer dangle.
+ if (!Value) {
+ for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) {
+ if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit)
+ continue;
+ if (PE.Kind == IndirectLocalPathEntry::GslPointerInit ||
+ PE.Kind == IndirectLocalPathEntry::GslPointerAssignment)
+ return;
+ break;
+ }
+ }
+ Path.push_back({Value ? IndirectLocalPathEntry::GslPointerInit
+ : IndirectLocalPathEntry::GslReferenceInit,
+ Arg, D});
+ if (Arg->isGLValue())
+ visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
+ Visit,
+ /*EnableLifetimeWarnings=*/true);
+ else
+ visitLocalsRetainedByInitializer(Path, Arg, Visit, true,
+ /*EnableLifetimeWarnings=*/true);
+ Path.pop_back();
+ };
+
bool CheckCoroCall = false;
if (const auto *RD = Callee->getReturnType()->getAsRecordDecl()) {
@@ -478,6 +449,12 @@ static void visitLifetimeBoundArguments(IndirectLocalPath
&Path, Expr *Call,
CheckCoroObjArg = false;
if (implicitObjectParamIsLifetimeBound(Callee) || CheckCoroObjArg)
VisitLifetimeBoundArg(Callee, ObjectArg);
+ else if (EnableLifetimeWarnings) {
+ if (auto *CME = dyn_cast<CXXMethodDecl>(Callee);
+ CME && shouldTrackImplicitObjectArg(CME))
+ VisitGSLPointerArg(Callee, ObjectArg,
+ !Callee->getReturnType()->isReferenceType());
+ }
}
for (unsigned I = 0,
@@ -485,6 +462,19 @@ static void visitLifetimeBoundArguments(IndirectLocalPath
&Path, Expr *Call,
I != N; ++I) {
if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>())
VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]);
+ else if (EnableLifetimeWarnings) {
+ if (I == 0) {
+ if (shouldTrackFirstArgument(Callee)) { // gsl
+ VisitGSLPointerArg(Callee, Args[0],
+ !Callee->getReturnType()->isReferenceType());
+ } else {
+ if (auto *CCE = dyn_cast<CXXConstructExpr>(Call);
+ CCE &&
CCE->getConstructor()->getParent()->hasAttr<PointerAttr>())
+ VisitGSLPointerArg(CCE->getConstructor()->getParamDecl(0), Args[0],
+ true);
+ }
+ }
+ }
}
}
@@ -557,11 +547,9 @@ static void
visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
EnableLifetimeWarnings);
}
- if (isa<CallExpr>(Init)) {
- if (EnableLifetimeWarnings)
- handleGslAnnotatedTypes(Path, Init, Visit);
- return visitLifetimeBoundArguments(Path, Init, Visit);
- }
+ if (isa<CallExpr>(Init))
+ return visitFunctionCallArguments(Path, Init, Visit,
+ EnableLifetimeWarnings);
switch (Init->getStmtClass()) {
case Stmt::DeclRefExprClass: {
@@ -835,11 +823,8 @@ static void
visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
}
}
- if (isa<CallExpr>(Init) || isa<CXXConstructExpr>(Init)) {
- if (EnableLifetimeWarnings)
- handleGslAnnotatedTypes(Path, Init, Visit);
- return visitLifetimeBoundArguments(Path, Init, Visit);
- }
+ if (isa<CallExpr>(Init) || isa<CXXConstructExpr>(Init))
+ return visitFunctionCallArguments(Path, Init, Visit,
EnableLifetimeWarnings);
switch (Init->getStmtClass()) {
case Stmt::UnaryOperatorClass: {
diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index 09dfb2b5d96a89..86ee90ed6df8dd 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -479,3 +479,16 @@ void testForBug49342()
{
auto it = std::iter<char>{} - 2; // Used to be false positive.
}
+
+namespace GH93386 {
+// verify no duplicated diagnostics are emitted.
+struct [[gsl::Pointer]] S {
+ S(const std::vector<int>& abc [[clang::lifetimebound]]);
+};
+
+S test(std::vector<int> a) {
+ return S(a); // expected-warning {{address of stack memory associated with}}
+}
+
+auto s = S(std::vector<int>()); // expected-warning {{temporary whose address
is used as value of local variable}}
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
