[PATCH] D119509: [analyzer] Fix a crash in NoStateChangeVisitor with body-farmed stack frames.

[Artem Dergachev via Phabricator via cfe-commits]

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(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D119509.409708.patch
Type: text/x-patch
Size: 2966 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220217/a33fc17e/attachment.bin>