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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: Szelethus, t-rasmud, martong, steakhal, ASDenysPetrov, balazske, gamesh411.
Herald added subscribers: manas, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, kristof.beyls, xazax.hun.
NoQ requested review of this revision.

`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. This manifests in hard crashes every time the visitor tries to scan the body-farmed stack frame.

Note that it's technically possible to obtain and scan the body. It's useless to do so though, given that you can't put notes into it, given that it doesn't map into any user source code.

The C test case is libdispatch/blocks <https://www.youtube.com/watch?v=hWTFG3J1CP8>-specific but the C++ test case is something that can happen to everybody, even though the premise is roughly the same.


Repository:
  rC Clang

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,21 @@
+// 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}}
Index: clang/test/Analysis/malloc-bodyfarms.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/malloc-bodyfarms.c
@@ -0,0 +1,13 @@
+// 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}}
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.407748.patch
Type: text/x-patch
Size: 2533 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220211/3fd79462/attachment.bin>