From e0e174845b08b36a3888f47f6b06e496f75cf847 Mon Sep 17 00:00:00 2001
From: Artem Dergachev <artem.dergachev@gmail.com>
Date: Wed, 16 Feb 2022 21:09:09 -0800
Subject: [PATCH] [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
---
 .../lib/StaticAnalyzer/Checkers/MallocChecker.cpp  | 10 +++++++-
 clang/test/Analysis/malloc-bodyfarms.c             | 19 +++++++++++++++
 clang/test/Analysis/malloc-bodyfarms.cpp           | 28 ++++++++++++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/Analysis/malloc-bodyfarms.c
 create mode 100644 clang/test/Analysis/malloc-bodyfarms.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 57080a8..63194d6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -797,8 +797,16 @@ protected:
   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(
diff --git a/clang/test/Analysis/malloc-bodyfarms.c b/clang/test/Analysis/malloc-bodyfarms.c
new file mode 100644
index 0000000..c613a6f
--- /dev/null
+++ b/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'}}
diff --git a/clang/test/Analysis/malloc-bodyfarms.cpp b/clang/test/Analysis/malloc-bodyfarms.cpp
new file mode 100644
index 0000000..f09b2fe
--- /dev/null
+++ b/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'}}
-- 
2.7.4