Skip to content

Commit a49cf6c

Browse files
committed
[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"
1 parent d1856b2 commit a49cf6c

File tree

3 files changed

+26
-5
lines changed

3 files changed

+26
-5
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,6 +1115,9 @@ Crash and bug fixes
11151115
`#59493 <https://github.com/llvm/llvm-project/issues/59493>`_,
11161116
`#54533 <https://github.com/llvm/llvm-project/issues/54533>`_)
11171117

1118+
- Fixed an ``alpha.unix.cstring`` crash on variadic functions.
1119+
(`#74269 <https://github.com/llvm/llvm-project/issues/74269>`_)
1120+
11181121
- Fix false positive in mutation check when using pointer to member function.
11191122
(`#66204 <https://github.com/llvm/llvm-project/issues/66204>`_)
11201123

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2487,8 +2487,7 @@ void CStringChecker::evalSprintfCommon(CheckerContext &C, const CallEvent &Call,
24872487
const auto *CE = cast<CallExpr>(Call.getOriginExpr());
24882488
DestinationArgExpr Dest = {{Call.getArgExpr(0), 0}};
24892489

2490-
// FIXME: We should use `Call.parameters().size()` here.
2491-
const auto NumParams = CE->getCalleeDecl()->getAsFunction()->getNumParams();
2490+
const auto NumParams = Call.parameters().size();
24922491
assert(CE->getNumArgs() >= NumParams);
24932492

24942493
const auto AllArguments =

clang/test/Analysis/string.cpp

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
1-
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s
2-
3-
// expected-no-diagnostics
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,debug.ExprInspection -verify %s
42

53
// Test functions that are called "memcpy" but aren't the memcpy
64
// we're looking for. Unfortunately, this test cannot be put into
@@ -9,6 +7,11 @@
97
typedef __typeof(sizeof(int)) size_t;
108
void *memcpy(void *, const void *, size_t);
119

10+
int sprintf(char *str, const char *format, ...);
11+
int snprintf(char *str, size_t size, const char *format, ...);
12+
13+
void clang_analyzer_warnIfReached();
14+
1215
struct S {
1316
static S s1, s2;
1417

@@ -26,3 +29,19 @@ void *memcpy(void *, const S &, size_t);
2629
void test_out_of_class_weird_memcpy() {
2730
memcpy(&S::s1, S::s2, 1); // no-crash
2831
}
32+
33+
template<typename... Args>
34+
void log(const char* fmt, const Args&... args) {
35+
char buf[100] = {};
36+
auto f = snprintf;
37+
auto g = sprintf;
38+
int n = 0;
39+
n += f(buf, 99, fmt, args...); // no-crash: The CalleeDecl is a VarDecl, but it's okay.
40+
n += g(buf, fmt, args...); // no-crash: Same.
41+
(void)n;
42+
clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
43+
}
44+
45+
void test_gh_74269_no_crash() {
46+
log("%d", 1);
47+
}

0 commit comments

Comments
 (0)