llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-analysis Author: Ziqing Luo (ziqingluo-90) <details> <summary>Changes</summary> The function `EvaluateAsBooleanCondition` assumes (asserts) that the input `Expr` is not in a dependent context. So it is caller's responsibility to check the condition before the call. This commit fixes the issue in `UnsafeBufferUsage.cpp`. The issue was first found downstream because of some code not upstreamed. This commit also includes those un-upstreamed code. rdar://166217941 --- Full diff: https://github.com/llvm/llvm-project/pull/172091.diff 3 Files Affected: - (modified) clang/lib/Analysis/UnsafeBufferUsage.cpp (+40-34) - (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-fold-conditional.cpp (+13) - (modified) clang/test/SemaCXX/warn-unsafe-buffer-usage-libc-functions.cpp (+2) ``````````diff 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, `````````` </details> https://github.com/llvm/llvm-project/pull/172091 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
