llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-analysis Author: Ziqing Luo (ziqingluo-90) <details> <summary>Changes</summary> 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 --- Full diff: https://github.com/llvm/llvm-project/pull/176262.diff 2 Files Affected: - (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+13-1) - (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp (+17-3) ``````````diff 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 `````````` </details> https://github.com/llvm/llvm-project/pull/176262 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
