[clang] [-Wunsafe-buffer-usage] Improve null-termination analysis on conditionals (PR #176262)
Ziqing Luo via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 16 15:35:49 PST 2026
https://github.com/ziqingluo-90 updated https://github.com/llvm/llvm-project/pull/176262
>From ad882b32bc6ed74795a7ce8ce793e7cf9347034e Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo at apple.com>
Date: Thu, 15 Jan 2026 14:17:24 -0800
Subject: [PATCH 1/3] [-Wunsafe-buffer-usage] Improve null-termination analysis
on conditionals
This commit adds two improvements to null-termination analysis on
conditionals
- perform recursive constant folding on the conditions
- when the condition is not a constant, analyze both branches for null-termination
rdar://168256816
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 14 ++++++++++++-
...n-unsafe-buffer-usage-fold-conditional.cpp | 20 ++++++++++++++++---
2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 6bb08102c0345..65b8d0ab7e4c1 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -758,7 +758,19 @@ static bool isNullTermPointer(const Expr *Ptr, ASTContext &Ctx) {
// Strip CXXDefaultArgExpr before check:
if (const auto *DefaultArgE = dyn_cast<CXXDefaultArgExpr>(Ptr))
Ptr = DefaultArgE->getExpr();
- Ptr = tryConstantFoldConditionalExpr(Ptr, Ctx);
+ // Try to perform constant fold recursively:
+ if (const auto *NewPtr =
+ tryConstantFoldConditionalExpr(Ptr->IgnoreParenImpCasts(), Ctx);
+ NewPtr != Ptr)
+ return isNullTermPointer(NewPtr, Ctx);
+ // Split the analysis for conditional expressions that cannot be
+ // constant-folded:
+ if (const auto *CondE =
+ dyn_cast<ConditionalOperator>(Ptr->IgnoreParenImpCasts())) {
+ return isNullTermPointer(CondE->getLHS(), Ctx) &&
+ isNullTermPointer(CondE->getRHS(), Ctx);
+ }
+
if (isa<clang::StringLiteral>(Ptr->IgnoreParenImpCasts()))
return true;
if (isa<PredefinedExpr>(Ptr->IgnoreParenImpCasts()))
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 01f7bdf0cf94b..13ba47e5acb42 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp
@@ -1,6 +1,6 @@
-// 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
+// RUN: %clang_cc1 -fsyntax-only -Wno-all -Wunsafe-buffer-usage -verify=cxx %s -std=c++20
+// RUN: %clang_cc1 -fsyntax-only -Wno-all -Wunsafe-buffer-usage -verify=c-only %s -x c
+// c-only-no-diagnostics
typedef struct {} FILE;
int fprintf( FILE* stream, const char* format, ... );
@@ -29,6 +29,20 @@ void f(int x, int y) {
return;
}
+// Test nested conditional expressions:
+void testNested(char * message) {
+ fprintf(stderr, "AssertMacros: %s", (0!=0) ? message : ((0!=0) ? message : ""));
+}
+
+// If the conditional cannot be constant-folded, try analyze both branches:
+void testConditionalAnalysis(char * message, int x) {
+ fprintf(stderr, "AssertMacros: %s", (x!=0) ? "hello" : "world");
+ fprintf(stderr, "AssertMacros: %s", (0!=0) ? message : ((x!=0) ? "hello" : "world"));
+ fprintf(stderr, "AssertMacros: %s", (x!=0) ? (((x!=0) ? "hello" : "world")) : ((x!=0) ? "hello" : "world"));
+ fprintf(stderr, "AssertMacros: %s", (x!=0) ? (((x!=0) ? "hello" : "world")) : ((x!=0) ? "hello" : message)); //\
+ cxx-warning{{function 'fprintf' is unsafe}} cxx-note{{string argument is not guaranteed to be null-terminated}}
+}
+
// Test that the analysis will not crash when a conditional expression
// appears in dependent context:
#ifdef __cplusplus
>From 7e982bfa95364ed47dcb19c4cadafcedab7d9bb3 Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing_luo at apple.com>
Date: Fri, 16 Jan 2026 15:28:18 -0800
Subject: [PATCH 2/3] Address comments
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 65b8d0ab7e4c1..4fcdd7e43efeb 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -756,26 +756,25 @@ static const Expr *tryConstantFoldConditionalExpr(const Expr *E,
// form: E.c_str(), for any expression E of `std::string` type.
static bool isNullTermPointer(const Expr *Ptr, ASTContext &Ctx) {
// Strip CXXDefaultArgExpr before check:
+ Ptr = Ptr->IgnoreParenImpCasts();
if (const auto *DefaultArgE = dyn_cast<CXXDefaultArgExpr>(Ptr))
- Ptr = DefaultArgE->getExpr();
+ Ptr = DefaultArgE->getExpr()->IgnoreParenImpCasts();
// Try to perform constant fold recursively:
- if (const auto *NewPtr =
- tryConstantFoldConditionalExpr(Ptr->IgnoreParenImpCasts(), Ctx);
+ if (const auto *NewPtr = tryConstantFoldConditionalExpr(Ptr, Ctx);
NewPtr != Ptr)
return isNullTermPointer(NewPtr, Ctx);
// Split the analysis for conditional expressions that cannot be
// constant-folded:
- if (const auto *CondE =
- dyn_cast<ConditionalOperator>(Ptr->IgnoreParenImpCasts())) {
+ if (const auto *CondE = dyn_cast<ConditionalOperator>(Ptr)) {
return isNullTermPointer(CondE->getLHS(), Ctx) &&
isNullTermPointer(CondE->getRHS(), Ctx);
}
- if (isa<clang::StringLiteral>(Ptr->IgnoreParenImpCasts()))
+ if (isa<clang::StringLiteral>(Ptr))
return true;
- if (isa<PredefinedExpr>(Ptr->IgnoreParenImpCasts()))
+ if (isa<PredefinedExpr>(Ptr))
return true;
- if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Ptr->IgnoreParenImpCasts())) {
+ if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Ptr)) {
const CXXMethodDecl *MD = MCE->getMethodDecl();
const CXXRecordDecl *RD = MCE->getRecordDecl()->getCanonicalDecl();
@@ -786,7 +785,7 @@ static bool isNullTermPointer(const Expr *Ptr, ASTContext &Ctx) {
// Functions known to return properly null terminated strings.
static const llvm::StringSet<> NullTermFunctions = {"strerror"};
- if (auto *CE = dyn_cast<CallExpr>(Ptr->IgnoreParenImpCasts())) {
+ if (auto *CE = dyn_cast<CallExpr>(Ptr)) {
const FunctionDecl *F = CE->getDirectCallee();
if (F && F->getIdentifier() && NullTermFunctions.contains(F->getName()))
return true;
>From 91eb2e30a945b3c2de9de0ca547e63af122dc8c8 Mon Sep 17 00:00:00 2001
From: Ziqing Luo <ziqing at udel.edu>
Date: Fri, 16 Jan 2026 15:33:35 -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 steakhal's format
Co-authored-by: Balázs Benics <benicsbalazs at gmail.com>
---
.../SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp | 5 +++--
1 file changed, 3 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 13ba47e5acb42..e8f16c485e05b 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp
@@ -39,8 +39,9 @@ void testConditionalAnalysis(char * message, int x) {
fprintf(stderr, "AssertMacros: %s", (x!=0) ? "hello" : "world");
fprintf(stderr, "AssertMacros: %s", (0!=0) ? message : ((x!=0) ? "hello" : "world"));
fprintf(stderr, "AssertMacros: %s", (x!=0) ? (((x!=0) ? "hello" : "world")) : ((x!=0) ? "hello" : "world"));
- fprintf(stderr, "AssertMacros: %s", (x!=0) ? (((x!=0) ? "hello" : "world")) : ((x!=0) ? "hello" : message)); //\
- cxx-warning{{function 'fprintf' is unsafe}} cxx-note{{string argument is not guaranteed to be null-terminated}}
+ fprintf(stderr, "AssertMacros: %s", (x!=0) ? (((x!=0) ? "hello" : "world")) : ((x!=0) ? "hello" : message));
+ // cxx-warning at -1 {{function 'fprintf' is unsafe}}
+ // cxx-note at -2 {{string argument is not guaranteed to be null-terminated}}
}
// Test that the analysis will not crash when a conditional expression
More information about the cfe-commits
mailing list