Author: Björn Svensson Date: 2026-05-06T16:07:35+01:00 New Revision: 3a85bd776903ec83334276762de27b453be04f21
URL: https://github.com/llvm/llvm-project/commit/3a85bd776903ec83334276762de27b453be04f21 DIFF: https://github.com/llvm/llvm-project/commit/3a85bd776903ec83334276762de27b453be04f21.diff LOG: [analyzer] Fix security.VAList false positives with C23 va_start (#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`. --------- Signed-off-by: Björn Svensson <[email protected]> Added: clang/test/Analysis/Inputs/system-header-simulator-for-valist-c23.h Modified: clang/docs/ReleaseNotes.rst clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp clang/test/Analysis/valist-uninitialized.c clang/test/Analysis/valist-unterminated.c Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index eb1d926926d9e..c92bd8dad11af 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -745,6 +745,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..9632997a7717a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VAListChecker.cpp @@ -64,7 +64,8 @@ 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 CallDescriptionSet VaStart; + static const CallDescription VaEnd, VaCopy; public: void checkPreStmt(const VAArgExpr *VAA, CheckerContext &C) const; @@ -133,16 +134,18 @@ const SmallVector<VAListChecker::VAListAccepter, 15> // vsnprintf, vsprintf has no wide version {{CDM::CLibrary, {"vswscanf"}, 3}, 2}}; -const CallDescription VAListChecker::VaStart(CDM::CLibrary, - {"__builtin_va_start"}, /*Args=*/2, - /*Params=*/1), - VAListChecker::VaCopy(CDM::CLibrary, {"__builtin_va_copy"}, 2), +const CallDescriptionSet VAListChecker::VaStart{ + {CDM::CLibrary, {"__builtin_va_start"}}, + {CDM::CLibrary, {"__builtin_c23_va_start"}}}; + +const CallDescription 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.contains(Call)) checkVAListStartCall(Call, C); else if (VaCopy.matches(Call)) checkVAListCopyCall(Call, C); @@ -294,6 +297,9 @@ void VAListChecker::reportLeaked(const RegionVector &Leaked, StringRef Msg1, void VAListChecker::checkVAListStartCall(const CallEvent &Call, CheckerContext &C) const { + if (Call.getNumArgs() == 0) + return; // Prevent a crash on grossly invalid input. + const MemRegion *Arg = getVAListAsRegion(Call.getArgSVal(0), Call.getArgExpr(0), C); if (!Arg) 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..f8aa536b5f41c --- /dev/null +++ b/clang/test/Analysis/Inputs/system-header-simulator-for-valist-c23.h @@ -0,0 +1,18 @@ +// Like the compiler, the static analyzer treats some functions diff erently 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); + +int some_library_function(int n, va_list arg); 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.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
