[clang] b1560bd - Reland "[clang] Merge lifetimebound and GSL code paths for lifetime analysis (#104906)" (#105838)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 23 08:50:30 PDT 2024
Author: Haojian Wu
Date: 2024-08-23T17:50:27+02:00
New Revision: b1560bdb2bc67006f3b8f7e84ee0356632bf8126
URL: https://github.com/llvm/llvm-project/commit/b1560bdb2bc67006f3b8f7e84ee0356632bf8126
DIFF: https://github.com/llvm/llvm-project/commit/b1560bdb2bc67006f3b8f7e84ee0356632bf8126.diff
LOG: Reland "[clang] Merge lifetimebound and GSL code paths for lifetime analysis (#104906)" (#105838)
Reland without the `EnableLifetimeWarnings` removal. I will remove the
EnableLifetimeWarnings in a follow-up patch.
I have added a test to prevent regression.
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/CheckExprLifetime.cpp
clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index baedc3cd6f03fc..17a707102d041f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -234,6 +234,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).
+
- Improved diagnostic when trying to befriend a concept. (#GH45182).
Improvements to Clang's time-trace
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 7389046eaddde1..c1362559536962 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,34 @@ 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 +448,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 +461,17 @@ 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 && I == 0) {
+ if (shouldTrackFirstArgument(Callee)) {
+ 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 +544,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 +820,9 @@ 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..cd1904db327105 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -479,3 +479,23 @@ 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}}
+
+// Verify no regression on the follow case.
+std::string_view test2(int i, std::optional<std::string_view> a) {
+ if (i)
+ return std::move(*a);
+ return std::move(a.value());
+}
+}
More information about the cfe-commits
mailing list