https://github.com/bjosv updated https://github.com/llvm/llvm-project/pull/192024
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 1/3] [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 From a483211c84cdc15681fd583bb185ab5e085cf4f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <[email protected]> Date: Mon, 4 May 2026 12:59:41 +0200 Subject: [PATCH 2/3] fixup: handle review comment to use __restrict and matchesAny MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Björn Svensson <[email protected]> --- clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp | 2 +- .../Analysis/Inputs/system-header-simulator-for-valist-c23.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp index bc8c82100b6fe..f9172e205699e 100644 --- a/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp @@ -143,7 +143,7 @@ const CallDescription VAListChecker::VaStart(CDM::CLibrary, void VAListChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { - if (VaStart.matches(Call) || VaStartC23.matches(Call)) + if (matchesAny(Call, VaStart, VaStartC23)) 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 index 89bc95abdad5c..bcca99852c53b 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist-c23.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist-c23.h @@ -13,4 +13,4 @@ typedef __builtin_va_list va_list; #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); +int vprintf(const char *__restrict format, va_list arg); From 2dc6b381a2f49f2ff9c6b0f5af0ce77fda9aab45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <[email protected]> Date: Tue, 5 May 2026 14:42:04 +0200 Subject: [PATCH 3/3] fixup: reuse existing valist testcases for C23 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Björn Svensson <[email protected]> --- .../system-header-simulator-for-valist-c23.h | 2 + .../test/Analysis/valist-uninitialized-c23.c | 61 ------------------- clang/test/Analysis/valist-uninitialized.c | 27 ++++++++ clang/test/Analysis/valist-unterminated-c23.c | 35 ----------- clang/test/Analysis/valist-unterminated.c | 6 ++ 5 files changed, 35 insertions(+), 96 deletions(-) delete mode 100644 clang/test/Analysis/valist-uninitialized-c23.c delete mode 100644 clang/test/Analysis/valist-unterminated-c23.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 index bcca99852c53b..f8aa536b5f41c 100644 --- a/clang/test/Analysis/Inputs/system-header-simulator-for-valist-c23.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist-c23.h @@ -14,3 +14,5 @@ typedef __builtin_va_list va_list; #define va_copy(dst, src) __builtin_va_copy(dst, src) int vprintf(const char *__restrict format, va_list arg); + +int some_library_function(int n, va_list arg); diff --git a/clang/test/Analysis/valist-uninitialized-c23.c b/clang/test/Analysis/valist-uninitialized-c23.c deleted file mode 100644 index 9ec99ffe3c74b..0000000000000 --- a/clang/test/Analysis/valist-uninitialized-c23.c +++ /dev/null @@ -1,61 +0,0 @@ -// 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-uninitialized.c b/clang/test/Analysis/valist-uninitialized.c index f28f928024315..d964df6f11b7e 100644 --- a/clang/test/Analysis/valist-uninitialized.c +++ b/clang/test/Analysis/valist-uninitialized.c @@ -10,8 +10,25 @@ // // RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu %s \ // RUN: -analyzer-checker=core,security.VAList +// +// 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 +#if __STDC_VERSION__ >= 202311L +#include "Inputs/system-header-simulator-for-valist-c23.h" +#else #include "Inputs/system-header-simulator-for-valist.h" +#endif void f1(int fst, ...) { va_list va; @@ -164,3 +181,13 @@ void all_state_changes(va_list unknown, int fst, ...) { va_end(va); // expected-warning{{va_end() is called on an already released va_list}} // expected-note@-1{{va_end() is called on an already released va_list}} } + +#if __STDC_VERSION__ >= 202311L +// C23 allows va_start with a single argument (no second parameter required). +void c23_one_arg_no_warning(int fst, ...) { + va_list va; + va_start(va); + (void)va_arg(va, int); + va_end(va); +} // no-warning +#endif diff --git a/clang/test/Analysis/valist-unterminated-c23.c b/clang/test/Analysis/valist-unterminated-c23.c deleted file mode 100644 index 8f1aae919239f..0000000000000 --- a/clang/test/Analysis/valist-unterminated-c23.c +++ /dev/null @@ -1,35 +0,0 @@ -// 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 diff --git a/clang/test/Analysis/valist-unterminated.c b/clang/test/Analysis/valist-unterminated.c index 93e2074005512..bf07714adf5ec 100644 --- a/clang/test/Analysis/valist-unterminated.c +++ b/clang/test/Analysis/valist-unterminated.c @@ -1,7 +1,13 @@ // RUN: %clang_analyze_cc1 -triple hexagon-unknown-linux -analyzer-checker=core,security.VAList -analyzer-output=text -verify %s // RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,security.VAList -analyzer-output=text -verify %s +// RUN: %clang_analyze_cc1 -triple hexagon-unknown-linux -std=c23 -analyzer-checker=core,security.VAList -analyzer-output=text -verify %s +// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -std=c23 -analyzer-checker=core,security.VAList -analyzer-output=text -verify %s +#if __STDC_VERSION__ >= 202311L +#include "Inputs/system-header-simulator-for-valist-c23.h" +#else #include "Inputs/system-header-simulator-for-valist.h" +#endif void f1(int fst, ...) { va_list va; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
