This revision was automatically updated to reflect the committed changes. Closed by commit rGe0e174845b08: [analyzer] Fix a crash in NoStateChangeVisitor with body-farmed stack frames. (authored by dergachev.a). Herald added a project: clang.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119509/new/ https://reviews.llvm.org/D119509 Files: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/test/Analysis/malloc-bodyfarms.c clang/test/Analysis/malloc-bodyfarms.cpp Index: clang/test/Analysis/malloc-bodyfarms.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/malloc-bodyfarms.cpp @@ -0,0 +1,28 @@ +// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker core,unix -verify %s + +namespace std { +typedef struct once_flag_s { + int _M_once = 0; +} once_flag; + +template <class Callable, class... Args> +void call_once(once_flag &o, Callable&& func, Args&&... args); +} // namespace std + +typedef __typeof(sizeof(int)) size_t; +void *malloc(size_t); + +void callee() {} + +void test_no_state_change_in_body_farm() { + std::once_flag flag; + call_once(flag, callee); // no-crash + malloc(1); +} // expected-warning{{Potential memory leak}} + +void test_no_state_change_in_body_farm_2() { + void *p = malloc(1); + std::once_flag flag; + call_once(flag, callee); // no-crash + p = 0; +} // expected-warning{{Potential leak of memory pointed to by 'p'}} Index: clang/test/Analysis/malloc-bodyfarms.c =================================================================== --- /dev/null +++ clang/test/Analysis/malloc-bodyfarms.c @@ -0,0 +1,19 @@ +// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker core,unix -verify %s + +typedef __typeof(sizeof(int)) size_t; +void *calloc(size_t, size_t); + +typedef struct dispatch_queue_s *dispatch_queue_t; +typedef void (^dispatch_block_t)(void); +void dispatch_sync(dispatch_queue_t, dispatch_block_t); + +void test_no_state_change_in_body_farm(dispatch_queue_t queue) { + dispatch_sync(queue, ^{}); // no-crash + calloc(1, 1); +} // expected-warning{{Potential memory leak}} + +void test_no_state_change_in_body_farm_2(dispatch_queue_t queue) { + void *p = calloc(1, 1); + dispatch_sync(queue, ^{}); // no-crash + p = 0; +} // expected-warning{{Potential leak of memory pointed to by 'p'}} Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -797,8 +797,16 @@ bool doesFnIntendToHandleOwnership(const Decl *Callee, ASTContext &ACtx) { using namespace clang::ast_matchers; const FunctionDecl *FD = dyn_cast<FunctionDecl>(Callee); - if (!FD) + + // Given that the stack frame was entered, the body should always be + // theoretically obtainable. In case of body farms, the synthesized body + // is not attached to declaration, thus triggering the '!FD->hasBody()' + // branch. That said, would a synthesized body ever intend to handle + // ownership? As of today they don't. And if they did, how would we + // put notes inside it, given that it doesn't match any source locations? + if (!FD || !FD->hasBody()) return false; + // TODO: Operator delete is hardly the only deallocator -- Can we reuse // isFreeingCall() or something thats already here? auto Deallocations = match(
Index: clang/test/Analysis/malloc-bodyfarms.cpp =================================================================== --- /dev/null +++ clang/test/Analysis/malloc-bodyfarms.cpp @@ -0,0 +1,28 @@ +// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker core,unix -verify %s + +namespace std { +typedef struct once_flag_s { + int _M_once = 0; +} once_flag; + +template <class Callable, class... Args> +void call_once(once_flag &o, Callable&& func, Args&&... args); +} // namespace std + +typedef __typeof(sizeof(int)) size_t; +void *malloc(size_t); + +void callee() {} + +void test_no_state_change_in_body_farm() { + std::once_flag flag; + call_once(flag, callee); // no-crash + malloc(1); +} // expected-warning{{Potential memory leak}} + +void test_no_state_change_in_body_farm_2() { + void *p = malloc(1); + std::once_flag flag; + call_once(flag, callee); // no-crash + p = 0; +} // expected-warning{{Potential leak of memory pointed to by 'p'}} Index: clang/test/Analysis/malloc-bodyfarms.c =================================================================== --- /dev/null +++ clang/test/Analysis/malloc-bodyfarms.c @@ -0,0 +1,19 @@ +// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker core,unix -verify %s + +typedef __typeof(sizeof(int)) size_t; +void *calloc(size_t, size_t); + +typedef struct dispatch_queue_s *dispatch_queue_t; +typedef void (^dispatch_block_t)(void); +void dispatch_sync(dispatch_queue_t, dispatch_block_t); + +void test_no_state_change_in_body_farm(dispatch_queue_t queue) { + dispatch_sync(queue, ^{}); // no-crash + calloc(1, 1); +} // expected-warning{{Potential memory leak}} + +void test_no_state_change_in_body_farm_2(dispatch_queue_t queue) { + void *p = calloc(1, 1); + dispatch_sync(queue, ^{}); // no-crash + p = 0; +} // expected-warning{{Potential leak of memory pointed to by 'p'}} Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -797,8 +797,16 @@ bool doesFnIntendToHandleOwnership(const Decl *Callee, ASTContext &ACtx) { using namespace clang::ast_matchers; const FunctionDecl *FD = dyn_cast<FunctionDecl>(Callee); - if (!FD) + + // Given that the stack frame was entered, the body should always be + // theoretically obtainable. In case of body farms, the synthesized body + // is not attached to declaration, thus triggering the '!FD->hasBody()' + // branch. That said, would a synthesized body ever intend to handle + // ownership? As of today they don't. And if they did, how would we + // put notes inside it, given that it doesn't match any source locations? + if (!FD || !FD->hasBody()) return false; + // TODO: Operator delete is hardly the only deallocator -- Can we reuse // isFreeingCall() or something thats already here? auto Deallocations = match(
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits