https://github.com/ziqingluo-90 updated https://github.com/llvm/llvm-project/pull/172091
>From 00349cc36ead6258c94499f8b7702deb5f0ff7c8 Mon Sep 17 00:00:00 2001 From: Ziqing Luo <[email protected]> Date: Fri, 12 Dec 2025 13:45:07 -0800 Subject: [PATCH] [-Wunsafe-buffer-usage] Check isValueDependent before EvaluateAsBooleanCondition The function EvaluateAsBooleanCondition assumes (asserts) that the input Expr is not in a dependent context. So it is caller's responsibility to check it before the call. This commit fixes the issue in UnsafeBufferUsage.cpp The issue was first found downstream because there was code not upstreamed. This commit also includes those un-upstreamed code. rdar://166217941 --- clang/lib/Analysis/UnsafeBufferUsage.cpp | 74 ++++++++++--------- ...n-unsafe-buffer-usage-fold-conditional.cpp | 13 ++++ ...arn-unsafe-buffer-usage-libc-functions.cpp | 2 + 3 files changed, 55 insertions(+), 34 deletions(-) diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index da155d31d4a88..0b9e908517412 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -736,6 +736,42 @@ static bool isSafeArraySubscript(const ArraySubscriptExpr &Node, return false; } +// Constant fold a conditional expression 'cond ? A : B' to +// - 'A', if 'cond' has constant true value; +// - 'B', if 'cond' has constant false value. +static const Expr *tryConstantFoldConditionalExpr(const Expr *E, + const ASTContext &Ctx) { + // FIXME: more places can use this function + if (const auto *CE = dyn_cast<ConditionalOperator>(E)) { + bool CondEval; + const auto *Cond = CE->getCond(); + + if (!Cond->isValueDependent() && + Cond->EvaluateAsBooleanCondition(CondEval, Ctx)) + return CondEval ? CE->getLHS() : CE->getRHS(); + } + return E; +} + +// A pointer type expression is known to be null-terminated, if it has the +// form: E.c_str(), for any expression E of `std::string` type. +static bool isNullTermPointer(const Expr *Ptr, ASTContext &Ctx) { + Ptr = tryConstantFoldConditionalExpr(Ptr, Ctx); + if (isa<clang::StringLiteral>(Ptr->IgnoreParenImpCasts())) + return true; + if (isa<PredefinedExpr>(Ptr->IgnoreParenImpCasts())) + return true; + if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Ptr->IgnoreParenImpCasts())) { + const CXXMethodDecl *MD = MCE->getMethodDecl(); + const CXXRecordDecl *RD = MCE->getRecordDecl()->getCanonicalDecl(); + + if (MD && RD && RD->isInStdNamespace() && MD->getIdentifier()) + if (MD->getName() == "c_str" && RD->getName() == "basic_string") + return true; + } + return false; +} + namespace libc_func_matchers { // Under `libc_func_matchers`, define a set of matchers that match unsafe // functions in libc and unsafe calls to them. @@ -781,40 +817,6 @@ struct LibcFunNamePrefixSuffixParser { } }; -// Constant fold a conditional expression 'cond ? A : B' to -// - 'A', if 'cond' has constant true value; -// - 'B', if 'cond' has constant false value. -static const Expr *tryConstantFoldConditionalExpr(const Expr *E, - const ASTContext &Ctx) { - // FIXME: more places can use this function - if (const auto *CE = dyn_cast<ConditionalOperator>(E)) { - bool CondEval; - - if (CE->getCond()->EvaluateAsBooleanCondition(CondEval, Ctx)) - return CondEval ? CE->getLHS() : CE->getRHS(); - } - return E; -} - -// A pointer type expression is known to be null-terminated, if it has the -// form: E.c_str(), for any expression E of `std::string` type. -static bool isNullTermPointer(const Expr *Ptr, ASTContext &Ctx) { - Ptr = tryConstantFoldConditionalExpr(Ptr, Ctx); - if (isa<clang::StringLiteral>(Ptr->IgnoreParenImpCasts())) - return true; - if (isa<PredefinedExpr>(Ptr->IgnoreParenImpCasts())) - return true; - if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Ptr->IgnoreParenImpCasts())) { - const CXXMethodDecl *MD = MCE->getMethodDecl(); - const CXXRecordDecl *RD = MCE->getRecordDecl()->getCanonicalDecl(); - - if (MD && RD && RD->isInStdNamespace() && MD->getIdentifier()) - if (MD->getName() == "c_str" && RD->getName() == "basic_string") - return true; - } - return false; -} - // Return true iff at least one of following cases holds: // 1. Format string is a literal and there is an unsafe pointer argument // corresponding to an `s` specifier; @@ -2106,6 +2108,10 @@ class UnsafeLibcFunctionCallGadget : public WarningGadget { // function that is not in any namespace: if (!FD->isInStdNamespace() && !IsGlobalAndNotInAnyNamespace) return false; + // If the call has a sole null-terminated argument, e.g., strlen, + // printf, atoi, we consider it safe: + if (CE->getNumArgs() == 1 && isNullTermPointer(CE->getArg(0), Ctx)) + return false; auto isSingleStringLiteralArg = false; if (CE->getNumArgs() == 1) { isSingleStringLiteralArg = 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 b4f30b533bc4b..ad914b4fea89c 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp @@ -29,3 +29,16 @@ void f(int x, int y) { return; } +// Test that the analysis will not crash when a conditional expression +// appears in dependent context: +#ifdef __cplusplus +struct Foo { + static void static_method(int); +}; +void conditional_inside_dependent_context(void) { + auto lambda = [](auto result) { // opens a dependent context + Foo::static_method(result ? 1 : 2); + }; + (void)lambda; +} +#endif // __cplusplus diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp index 765dcbcc07df5..b23b4acd0d1c7 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp @@ -156,6 +156,8 @@ void safe_examples(std::string s1, int *p) { snprintf(a, 10, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn snprintf(&c, 1, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn snprintf(nullptr, 0, "%s%d%s%p%s", __PRETTY_FUNCTION__, *p, "hello", s1.c_str()); // no warn + + strlen(s1.c_str()); } void test_sarg_precision(std::string Str, std::string_view Sv, std::wstring_view WSv, _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
