Skip to content

Commit ff2f955

Browse files
committed
[clang] Detect dangling assignment for "Container<Pointer>" case.
1 parent 36235ce commit ff2f955

File tree

2 files changed

+34
-5
lines changed

2 files changed

+34
-5
lines changed

clang/lib/Sema/CheckExprLifetime.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,7 +1096,8 @@ static bool shouldRunGSLAssignmentAnalysis(const Sema &SemaRef,
10961096
diag::warn_dangling_lifetime_pointer_assignment, SourceLocation());
10971097
return (EnableGSLAssignmentWarnings &&
10981098
(isRecordWithAttr<PointerAttr>(Entity.LHS->getType()) ||
1099-
isAssignmentOperatorLifetimeBound(Entity.AssignmentOperator)));
1099+
isAssignmentOperatorLifetimeBound(Entity.AssignmentOperator) ||
1100+
isContainerOfPointer(Entity.LHS->getType()->getAsRecordDecl())));
11001101
}
11011102

11021103
static void checkExprLifetimeImpl(Sema &SemaRef,
@@ -1391,8 +1392,21 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
13911392
};
13921393

13931394
llvm::SmallVector<IndirectLocalPathEntry, 8> Path;
1394-
if (LK == LK_Assignment && shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity))
1395+
if (LK == LK_Assignment &&
1396+
shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity)) {
1397+
if (isContainerOfPointer(AEntity->LHS->getType()->getAsRecordDecl())) {
1398+
// Bail out for user-defined assignment operators, as their contract is
1399+
// unknown.
1400+
if (!AEntity->AssignmentOperator->isDefaulted())
1401+
return;
1402+
// Skip the top MaterializeTemoraryExpr because it is temporary object of
1403+
// the containerOfPointer itself.
1404+
if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Init))
1405+
Init = MTE->getSubExpr();
1406+
}
1407+
13951408
Path.push_back({IndirectLocalPathEntry::GslPointerAssignment, Init});
1409+
}
13961410

13971411
if (Init->isGLValue())
13981412
visitLocalsRetainedByReferenceBinding(Path, Init, RK_ReferenceBinding,

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -634,18 +634,27 @@ void test() {
634634
std::optional<std::string_view> o3 = std::optional<std::string>(s); // expected-warning {{object backing the pointer}}
635635
std::optional<std::string_view> o4 = std::optional<std::string_view>(s);
636636

637-
// FIXME: should work for assignment cases
638-
v1 = {std::string()};
639-
o1 = std::string();
637+
v1 = {std::string()}; // expected-warning {{object backing the pointer}}
638+
o1 = std::string(); // expected-warning {{object backing the pointer}}
640639

641640
// no warning on copying pointers.
642641
std::vector<std::string_view> n1 = {std::string_view()};
642+
n1 = {std::string_view()};
643643
std::optional<std::string_view> n2 = {std::string_view()};
644+
n2 = {std::string_view()};
644645
std::optional<std::string_view> n3 = std::string_view();
646+
n3 = std::string_view();
645647
std::optional<std::string_view> n4 = std::make_optional(std::string_view());
648+
n4 = std::make_optional(std::string_view());
646649
const char* b = "";
647650
std::optional<std::string_view> n5 = std::make_optional(b);
651+
n5 = std::make_optional(b);
648652
std::optional<std::string_view> n6 = std::make_optional("test");
653+
n6 = std::make_optional("test");
654+
// Deeper nested containers are not supported; only first-level nesting is
655+
// supported.
656+
std::vector<std::vector<std::string_view>> n7 = {{std::string()}};
657+
n7 = {{std::string()}};
649658
}
650659

651660
std::vector<std::string_view> test2(int i) {
@@ -683,7 +692,9 @@ std::optional<std::string_view> test3(int i) {
683692
return s; // expected-warning {{address of stack memory associated}}
684693
return sv; // fine
685694
Container2<std::string_view> c1 = Container1<Foo>(); // no diagnostic as Foo is not an Owner.
695+
c1 = Container1<Foo>();
686696
Container2<std::string_view> c2 = Container1<FooOwner>(); // expected-warning {{object backing the pointer will be destroyed}}
697+
c2 = Container1<FooOwner>(); // expected-warning {{object backing the pointer}}
687698
return GetFoo(); // fine, we don't know Foo is owner or not, be conservative.
688699
return GetFooOwner(); // expected-warning {{returning address of local temporary object}}
689700
}
@@ -713,10 +724,12 @@ struct [[gsl::Pointer]] Span {
713724
// Pointer from Owner<Pointer>
714725
std::string_view test5() {
715726
std::string_view a = StatusOr<std::string_view>().valueLB(); // expected-warning {{object backing the pointer will be dest}}
727+
a = StatusOr<std::string_view>().valueLB(); // expected-warning {{object backing the pointer}}
716728
return StatusOr<std::string_view>().valueLB(); // expected-warning {{returning address of local temporary}}
717729

718730
// No dangling diagnostics on non-lifetimebound methods.
719731
std::string_view b = StatusOr<std::string_view>().valueNoLB();
732+
b = StatusOr<std::string_view>().valueNoLB();
720733
return StatusOr<std::string_view>().valueNoLB();
721734
}
722735

@@ -773,8 +786,10 @@ const int& test12(Span<int> a) {
773786
void test13() {
774787
// FIXME: RHS is Owner<Pointer>, we skip this case to avoid false positives.
775788
std::optional<Span<int*>> abc = std::vector<int*>{};
789+
abc = std::vector<int*> {};
776790

777791
std::optional<Span<int>> t = std::vector<int> {}; // expected-warning {{object backing the pointer will be destroyed}}
792+
t = std::vector<int> {}; // expected-warning {{object backing the pointer}}
778793
}
779794

780795
} // namespace GH100526

0 commit comments

Comments
 (0)