[clang] aa4f81e - [analyzer] Fix false positive for stack-addr leak on simple param ptr (#107003)

[via cfe-commits]

Author: Arseniy Zaostrovnykh
Date: 2024-09-03T14:06:57+02:00
New Revision: aa4f81efb99c93da3283ff3178be8db51cd655e2

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

LOG: [analyzer] Fix false positive for stack-addr leak on simple param ptr (#107003)

Assigning to a pointer parameter does not leak the stack address because
it stays within the function and is not shared with the caller.

Previous implementation reported any association of a pointer parameter
with a local address, which is too broad.

This fix enforces that the pointer to a stack variable is related by at
least one level of indirection.

CPP-5642

Fixes #106834

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
    clang/test/Analysis/stack-addr-ps.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index ec577c36188e6c..5394c2257514dc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -420,6 +420,8 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS,
         return true;
       }
       if (isa<StackArgumentsSpaceRegion>(ReferrerMemSpace) &&
+          // Not a simple ptr (int*) but something deeper, e.g. int**
+          isa<SymbolicRegion>(Referrer->getBaseRegion()) &&
           ReferrerStackSpace->getStackFrame() == PoppedFrame && TopFrame) {
         // Output parameter of a top-level function
         V.emplace_back(Referrer, Referred);

diff  --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp
index 88bf6512165201..35f38fbbfbefdc 100644
--- a/clang/test/Analysis/stack-addr-ps.cpp
+++ b/clang/test/Analysis/stack-addr-ps.cpp
@@ -137,7 +137,7 @@ namespace rdar13296133 {
     ConvertsToPointer obj;
     return obj; // no-warning
   }
-}
+} // namespace rdar13296133
 
 void write_stack_address_to(char **q) {
   char local;
@@ -791,3 +791,58 @@ void global_ptr_to_ptr() {
   *global_pp = nullptr;
 }
 } // namespace leaking_via_indirect_global_invalidated
+
+namespace not_leaking_via_simple_ptr {
+void simple_ptr(const char *p) {
+  char tmp;
+  p = &tmp; // no-warning
+}
+
+void ref_ptr(const char *&p) {
+  char tmp;
+  p = &tmp; // expected-warning{{variable 'tmp' is still referred to by the caller variable 'p'}}
+}
+
+struct S {
+  const char *p;
+};
+
+void struct_ptr(S s) {
+  char tmp;
+  s.p = &tmp; // no-warning
+}
+
+void array(const char arr[2]) {
+  char tmp;
+  arr = &tmp; // no-warning
+}
+
+extern void copy(char *output, const char *input, unsigned size);
+extern bool foo(const char *input);
+extern void bar(char *output, unsigned count);
+extern bool baz(char *output, const char *input);
+
+void repo(const char *input, char *output) {
+  char temp[64];
+  copy(temp, input, sizeof(temp));
+
+  char result[64];
+  input = temp;
+  if (foo(temp)) {
+    bar(result, sizeof(result));
+    input = result;
+  }
+  if (!baz(output, input)) {
+    copy(output, input, sizeof(result));
+  }
+}
+} // namespace not_leaking_via_simple_ptr
+
+namespace early_reclaim_dead_limitation {
+void foo();
+void top(char **p) {
+  char local;
+  *p = &local;
+  foo(); // no-warning FIXME: p binding is reclaimed before the function end
+}
+} // namespace early_reclaim_dead_limitation