https://github.com/bjosv created https://github.com/llvm/llvm-project/pull/192024
The `security.VAList` checker only recognized `__builtin_va_start` when matching `va_start` calls. In C23, `va_start` was changed to expand to `__builtin_c23_va_start` instead, causing the checker to never see the initialization. This resulted in false positives for every use of `va_arg`, `va_end`, `va_copy`, and functions like `vsnprintf` on any `va_list` initialized with the C23 `va_start`. Add a CallDescription for `__builtin_c23_va_start` and match it alongside the existing `__builtin_va_start`. From c8e55ef65272a8928d47740e41b74c3b01ef1e39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <[email protected]> Date: Tue, 14 Apr 2026 09:55:23 +0200 Subject: [PATCH] [analyzer] Fix security.VAList false positives with C23 va_start MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The security.VAList checker only recognized __builtin_va_start when matching va_start calls. In C23, va_start was changed to expand to __builtin_c23_va_start instead, causing the checker to never see the initialization. This resulted in false positives for every use of va_arg, va_end, va_copy, and functions like vsnprintf on any va_list initialized with the C23 va_start. Add a CallDescription for __builtin_c23_va_start and match it alongside the existing __builtin_va_start. Signed-off-by: Björn Svensson <[email protected]> --- clang/docs/ReleaseNotes.rst | 6 ++ .../StaticAnalyzer/Checkers/VAListChecker.cpp | 5 +- .../system-header-simulator-for-valist-c23.h | 16 +++++ .../test/Analysis/valist-uninitialized-c23.c | 61 +++++++++++++++++++ clang/test/Analysis/valist-unterminated-c23.c | 35 +++++++++++ 5 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 clang/test/Analysis/Inputs/system-header-simulator-for-valist-c23.h create mode 100644 clang/test/Analysis/valist-uninitialized-c23.c create mode 100644 clang/test/Analysis/valist-unterminated-c23.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 3e2d287d1eb1f..019e01fda378f 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -611,6 +611,12 @@ Code Completion Static Analyzer --------------- +Crash and bug fixes +^^^^^^^^^^^^^^^^^^^ + +- Fixed ``security.VAList`` checker producing false positives when analyzing + C23 code where ``va_start`` expands to ``__builtin_c23_va_start``. + .. comment: This is for the Static Analyzer. Using the caret `^^^` underlining for subsections: diff --git a/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp index e81c8bfa94cb1..bc8c82100b6fe 100644 --- a/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp @@ -64,7 +64,7 @@ class VAListChecker : public Checker<check::PreCall, check::PreStmt<VAArgExpr>, int ParamIndex; }; static const SmallVector<VAListAccepter, 15> VAListAccepters; - static const CallDescription VaStart, VaEnd, VaCopy; + static const CallDescription VaStart, VaStartC23, VaEnd, VaCopy; public: void checkPreStmt(const VAArgExpr *VAA, CheckerContext &C) const; @@ -136,13 +136,14 @@ const SmallVector<VAListChecker::VAListAccepter, 15> const CallDescription VAListChecker::VaStart(CDM::CLibrary, {"__builtin_va_start"}, /*Args=*/2, /*Params=*/1), + VAListChecker::VaStartC23(CDM::CLibrary, {"__builtin_c23_va_start"}), VAListChecker::VaCopy(CDM::CLibrary, {"__builtin_va_copy"}, 2), VAListChecker::VaEnd(CDM::CLibrary, {"__builtin_va_end"}, 1); } // end anonymous namespace void VAListChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { - if (VaStart.matches(Call)) + if (VaStart.matches(Call) || VaStartC23.matches(Call)) checkVAListStartCall(Call, C); else if (VaCopy.matches(Call)) checkVAListCopyCall(Call, C); diff --git a/clang/test/Analysis/Inputs/system-header-simulator-for-valist-c23.h b/clang/test/Analysis/Inputs/system-header-simulator-for-valist-c23.h new file mode 100644 index 0000000000000..89bc95abdad5c --- /dev/null +++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist-c23.h @@ -0,0 +1,16 @@ +// Like the compiler, the static analyzer treats some functions differently if +// they come from a system header -- for example, it is assumed that system +// functions do not arbitrarily free() their parameters, and that some bugs +// found in system headers cannot be fixed by the user and should be +// suppressed. + +#pragma clang system_header + +typedef __builtin_va_list va_list; + +#define va_start(...) __builtin_c23_va_start(__VA_ARGS__) +#define va_end(ap) __builtin_va_end(ap) +#define va_arg(ap, type) __builtin_va_arg(ap, type) +#define va_copy(dst, src) __builtin_va_copy(dst, src) + +int vprintf(const char *restrict format, va_list arg); diff --git a/clang/test/Analysis/valist-uninitialized-c23.c b/clang/test/Analysis/valist-uninitialized-c23.c new file mode 100644 index 0000000000000..9ec99ffe3c74b --- /dev/null +++ b/clang/test/Analysis/valist-uninitialized-c23.c @@ -0,0 +1,61 @@ +// RUN: %clang_analyze_cc1 -triple hexagon-unknown-linux -std=c23 -verify %s \ +// RUN: -analyzer-checker=core,security.VAList \ +// RUN: -analyzer-disable-checker=core.CallAndMessage \ +// RUN: -analyzer-output=text +// +// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -std=c23 -verify %s \ +// RUN: -analyzer-checker=core,security.VAList \ +// RUN: -analyzer-disable-checker=core.CallAndMessage \ +// RUN: -analyzer-output=text +// +// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -std=c23 %s \ +// RUN: -analyzer-checker=core,security.VAList + +// Test that the security.VAList checker recognizes __builtin_c23_va_start, +// which is used when compiling in C23 mode. + +#include "Inputs/system-header-simulator-for-valist-c23.h" + +void c23_no_warning(int fst, ...) { + va_list va; + va_start(va, fst); + (void)va_arg(va, int); + va_end(va); +} // no-warning + +void c23_one_arg_no_warning(int fst, ...) { + va_list va; + va_start(va); + (void)va_arg(va, int); + va_end(va); +} // no-warning + +void c23_uninitialized(int fst, ...) { + va_list va; + (void)va_arg(va, int); // expected-warning{{va_arg() is called on an uninitialized va_list}} + // expected-note@-1{{va_arg() is called on an uninitialized va_list}} +} + +void c23_use_after_end(int fst, ...) { + va_list va; + va_start(va, fst); // expected-note{{Initialized va_list}} + va_end(va); // expected-note{{Ended va_list}} + (void)va_arg(va, int); // expected-warning{{va_arg() is called on an already released va_list}} + // expected-note@-1{{va_arg() is called on an already released va_list}} +} + +void c23_copy(int fst, ...) { + va_list va, va2; + va_start(va, fst); + va_copy(va2, va); + va_end(va); + (void)va_arg(va2, int); + va_end(va2); +} // no-warning + +void c23_vprintf(int isstring, ...) { + va_list va; + va_start(va, isstring); + vprintf(isstring ? "%s" : "%d", va); + va_end(va); +} // no-warning diff --git a/clang/test/Analysis/valist-unterminated-c23.c b/clang/test/Analysis/valist-unterminated-c23.c new file mode 100644 index 0000000000000..8f1aae919239f --- /dev/null +++ b/clang/test/Analysis/valist-unterminated-c23.c @@ -0,0 +1,35 @@ +// RUN: %clang_analyze_cc1 -triple hexagon-unknown-linux -std=c23 -verify %s \ +// RUN: -analyzer-checker=core,security.VAList \ +// RUN: -analyzer-output=text +// +// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -std=c23 -verify %s \ +// RUN: -analyzer-checker=core,security.VAList \ +// RUN: -analyzer-output=text + +// Test that the security.VAList checker detects leaks with __builtin_c23_va_start. + +#include "Inputs/system-header-simulator-for-valist-c23.h" + +void c23_leak(int fst, ...) { + va_list va; + va_start(va, fst); // expected-note{{Initialized va_list}} + return; // expected-warning{{Initialized va_list 'va' is leaked}} + // expected-note@-1{{Initialized va_list 'va' is leaked}} +} + +void c23_reinit(int fst, ...) { + va_list va; + va_start(va, fst); // expected-note{{Initialized va_list}} + // expected-note@-1{{Initialized va_list}} + va_start(va, fst); // expected-warning{{Initialized va_list 'va' is initialized again}} + // expected-note@-1{{Initialized va_list 'va' is initialized again}} +} // expected-warning{{Initialized va_list 'va' is leaked}} + // expected-note@-1{{Initialized va_list 'va' is leaked}} + +void c23_reinit_ok(int fst, ...) { + va_list va; + va_start(va, fst); + va_end(va); + va_start(va, fst); + va_end(va); +} // no-warning _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
