[clang] 6438ddf - [-Wunsafe-buffer-usage] Improve null-termination analysis on conditionals (#176262)

[via cfe-commits]

Author: Ziqing Luo
Date: 2026-01-20T09:16:54-08:00
New Revision: 6438ddf252b0e44fe97cfd77673c5fc3dabb0210

URL: https://github.com/llvm/llvm-project/commit/6438ddf252b0e44fe97cfd77673c5fc3dabb0210
DIFF: https://github.com/llvm/llvm-project/commit/6438ddf252b0e44fe97cfd77673c5fc3dabb0210.diff

LOG: [-Wunsafe-buffer-usage] Improve null-termination analysis on conditionals (#176262)

This commit adds two improvements to null-termination analysis on
conditionals:
- perform recursive constant folding
- when the condition is not a constant, analyze both branches for
null-termination

rdar://168256816

Added: 
    

Modified: 
    clang/lib/Analysis/UnsafeBufferUsage.cpp
    clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 3f5704458d59e..a7530d1250729 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -763,14 +763,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 = tryConstantFoldConditionalExpr(Ptr, Ctx);
-  if (isa<clang::StringLiteral>(Ptr->IgnoreParenImpCasts()))
+    Ptr = DefaultArgE->getExpr()->IgnoreParenImpCasts();
+  // Try to perform constant fold recursively:
+  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)) {
+    return isNullTermPointer(CondE->getLHS(), Ctx) &&
+           isNullTermPointer(CondE->getRHS(), Ctx);
+  }
+
+  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();
 
@@ -781,7 +792,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;

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..e8f16c485e05b 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,21 @@ 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 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
 // appears in dependent context:
 #ifdef __cplusplus