From 321e55fb04cc7474c12a7ea374618d16aa32cf12 Mon Sep 17 00:00:00 2001 From: Ziqing Luo Date: Thu, 13 Nov 2025 16:31:39 -0800 Subject: [PATCH 1/3] [-Wunsafe-buffer-usage] Fold the expression "cond ? E1 : E2" when checking safe patterns, if "cond" is a constant In -Wunsafe-buffer-usage, many safe pattern checks can benefit from constant folding. This commit improves null-terminated pointer checks by folding conditional expressions. rdar://159374822 --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 26 +++++++++++++--- ...n-unsafe-buffer-usage-fold-conditional.cpp | 31 +++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index f5a368636c43d..1203a27e7dae5 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -781,9 +781,25 @@ struct LibcFunNamePrefixSuffixParser { } }; +// Constant fold a conditional expression 'cond ? A : B' to +// - 'A', if 'cond' has constant true value; +// - 'B', if 'cond' has constant false value. +static const Expr *tryConstantFoldConditionalExpr(const Expr *E, + const ASTContext &Ctx) { + // FIXME: more places can use this function + if (const auto *CE = dyn_cast(E)) { + bool CondEval; + + if (CE->getCond()->EvaluateAsBooleanCondition(CondEval, Ctx)) + return CondEval ? CE->getLHS() : CE->getRHS(); + } + return E; +} + // A pointer type expression is known to be null-terminated, if it has the // form: E.c_str(), for any expression E of `std::string` type. -static bool isNullTermPointer(const Expr *Ptr) { +static bool isNullTermPointer(const Expr *Ptr, ASTContext &Ctx) { + Ptr = tryConstantFoldConditionalExpr(Ptr, Ctx); if (isa(Ptr->IgnoreParenImpCasts())) return true; if (isa(Ptr->IgnoreParenImpCasts())) @@ -874,7 +890,7 @@ static bool hasUnsafeFormatOrSArg(const CallExpr *Call, const Expr *&UnsafeArg, const Expr *Arg = Call->getArg(ArgIdx); - if (isNullTermPointer(Arg)) + if (isNullTermPointer(Arg, Ctx)) // If Arg is a null-terminated pointer, it is safe anyway. return true; // continue parsing @@ -922,8 +938,8 @@ static bool hasUnsafeFormatOrSArg(const CallExpr *Call, const Expr *&UnsafeArg, // (including the format argument) is unsafe pointer. return llvm::any_of( llvm::make_range(Call->arg_begin() + FmtArgIdx, Call->arg_end()), - [&UnsafeArg](const Expr *Arg) -> bool { - if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg)) { + [&UnsafeArg, &Ctx](const Expr *Arg) -> bool { + if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg, Ctx)) { UnsafeArg = Arg; return true; } @@ -1175,7 +1191,7 @@ static bool hasUnsafePrintfStringArg(const CallExpr &Node, ASTContext &Ctx, // We don't really recognize this "normal" printf, the only thing we // can do is to require all pointers to be null-terminated: for (const auto *Arg : Node.arguments()) - if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg)) { + if (Arg->getType()->isPointerType() && !isNullTermPointer(Arg, Ctx)) { Result.addNode(Tag, DynTypedNode::create(*Arg)); return true; } diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp new file mode 100644 index 0000000000000..1efda7318157c --- /dev/null +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp @@ -0,0 +1,31 @@ +// RUN: %clang_cc1 -fsyntax-only -std=c++20 -Wno-all -Wunsafe-buffer-usage -verify %s +// RUN: %clang_cc1 -fsyntax-only -x c -Wno-all -Wunsafe-buffer-usage -verify %s +// expected-no-diagnostics + +typedef struct {} FILE; +int fprintf( FILE* stream, const char* format, ... ); +FILE * stderr; + +#define DEBUG_ASSERT_MESSAGE(name, assertion, label, message, file, line, value) \ + fprintf(stderr, "AssertMacros: %s, %s file: %s, line: %d, value: %lld\n", \ + assertion, (message!=0) ? message : "", file, line, (long long) (value)); + + +#define Require(assertion, exceptionLabel) \ + do \ + { \ + if ( __builtin_expect(!(assertion), 0) ) { \ + DEBUG_ASSERT_MESSAGE( \ + "DEBUG_ASSERT_COMPONENT_NAME_STRING", \ + #assertion, #exceptionLabel, 0, __FILE__, __LINE__, 0); \ + goto exceptionLabel; \ + } \ + } while ( 0 ) + + +void f(int x, int y) { + Require(x == y, L1); + L1: + return; +} + From 5f7cb5719f91d7da33959f6c6ad7f7a83efaa092 Mon Sep 17 00:00:00 2001 From: Ziqing Luo Date: Fri, 14 Nov 2025 13:27:33 -0800 Subject: [PATCH 2/3] Fix clang-format issue --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 1203a27e7dae5..da155d31d4a88 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -785,7 +785,7 @@ struct LibcFunNamePrefixSuffixParser { // - 'A', if 'cond' has constant true value; // - 'B', if 'cond' has constant false value. static const Expr *tryConstantFoldConditionalExpr(const Expr *E, - const ASTContext &Ctx) { + const ASTContext &Ctx) { // FIXME: more places can use this function if (const auto *CE = dyn_cast(E)) { bool CondEval; From 5e37944207d9dcf97bfc06bdbcebc858b7cd8c5f Mon Sep 17 00:00:00 2001 From: Ziqing Luo Date: Mon, 17 Nov 2025 11:29:17 -0800 Subject: [PATCH 3/3] Update clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Take reviewer's suggestion. Co-authored-by: Balázs Benics --- .../SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp index 1efda7318157c..b4f30b533bc4b 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -fsyntax-only -std=c++20 -Wno-all -Wunsafe-buffer-usage -verify %s -// RUN: %clang_cc1 -fsyntax-only -x c -Wno-all -Wunsafe-buffer-usage -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wno-all -Wunsafe-buffer-usage -verify %s -std=c++20 +// RUN: %clang_cc1 -fsyntax-only -Wno-all -Wunsafe-buffer-usage -verify %s -x c // expected-no-diagnostics typedef struct {} FILE;