[clang] a49cf6c - [analyzer] Fix "sprintf" parameter modeling in CStringChecker

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2023-12-28T16:06:21+01:00
New Revision: a49cf6c14ad498244fee6026da59cfdcdad6b80c

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

LOG: [analyzer] Fix "sprintf" parameter modeling in CStringChecker

`CE->getCalleeDecl()` returns `VarDecl` if the callee is actually a
function pointer variable. Consequently, calling `getAsFunction()` will
return null.

To workaround the case, we should use the `CallEvent::parameters()`,
which will internally recover the function being called and do the right
thing.

Fixes #74269
Depends on "[analyzer][NFC] Prefer CallEvent over CallExpr in APIs"

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
    clang/test/Analysis/string.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d3f352c6aabe79..e21ec78a1e8a77 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1115,6 +1115,9 @@ Crash and bug fixes
   `#59493 <https://github.com/llvm/llvm-project/issues/59493>`_,
   `#54533 <https://github.com/llvm/llvm-project/issues/54533>`_)
 
+- Fixed an ``alpha.unix.cstring`` crash on variadic functions.
+  (`#74269 <https://github.com/llvm/llvm-project/issues/74269>`_)
+
 - Fix false positive in mutation check when using pointer to member function.
   (`#66204 <https://github.com/llvm/llvm-project/issues/66204>`_)
 

diff  --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index f5dbf9d82beeee..b7b64c3da4f6c8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2487,8 +2487,7 @@ void CStringChecker::evalSprintfCommon(CheckerContext &C, const CallEvent &Call,
   const auto *CE = cast<CallExpr>(Call.getOriginExpr());
   DestinationArgExpr Dest = {{Call.getArgExpr(0), 0}};
 
-  // FIXME: We should use `Call.parameters().size()` here.
-  const auto NumParams = CE->getCalleeDecl()->getAsFunction()->getNumParams();
+  const auto NumParams = Call.parameters().size();
   assert(CE->getNumArgs() >= NumParams);
 
   const auto AllArguments =

diff  --git a/clang/test/Analysis/string.cpp b/clang/test/Analysis/string.cpp
index f86416da6ee237..1be6c21466cc03 100644
--- a/clang/test/Analysis/string.cpp
+++ b/clang/test/Analysis/string.cpp
@@ -1,6 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s
-
-// expected-no-diagnostics
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,debug.ExprInspection -verify %s
 
 // Test functions that are called "memcpy" but aren't the memcpy
 // we're looking for. Unfortunately, this test cannot be put into
@@ -9,6 +7,11 @@
 typedef __typeof(sizeof(int)) size_t;
 void *memcpy(void *, const void *, size_t);
 
+int sprintf(char *str, const char *format, ...);
+int snprintf(char *str, size_t size, const char *format, ...);
+
+void clang_analyzer_warnIfReached();
+
 struct S {
   static S s1, s2;
 
@@ -26,3 +29,19 @@ void *memcpy(void *, const S &, size_t);
 void test_out_of_class_weird_memcpy() {
   memcpy(&S::s1, S::s2, 1); // no-crash
 }
+
+template<typename... Args>
+void log(const char* fmt, const Args&... args) {
+  char buf[100] = {};
+  auto f = snprintf;
+  auto g = sprintf;
+  int n = 0;
+  n += f(buf, 99, fmt, args...); // no-crash: The CalleeDecl is a VarDecl, but it's okay.
+  n += g(buf, fmt, args...); // no-crash: Same.
+  (void)n;
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+}
+
+void test_gh_74269_no_crash() {
+  log("%d", 1);
+}