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@-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 // appears in dependent context: #ifdef __cplusplus _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
