Skip to content

Commit bc0d0cf

Browse files
dougsonosDoug Wyatt
andauthored
[Clang] FunctionEffect analysis was missing a CXXBindTemporaryExpr's implicit call to a destructor. (#166110)
This example is reduced from a discovery: resetting a shared pointer from a nonblocking function is not diagnosed. ``` void nb23() { struct X { int *ptr = nullptr; X() {} ~X() { delete ptr; } }; auto inner = []() [[clang::nonblocking]] { X(); }; } ``` `shared_ptr<T>::reset()` creates a temporary `shared_ptr` and swaps it with its current state. The temporary `shared_ptr` constructor is nonblocking but its destructor potentially deallocates memory and is unsafe. Analysis was ignoring the implicit call in the AST to destroy the temporary. --------- Co-authored-by: Doug Wyatt <[email protected]>
1 parent 5e8a0d6 commit bc0d0cf

File tree

3 files changed

+36
-0
lines changed

3 files changed

+36
-0
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,7 @@ Bug Fixes to Attribute Support
460460
- Fix a crash when the function name is empty in the `swift_name` attribute. (#GH157075)
461461
- Fixes crashes or missing diagnostics with the `device_kernel` attribute. (#GH161905)
462462
- Fix handling of parameter indexes when an attribute is applied to a C++23 explicit object member function.
463+
- Fixed several false positives and false negatives in function effect (`nonblocking`) analysis. (#GH166078) (#GH166101) (#GH166110)
463464

464465
Bug Fixes to C++ Support
465466
^^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/Sema/SemaFunctionEffects.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1279,7 +1279,15 @@ class Analyzer {
12791279
const CXXConstructorDecl *Ctor = Construct->getConstructor();
12801280
CallableInfo CI(*Ctor);
12811281
followCall(CI, Construct->getLocation());
1282+
return true;
1283+
}
12821284

1285+
bool VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *BTE) override {
1286+
const CXXDestructorDecl *Dtor = BTE->getTemporary()->getDestructor();
1287+
if (Dtor != nullptr) {
1288+
CallableInfo CI(*Dtor);
1289+
followCall(CI, BTE->getBeginLoc());
1290+
}
12831291
return true;
12841292
}
12851293

clang/test/Sema/attr-nonblocking-constraints.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,33 @@ struct Unsafe {
373373
Unsafe(float y) [[clang::nonblocking]] : Unsafe(int(y)) {} // expected-warning {{constructor with 'nonblocking' attribute must not call non-'nonblocking' constructor 'Unsafe::Unsafe'}}
374374
};
375375

376+
// Exercise cases of a temporary with a safe constructor and unsafe destructor.
377+
void nb23()
378+
{
379+
struct X {
380+
int *ptr = nullptr;
381+
X() {}
382+
~X() { delete ptr; } // expected-note 2 {{destructor cannot be inferred 'nonblocking' because it allocates or deallocates memory}}
383+
};
384+
385+
auto inner = []() [[clang::nonblocking]] {
386+
X(); // expected-warning {{lambda with 'nonblocking' attribute must not call non-'nonblocking' destructor 'nb23()::X::~X'}}
387+
};
388+
389+
auto inner2 = [](X x) [[clang::nonblocking]] { // expected-warning {{lambda with 'nonblocking' attribute must not call non-'nonblocking' destructor 'nb23()::X::~X'}}
390+
};
391+
392+
}
393+
394+
struct S2 { ~S2(); }; // expected-note 2 {{declaration cannot be inferred 'nonblocking' because it has no definition in this translation unit}}
395+
void nb24() {
396+
S2 s;
397+
[&]() [[clang::nonblocking]] {
398+
[s]{ auto x = &s; }(); // expected-warning {{lambda with 'nonblocking' attribute must not call non-'nonblocking' destructor}} expected-note {{destructor cannot be inferred 'nonblocking' because it calls non-'nonblocking' destructor 'S2::~S2'}}
399+
[=]{ auto x = &s; }(); // expected-warning {{lambda with 'nonblocking' attribute must not call non-'nonblocking' destructor}} expected-note {{destructor cannot be inferred 'nonblocking' because it calls non-'nonblocking' destructor 'S2::~S2'}}
400+
}();
401+
}
402+
376403
struct DerivedFromUnsafe : public Unsafe {
377404
DerivedFromUnsafe() [[clang::nonblocking]] {} // expected-warning {{constructor with 'nonblocking' attribute must not call non-'nonblocking' constructor 'Unsafe::Unsafe'}}
378405
DerivedFromUnsafe(int x) [[clang::nonblocking]] : Unsafe(x) {} // expected-warning {{constructor with 'nonblocking' attribute must not call non-'nonblocking' constructor 'Unsafe::Unsafe'}}

0 commit comments

Comments
 (0)