llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Haojian Wu (hokein)
<details>
<summary>Changes</summary>
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](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/CheckExprLifetime.cpp#L341-L343)
in the code.
This bailout was introduced in [this
commit](https://github.com/llvm/llvm-project/commit/bcd0798c47ca865f95226859893016a17402441e)
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/104556.diff
3 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+2)
- (modified) clang/lib/Sema/CheckExprLifetime.cpp (+10-27)
- (modified) clang/test/Sema/warn-lifetime-analysis-nocfg.cpp (+12)
``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ffdd063ec99037..c9864abc593e5e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -214,6 +214,8 @@ Improvements to Clang's diagnostics
- Clang now diagnoses the use of ``main`` in an ``extern`` context as invalid
according to [basic.start.main] p3. Fixes #GH101512.
+- Clang now diagnoses dangling cases where a gsl-pointer is constructed from a
gsl-owner object inside a container (#GH100384).
+
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp
b/clang/lib/Sema/CheckExprLifetime.cpp
index 7389046eaddde1..b543c5e67f9550 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -191,7 +191,6 @@ struct IndirectLocalPathEntry {
LifetimeBoundCall,
TemporaryCopy,
LambdaCaptureInit,
- GslReferenceInit,
GslPointerInit,
GslPointerAssignment,
} Kind;
@@ -328,25 +327,13 @@ static bool shouldTrackFirstArgument(const FunctionDecl
*FD) {
static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
LocalVisitor Visit) {
- auto VisitPointerArg = [&](const Decl *D, Expr *Arg, bool Value) {
+ auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
// 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});
+
+ Path.push_back({IndirectLocalPathEntry::GslPointerInit, Arg, D});
if (Arg->isGLValue())
visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding,
Visit,
@@ -360,29 +347,28 @@ static void handleGslAnnotatedTypes(IndirectLocalPath
&Path, Expr *Call,
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());
+ VisitPointerArg(MD, MCE->getImplicitObjectArgument());
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());
+ VisitPointerArg(Callee, OCE->getArg(0));
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());
+ VisitPointerArg(Callee, CE->getArg(0));
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);
+ if (CCE->getNumArgs() > 0 && RD->hasAttr<PointerAttr>() &&
+ isRecordWithAttr<OwnerAttr>(
+ CCE->getArg(0)->IgnoreImpCasts()->getType()))
+ VisitPointerArg(Ctor->getParamDecl(0), CCE->getArgs()[0]);
}
}
@@ -944,7 +930,6 @@ static SourceRange nextPathEntryRange(const
IndirectLocalPath &Path, unsigned I,
case IndirectLocalPathEntry::LValToRVal:
case IndirectLocalPathEntry::LifetimeBoundCall:
case IndirectLocalPathEntry::TemporaryCopy:
- case IndirectLocalPathEntry::GslReferenceInit:
case IndirectLocalPathEntry::GslPointerInit:
case IndirectLocalPathEntry::GslPointerAssignment:
// These exist primarily to mark the path as not permitting or
@@ -975,7 +960,6 @@ static bool pathOnlyHandlesGslPointer(IndirectLocalPath
&Path) {
case IndirectLocalPathEntry::LifetimeBoundCall:
continue;
case IndirectLocalPathEntry::GslPointerInit:
- case IndirectLocalPathEntry::GslReferenceInit:
case IndirectLocalPathEntry::GslPointerAssignment:
return true;
default:
@@ -1242,7 +1226,6 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
case IndirectLocalPathEntry::LifetimeBoundCall:
case IndirectLocalPathEntry::TemporaryCopy:
case IndirectLocalPathEntry::GslPointerInit:
- case IndirectLocalPathEntry::GslReferenceInit:
case IndirectLocalPathEntry::GslPointerAssignment:
// FIXME: Consider adding a note for these.
break;
diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index 09dfb2b5d96a89..c0fb2ed042cc7b 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -475,6 +475,18 @@ std::vector<int>::iterator
noFalsePositiveWithVectorOfPointers() {
return iters.at(0);
}
+// GH100384
+std::string_view ContainerWithAnnotatedElements() {
+ std::string_view c1 = std::vector<std::string>().at(0); // expected-warning
{{object backing the pointer will be destroyed at the end of the
full-expression}}
+ c1 = std::vector<std::string>().at(0); // expected-warning {{object backing
the pointer}}
+
+ // no warning on constructing from gsl-pointer
+ std::string_view c2 = std::vector<std::string_view>().at(0);
+
+ std::vector<std::string> local;
+ return local.at(0); // expected-warning {{address of stack memory associated
with local variable 'local' returned}}
+}
+
void testForBug49342()
{
auto it = std::iter<char>{} - 2; // Used to be false positive.
``````````
</details>
https://github.com/llvm/llvm-project/pull/104556
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
