Skip to content

Commit d4778d0

Browse files
hokeinhokein
committed
[clang] Merge the lifetimebound and GSL code paths for lifetime
analysis.
1 parent 0534c4f commit d4778d0

File tree

3 files changed

+72
-72
lines changed

3 files changed

+72
-72
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,8 @@ Improvements to Clang's diagnostics
239239

240240
- Clang now diagnoses when the result of a [[nodiscard]] function is discarded after being cast in C. Fixes #GH104391.
241241

242+
- Don't emit duplicated dangling diagnostics. (#GH93386).
243+
242244
Improvements to Clang's time-trace
243245
----------------------------------
244246

‎clang/lib/Sema/CheckExprLifetime.cpp

Lines changed: 57 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -326,66 +326,6 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
326326
return false;
327327
}
328328

329-
static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
330-
LocalVisitor Visit) {
331-
auto VisitPointerArg = [&](const Decl *D, Expr *Arg, bool Value) {
332-
// We are not interested in the temporary base objects of gsl Pointers:
333-
// Temp().ptr; // Here ptr might not dangle.
334-
if (isa<MemberExpr>(Arg->IgnoreImpCasts()))
335-
return;
336-
// Once we initialized a value with a reference, it can no longer dangle.
337-
if (!Value) {
338-
for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) {
339-
if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit)
340-
continue;
341-
if (PE.Kind == IndirectLocalPathEntry::GslPointerInit ||
342-
PE.Kind == IndirectLocalPathEntry::GslPointerAssignment)
343-
return;
344-
break;
345-
}
346-
}
347-
Path.push_back({Value ? IndirectLocalPathEntry::GslPointerInit
348-
: IndirectLocalPathEntry::GslReferenceInit,
349-
Arg, D});
350-
if (Arg->isGLValue())
351-
visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
352-
Visit,
353-
/*EnableLifetimeWarnings=*/true);
354-
else
355-
visitLocalsRetainedByInitializer(Path, Arg, Visit, true,
356-
/*EnableLifetimeWarnings=*/true);
357-
Path.pop_back();
358-
};
359-
360-
if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Call)) {
361-
const auto *MD = cast_or_null<CXXMethodDecl>(MCE->getDirectCallee());
362-
if (MD && shouldTrackImplicitObjectArg(MD))
363-
VisitPointerArg(MD, MCE->getImplicitObjectArgument(),
364-
!MD->getReturnType()->isReferenceType());
365-
return;
366-
} else if (auto *OCE = dyn_cast<CXXOperatorCallExpr>(Call)) {
367-
FunctionDecl *Callee = OCE->getDirectCallee();
368-
if (Callee && Callee->isCXXInstanceMember() &&
369-
shouldTrackImplicitObjectArg(cast<CXXMethodDecl>(Callee)))
370-
VisitPointerArg(Callee, OCE->getArg(0),
371-
!Callee->getReturnType()->isReferenceType());
372-
return;
373-
} else if (auto *CE = dyn_cast<CallExpr>(Call)) {
374-
FunctionDecl *Callee = CE->getDirectCallee();
375-
if (Callee && shouldTrackFirstArgument(Callee))
376-
VisitPointerArg(Callee, CE->getArg(0),
377-
!Callee->getReturnType()->isReferenceType());
378-
return;
379-
}
380-
381-
if (auto *CCE = dyn_cast<CXXConstructExpr>(Call)) {
382-
const auto *Ctor = CCE->getConstructor();
383-
const CXXRecordDecl *RD = Ctor->getParent();
384-
if (CCE->getNumArgs() > 0 && RD->hasAttr<PointerAttr>())
385-
VisitPointerArg(Ctor->getParamDecl(0), CCE->getArgs()[0], true);
386-
}
387-
}
388-
389329
static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
390330
const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
391331
if (!TSI)
@@ -423,8 +363,10 @@ static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
423363
return false;
424364
}
425365

426-
static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
427-
LocalVisitor Visit) {
366+
// Visit lifetimebound or gsl-pointer arguments.
367+
static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
368+
LocalVisitor Visit,
369+
bool EnableLifetimeWarnings) {
428370
const FunctionDecl *Callee;
429371
ArrayRef<Expr *> Args;
430372

@@ -458,6 +400,35 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
458400
/*EnableLifetimeWarnings=*/false);
459401
Path.pop_back();
460402
};
403+
auto VisitGSLPointerArg = [&](const Decl *D, Expr *Arg, bool Value) {
404+
// We are not interested in the temporary base objects of gsl Pointers:
405+
// Temp().ptr; // Here ptr might not dangle.
406+
if (isa<MemberExpr>(Arg->IgnoreImpCasts()))
407+
return;
408+
// Once we initialized a value with a reference, it can no longer dangle.
409+
if (!Value) {
410+
for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) {
411+
if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit)
412+
continue;
413+
if (PE.Kind == IndirectLocalPathEntry::GslPointerInit ||
414+
PE.Kind == IndirectLocalPathEntry::GslPointerAssignment)
415+
return;
416+
break;
417+
}
418+
}
419+
Path.push_back({Value ? IndirectLocalPathEntry::GslPointerInit
420+
: IndirectLocalPathEntry::GslReferenceInit,
421+
Arg, D});
422+
if (Arg->isGLValue())
423+
visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
424+
Visit,
425+
/*EnableLifetimeWarnings=*/true);
426+
else
427+
visitLocalsRetainedByInitializer(Path, Arg, Visit, true,
428+
/*EnableLifetimeWarnings=*/true);
429+
Path.pop_back();
430+
};
431+
461432

462433
bool CheckCoroCall = false;
463434
if (const auto *RD = Callee->getReturnType()->getAsRecordDecl()) {
@@ -478,13 +449,32 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
478449
CheckCoroObjArg = false;
479450
if (implicitObjectParamIsLifetimeBound(Callee) || CheckCoroObjArg)
480451
VisitLifetimeBoundArg(Callee, ObjectArg);
452+
else if (EnableLifetimeWarnings) {
453+
if (auto *CME = dyn_cast<CXXMethodDecl>(Callee);
454+
CME && shouldTrackImplicitObjectArg(CME))
455+
VisitGSLPointerArg(Callee, ObjectArg,
456+
!Callee->getReturnType()->isReferenceType());
457+
}
481458
}
482459

483460
for (unsigned I = 0,
484461
N = std::min<unsigned>(Callee->getNumParams(), Args.size());
485462
I != N; ++I) {
486463
if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>())
487464
VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]);
465+
else if (EnableLifetimeWarnings) {
466+
if (I == 0) {
467+
if (shouldTrackFirstArgument(Callee)) { // gsl
468+
VisitGSLPointerArg(Callee, Args[0],
469+
!Callee->getReturnType()->isReferenceType());
470+
} else {
471+
if (auto *CCE = dyn_cast<CXXConstructExpr>(Call);
472+
CCE && CCE->getConstructor()->getParent()->hasAttr<PointerAttr>())
473+
VisitGSLPointerArg(CCE->getConstructor()->getParamDecl(0), Args[0],
474+
true);
475+
}
476+
}
477+
}
488478
}
489479
}
490480

@@ -557,11 +547,9 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
557547
EnableLifetimeWarnings);
558548
}
559549

560-
if (isa<CallExpr>(Init)) {
561-
if (EnableLifetimeWarnings)
562-
handleGslAnnotatedTypes(Path, Init, Visit);
563-
return visitLifetimeBoundArguments(Path, Init, Visit);
564-
}
550+
if (isa<CallExpr>(Init))
551+
return visitFunctionCallArguments(Path, Init, Visit,
552+
EnableLifetimeWarnings);
565553

566554
switch (Init->getStmtClass()) {
567555
case Stmt::DeclRefExprClass: {
@@ -835,11 +823,8 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
835823
}
836824
}
837825

838-
if (isa<CallExpr>(Init) || isa<CXXConstructExpr>(Init)) {
839-
if (EnableLifetimeWarnings)
840-
handleGslAnnotatedTypes(Path, Init, Visit);
841-
return visitLifetimeBoundArguments(Path, Init, Visit);
842-
}
826+
if (isa<CallExpr>(Init) || isa<CXXConstructExpr>(Init))
827+
return visitFunctionCallArguments(Path, Init, Visit, EnableLifetimeWarnings);
843828

844829
switch (Init->getStmtClass()) {
845830
case Stmt::UnaryOperatorClass: {

‎clang/test/Sema/warn-lifetime-analysis-nocfg.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,3 +479,16 @@ void testForBug49342()
479479
{
480480
auto it = std::iter<char>{} - 2; // Used to be false positive.
481481
}
482+
483+
namespace GH93386 {
484+
// verify no duplicated diagnostics are emitted.
485+
struct [[gsl::Pointer]] S {
486+
S(const std::vector<int>& abc [[clang::lifetimebound]]);
487+
};
488+
489+
S test(std::vector<int> a) {
490+
return S(a); // expected-warning {{address of stack memory associated with}}
491+
}
492+
493+
auto s = S(std::vector<int>()); // expected-warning {{temporary whose address is used as value of local variable}}
494+
}

0 commit comments

Comments
 (0)