From da3dc0011e06c9f1aebe71d76eae92dc76e3db2e Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 5 Feb 2020 19:01:33 -0800 Subject: [PATCH] PR44684: Look through parens and similar constructs when determining whether a call is to a builtin. We already had a general mechanism to do this but for some reason weren't using it. In passing, check for the other unary operators that can intervene in a reasonably-direct function call (we already handled '&' but missed '*' and '+'). This reverts commit aaae6b1b617378362462c1685e754813ed82b394, reinstating af80b8ccc5772c14920d4554b7ca7e15f2fad1c4, with a fix to clang-tidy. --- .../modernize/UseUncaughtExceptionsCheck.cpp | 9 ++-- clang/lib/AST/Expr.cpp | 52 +++++++++------------- clang/lib/AST/ExprConstant.cpp | 2 +- clang/test/Parser/builtin_classify_type.c | 2 +- clang/test/Sema/constant-builtins.c | 11 ++++- 5 files changed, 40 insertions(+), 36 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp index cb3f551..a7a0746 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp @@ -34,15 +34,18 @@ void UseUncaughtExceptionsCheck::registerMatchers(MatchFinder *Finder) { .bind("decl_ref_expr"), this); + auto DirectCallToUncaughtException = callee(expr(ignoringImpCasts( + declRefExpr(hasDeclaration(functionDecl(hasName(MatchText))))))); + // CallExpr: warning, fix-it. - Finder->addMatcher(callExpr(hasDeclaration(functionDecl(hasName(MatchText))), + Finder->addMatcher(callExpr(DirectCallToUncaughtException, unless(hasAncestor(initListExpr()))) .bind("call_expr"), this); // CallExpr in initialisation list: warning, fix-it with avoiding narrowing // conversions. - Finder->addMatcher(callExpr(hasAncestor(initListExpr()), - hasDeclaration(functionDecl(hasName(MatchText)))) + Finder->addMatcher(callExpr(DirectCallToUncaughtException, + hasAncestor(initListExpr())) .bind("init_call_expr"), this); } diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index ac4fa12..d929161 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -1446,19 +1446,28 @@ void CallExpr::updateDependenciesFromArg(Expr *Arg) { Decl *Expr::getReferencedDeclOfCallee() { Expr *CEE = IgnoreParenImpCasts(); - while (SubstNonTypeTemplateParmExpr *NTTP - = dyn_cast(CEE)) { - CEE = NTTP->getReplacement()->IgnoreParenCasts(); + while (SubstNonTypeTemplateParmExpr *NTTP = + dyn_cast(CEE)) { + CEE = NTTP->getReplacement()->IgnoreParenImpCasts(); } // If we're calling a dereference, look at the pointer instead. - if (BinaryOperator *BO = dyn_cast(CEE)) { - if (BO->isPtrMemOp()) - CEE = BO->getRHS()->IgnoreParenCasts(); - } else if (UnaryOperator *UO = dyn_cast(CEE)) { - if (UO->getOpcode() == UO_Deref) - CEE = UO->getSubExpr()->IgnoreParenCasts(); + while (true) { + if (BinaryOperator *BO = dyn_cast(CEE)) { + if (BO->isPtrMemOp()) { + CEE = BO->getRHS()->IgnoreParenImpCasts(); + continue; + } + } else if (UnaryOperator *UO = dyn_cast(CEE)) { + if (UO->getOpcode() == UO_Deref || UO->getOpcode() == UO_AddrOf || + UO->getOpcode() == UO_Plus) { + CEE = UO->getSubExpr()->IgnoreParenImpCasts(); + continue; + } + } + break; } + if (DeclRefExpr *DRE = dyn_cast(CEE)) return DRE->getDecl(); if (MemberExpr *ME = dyn_cast(CEE)) @@ -1469,28 +1478,11 @@ Decl *Expr::getReferencedDeclOfCallee() { return nullptr; } -/// getBuiltinCallee - If this is a call to a builtin, return the builtin ID. If -/// not, return 0. +/// If this is a call to a builtin, return the builtin ID. If not, return 0. unsigned CallExpr::getBuiltinCallee() const { - // All simple function calls (e.g. func()) are implicitly cast to pointer to - // function. As a result, we try and obtain the DeclRefExpr from the - // ImplicitCastExpr. - const ImplicitCastExpr *ICE = dyn_cast(getCallee()); - if (!ICE) // FIXME: deal with more complex calls (e.g. (func)(), (*func)()). - return 0; - - const DeclRefExpr *DRE = dyn_cast(ICE->getSubExpr()); - if (!DRE) - return 0; - - const FunctionDecl *FDecl = dyn_cast(DRE->getDecl()); - if (!FDecl) - return 0; - - if (!FDecl->getIdentifier()) - return 0; - - return FDecl->getBuiltinID(); + auto *FDecl = + dyn_cast_or_null(getCallee()->getReferencedDeclOfCallee()); + return FDecl ? FDecl->getBuiltinID() : 0; } bool CallExpr::isUnevaluatedBuiltinCall(const ASTContext &Ctx) const { diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index f045e5f..860eeb1 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -10684,7 +10684,7 @@ static bool getBuiltinAlignArguments(const CallExpr *E, EvalInfo &Info, bool IntExprEvaluator::VisitBuiltinCallExpr(const CallExpr *E, unsigned BuiltinOp) { - switch (unsigned BuiltinOp = E->getBuiltinCallee()) { + switch (BuiltinOp) { default: return ExprEvaluatorBaseTy::VisitCallExpr(E); diff --git a/clang/test/Parser/builtin_classify_type.c b/clang/test/Parser/builtin_classify_type.c index 63fd8e2..94434e9 100644 --- a/clang/test/Parser/builtin_classify_type.c +++ b/clang/test/Parser/builtin_classify_type.c @@ -9,7 +9,7 @@ int main() { struct foo s; static int ary[__builtin_classify_type(a)]; - static int ary2[(__builtin_classify_type)(a)]; // expected-error{{variable length array declaration cannot have 'static' storage duration}} + static int ary2[(__builtin_classify_type)(a)]; static int ary3[(*__builtin_classify_type)(a)]; // expected-error{{builtin functions must be directly called}} int result; diff --git a/clang/test/Sema/constant-builtins.c b/clang/test/Sema/constant-builtins.c index c98f62d..ae3b913 100644 --- a/clang/test/Sema/constant-builtins.c +++ b/clang/test/Sema/constant-builtins.c @@ -25,4 +25,13 @@ short somefunc(); short t = __builtin_constant_p(5353) ? 42 : somefunc(); - +// PR44684 +_Static_assert((__builtin_clz)(1u) >= 15, ""); +_Static_assert((__builtin_popcount)(1u) == 1, ""); +_Static_assert((__builtin_ctz)(2u) == 1, ""); +_Static_assert(_Generic(1u,unsigned:__builtin_clz)(1u) >= 15, ""); +_Static_assert(_Generic(1u,unsigned:__builtin_popcount)(1u) == 1, ""); +_Static_assert(_Generic(1u,unsigned:__builtin_ctz)(2u) == 1, ""); + +__SIZE_TYPE__ strlen(const char*); +_Static_assert((__builtin_constant_p(1) ? (***&strlen)("foo") : 0) == 3, ""); -- 2.7.4