Skip to content

Commit 3e8146f

Browse files
committed
[analyzer] Fix a crash in NoStateChangeVisitor with body-farmed stack frames.
LocationContext::getDecl() isn't useful for obtaining the "farmed" body because the (synthetic) body statement isn't actually attached to the (natural-grown) declaration in the AST. Differential Revision: https://reviews.llvm.org/D119509 (cherry picked from commit e0e1748)
1 parent a07342b commit 3e8146f

File tree

3 files changed

+56
-1
lines changed

3 files changed

+56
-1
lines changed

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -797,8 +797,16 @@ class NoOwnershipChangeVisitor final : public NoStateChangeFuncVisitor {
797797
bool doesFnIntendToHandleOwnership(const Decl *Callee, ASTContext &ACtx) {
798798
using namespace clang::ast_matchers;
799799
const FunctionDecl *FD = dyn_cast<FunctionDecl>(Callee);
800-
if (!FD)
800+
801+
// Given that the stack frame was entered, the body should always be
802+
// theoretically obtainable. In case of body farms, the synthesized body
803+
// is not attached to declaration, thus triggering the '!FD->hasBody()'
804+
// branch. That said, would a synthesized body ever intend to handle
805+
// ownership? As of today they don't. And if they did, how would we
806+
// put notes inside it, given that it doesn't match any source locations?
807+
if (!FD || !FD->hasBody())
801808
return false;
809+
802810
// TODO: Operator delete is hardly the only deallocator -- Can we reuse
803811
// isFreeingCall() or something thats already here?
804812
auto Deallocations = match(
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker core,unix -verify %s
2+
3+
typedef __typeof(sizeof(int)) size_t;
4+
void *calloc(size_t, size_t);
5+
6+
typedef struct dispatch_queue_s *dispatch_queue_t;
7+
typedef void (^dispatch_block_t)(void);
8+
void dispatch_sync(dispatch_queue_t, dispatch_block_t);
9+
10+
void test_no_state_change_in_body_farm(dispatch_queue_t queue) {
11+
dispatch_sync(queue, ^{}); // no-crash
12+
calloc(1, 1);
13+
} // expected-warning{{Potential memory leak}}
14+
15+
void test_no_state_change_in_body_farm_2(dispatch_queue_t queue) {
16+
void *p = calloc(1, 1);
17+
dispatch_sync(queue, ^{}); // no-crash
18+
p = 0;
19+
} // expected-warning{{Potential leak of memory pointed to by 'p'}}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker core,unix -verify %s
2+
3+
namespace std {
4+
typedef struct once_flag_s {
5+
int _M_once = 0;
6+
} once_flag;
7+
8+
template <class Callable, class... Args>
9+
void call_once(once_flag &o, Callable&& func, Args&&... args);
10+
} // namespace std
11+
12+
typedef __typeof(sizeof(int)) size_t;
13+
void *malloc(size_t);
14+
15+
void callee() {}
16+
17+
void test_no_state_change_in_body_farm() {
18+
std::once_flag flag;
19+
call_once(flag, callee); // no-crash
20+
malloc(1);
21+
} // expected-warning{{Potential memory leak}}
22+
23+
void test_no_state_change_in_body_farm_2() {
24+
void *p = malloc(1);
25+
std::once_flag flag;
26+
call_once(flag, callee); // no-crash
27+
p = 0;
28+
} // expected-warning{{Potential leak of memory pointed to by 'p'}}

0 commit comments

Comments
 (0)