[clang] e0e1748 - [analyzer] Fix a crash in NoStateChangeVisitor with body-farmed stack frames.

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2022-02-17T10:13:34-08:00
New Revision: e0e174845b08b36a3888f47f6b06e496f75cf847

URL: https://github.com/llvm/llvm-project/commit/e0e174845b08b36a3888f47f6b06e496f75cf847
DIFF: https://github.com/llvm/llvm-project/commit/e0e174845b08b36a3888f47f6b06e496f75cf847.diff

LOG: [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

Added: 
    clang/test/Analysis/malloc-bodyfarms.c
    clang/test/Analysis/malloc-bodyfarms.cpp

Modified: 
    clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 57080a84451ad..63194d69d6363 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -797,8 +797,16 @@ class NoOwnershipChangeVisitor final : public NoStateChangeFuncVisitor {
   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 0000000000000..c613a6f52dbc9
--- /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 0000000000000..f09b2fef9b5a7
--- /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'}}