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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> --- .../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@-1 {{function 'fprintf' is unsafe}} + // cxx-note@-2 {{string argument is not guaranteed to be null-terminated}} } // Test that the analysis will not crash when a conditional expression _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
