From: George Burgess IV Date: Thu, 7 Jan 2016 02:26:57 +0000 (+0000) Subject: [Sema] Teach overload resolution about unaddressable functions. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b22d422bb762bc73637972953acfcf5d1ba646c3;p=clang [Sema] Teach overload resolution about unaddressable functions. Given an expression like `(&Foo)();`, we perform overload resolution as if we are calling `Foo` directly. This causes problems if `Foo` is a function that can't have its address taken. This patch teaches overload resolution to ignore functions that can't have their address taken in such cases. Differential Revision: http://reviews.llvm.org/D15590 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@257016 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/Overload.h b/include/clang/Sema/Overload.h index 20958b057a..62437953b3 100644 --- a/include/clang/Sema/Overload.h +++ b/include/clang/Sema/Overload.h @@ -570,8 +570,8 @@ namespace clang { /// This conversion candidate is not viable because its result /// type is not implicitly convertible to the desired type. ovl_fail_bad_final_conversion, - - /// This conversion function template specialization candidate is not + + /// This conversion function template specialization candidate is not /// viable because the final conversion was not an exact match. ovl_fail_final_conversion_not_exact, @@ -582,7 +582,10 @@ namespace clang { /// This candidate function was not viable because an enable_if /// attribute disabled it. - ovl_fail_enable_if + ovl_fail_enable_if, + + /// This candidate was not viable because its address could not be taken. + ovl_fail_addr_not_available }; /// OverloadCandidate - A single candidate in an overload set (C++ 13.3). diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 77d06f2aff..279afbe740 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -2548,7 +2548,8 @@ public: MultiExprArg Args, SourceLocation RParenLoc, Expr *ExecConfig, - bool AllowTypoCorrection=true); + bool AllowTypoCorrection=true, + bool CalleesAddressIsTaken=false); bool buildOverloadedCallSet(Scope *S, Expr *Fn, UnresolvedLookupExpr *ULE, MultiExprArg Args, SourceLocation RParenLoc, diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 8e50d5a103..67d5db15cf 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -4935,7 +4935,9 @@ Sema::ActOnCallExpr(Scope *S, Expr *Fn, SourceLocation LParenLoc, OverloadExpr *ovl = find.Expression; if (UnresolvedLookupExpr *ULE = dyn_cast(ovl)) return BuildOverloadedCallExpr(S, Fn, ULE, LParenLoc, ArgExprs, - RParenLoc, ExecConfig); + RParenLoc, ExecConfig, + /*AllowTypoCorrection=*/true, + find.IsAddressOfOperand); return BuildCallToMemberFunction(S, Fn, LParenLoc, ArgExprs, RParenLoc); } } @@ -4949,10 +4951,14 @@ Sema::ActOnCallExpr(Scope *S, Expr *Fn, SourceLocation LParenLoc, Expr *NakedFn = Fn->IgnoreParens(); + bool CallingNDeclIndirectly = false; NamedDecl *NDecl = nullptr; - if (UnaryOperator *UnOp = dyn_cast(NakedFn)) - if (UnOp->getOpcode() == UO_AddrOf) + if (UnaryOperator *UnOp = dyn_cast(NakedFn)) { + if (UnOp->getOpcode() == UO_AddrOf) { + CallingNDeclIndirectly = true; NakedFn = UnOp->getSubExpr()->IgnoreParens(); + } + } if (isa(NakedFn)) { NDecl = cast(NakedFn)->getDecl(); @@ -4974,6 +4980,11 @@ Sema::ActOnCallExpr(Scope *S, Expr *Fn, SourceLocation LParenLoc, NDecl = cast(NakedFn)->getMemberDecl(); if (FunctionDecl *FD = dyn_cast_or_null(NDecl)) { + if (CallingNDeclIndirectly && + !checkAddressOfFunctionIsAvailable(FD, /*Complain=*/true, + Fn->getLocStart())) + return ExprError(); + if (FD->hasAttr()) { if (const EnableIfAttr *Attr = CheckEnableIf(FD, ArgExprs, true)) { Diag(Fn->getLocStart(), diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp index e0c10e4479..682f3692ba 100644 --- a/lib/Sema/SemaOverload.cpp +++ b/lib/Sema/SemaOverload.cpp @@ -9643,6 +9643,13 @@ static void NoteFunctionCandidate(Sema &S, OverloadCandidate *Cand, case ovl_fail_enable_if: return DiagnoseFailedEnableIfAttr(S, Cand); + + case ovl_fail_addr_not_available: { + bool Available = checkAddressOfCandidateIsAvailable(S, Cand->Function); + (void)Available; + assert(!Available); + break; + } } } @@ -11245,6 +11252,17 @@ static ExprResult FinishOverloadedCallExpr(Sema &SemaRef, Scope *S, Expr *Fn, return ExprError(); } +static void markUnaddressableCandidatesUnviable(Sema &S, + OverloadCandidateSet &CS) { + for (auto I = CS.begin(), E = CS.end(); I != E; ++I) { + if (I->Viable && + !S.checkAddressOfFunctionIsAvailable(I->Function, /*Complain=*/false)) { + I->Viable = false; + I->FailureKind = ovl_fail_addr_not_available; + } + } +} + /// BuildOverloadedCallExpr - Given the call expression that calls Fn /// (which eventually refers to the declaration Func) and the call /// arguments Args/NumArgs, attempt to resolve the function call down @@ -11257,7 +11275,8 @@ ExprResult Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn, MultiExprArg Args, SourceLocation RParenLoc, Expr *ExecConfig, - bool AllowTypoCorrection) { + bool AllowTypoCorrection, + bool CalleesAddressIsTaken) { OverloadCandidateSet CandidateSet(Fn->getExprLoc(), OverloadCandidateSet::CSK_Normal); ExprResult result; @@ -11266,6 +11285,11 @@ ExprResult Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn, &result)) return result; + // If the user handed us something like `(&Foo)(Bar)`, we need to ensure that + // functions that aren't addressible are considered unviable. + if (CalleesAddressIsTaken) + markUnaddressableCandidatesUnviable(*this, CandidateSet); + OverloadCandidateSet::iterator Best; OverloadingResult OverloadResult = CandidateSet.BestViableFunction(*this, Fn->getLocStart(), Best); diff --git a/test/CodeGenCXX/pass-object-size.cpp b/test/CodeGenCXX/pass-object-size.cpp index 254669b976..2c7f9742a8 100644 --- a/test/CodeGenCXX/pass-object-size.cpp +++ b/test/CodeGenCXX/pass-object-size.cpp @@ -25,3 +25,21 @@ void Lambdas(char *ptr) { // CHECK-DAG: define internal i64 @"_ZZN7lambdas7LambdasEPcENK3$_1clEPvU17pass_object_size0" // CHECK-NOT: call i64 @llvm.objectsize } + +// This is here instead of in Sema/ because we need to check to make sure the +// proper function is called. If it's not, we'll end up with assertion errors. +namespace addrof { +void OvlFoo(void *const __attribute__((pass_object_size(0)))) {} +void OvlFoo(int *const) {} + +// CHECK: define void @_ZN6addrof4TestEv +void Test() { + // Treating parens-only calls as though they were direct is consistent with + // how we handle other implicitly unaddressable functions (e.g. builtins). + // CHECK: call void @_ZN6addrof6OvlFooEPvU17pass_object_size0 + (OvlFoo)(nullptr); + + // CHECK: call void @_ZN6addrof6OvlFooEPi + (&OvlFoo)(nullptr); +} +} diff --git a/test/Sema/pass-object-size.c b/test/Sema/pass-object-size.c index e4f460b178..6f375c0e94 100644 --- a/test/Sema/pass-object-size.c +++ b/test/Sema/pass-object-size.c @@ -33,7 +33,7 @@ void TakeFnOvl(void (*)(int *)) overloaded; void NotOverloaded(void *p PS(0)); void IsOverloaded(void *p PS(0)) overloaded; -void IsOverloaded(char *p) overloaded; +void IsOverloaded(char *p) overloaded; // char* inestead of void* is intentional void FunctionPtrs() { void (*p)(void *) = NotOverloaded; //expected-error{{cannot take address of function 'NotOverloaded' because parameter 1 has pass_object_size attribute}} void (*p2)(void *) = &NotOverloaded; //expected-error{{cannot take address of function 'NotOverloaded' because parameter 1 has pass_object_size attribute}} @@ -49,4 +49,8 @@ void FunctionPtrs() { TakeFnOvl(NotOverloaded); //expected-error{{cannot take address of function 'NotOverloaded' because parameter 1 has pass_object_size attribute}} TakeFnOvl(&NotOverloaded); //expected-error{{cannot take address of function 'NotOverloaded' because parameter 1 has pass_object_size attribute}} + + int P; + (&NotOverloaded)(&P); //expected-error{{cannot take address of function 'NotOverloaded' because parameter 1 has pass_object_size attribute}} + (&IsOverloaded)(&P); //expected-error{{no matching function}} expected-note@35{{candidate address cannot be taken because parameter 1 has pass_object_size attribute}} expected-note@36{{candidate function not viable: no known conversion from 'int *' to 'char *' for 1st argument}} } diff --git a/test/SemaCXX/pass-object-size.cpp b/test/SemaCXX/pass-object-size.cpp index bec0c1c55f..94c9cc9c8a 100644 --- a/test/SemaCXX/pass-object-size.cpp +++ b/test/SemaCXX/pass-object-size.cpp @@ -120,3 +120,17 @@ void Bar() { (void)+[](void *const p __attribute__((pass_object_size(0)))) {}; //expected-error-re{{invalid argument type '(lambda at {{.*}})' to unary expression}} } } + +namespace ovlbug { +// Directly calling an address-of function expression (e.g. in (&foo)(args...)) +// doesn't go through regular address-of-overload logic. This caused the above +// code to generate an ICE. +void DirectAddrOf(void *__attribute__((pass_object_size(0)))); +void DirectAddrOfOvl(void *__attribute__((pass_object_size(0)))); +void DirectAddrOfOvl(int *); + +void Test() { + (&DirectAddrOf)(nullptr); //expected-error{{cannot take address of function 'DirectAddrOf' because parameter 1 has pass_object_size attribute}} + (&DirectAddrOfOvl)((char*)nullptr); //expected-error{{no matching function}} expected-note@129{{candidate address cannot be taken because parameter 1 has pass_object_size attribute}} expected-note@130{{candidate function not viable: no known conversion from 'char *' to 'int *' for 1st argument}} +} +}