george.burgess.iv created this revision.
george.burgess.iv added a reviewer: rsmith.
george.burgess.iv added a subscriber: cfe-commits.
"directly called indirectly" wasn't a typo; I can't think of a better name for
this pattern. :)
Currently, Sema treats `foo()` and `(&foo)()` both as direct calls of `foo`,
for the purposes of overload resolution. CodeGen treats the latter as an
indirect call. This allows one to write code that indirectly calls an
unaddressable function, which can result in ICEs (yay `pass_object_size`).
It seems that some code relies on how Sema resolves `(&foo)()`, so I don't
think we can simply ask `AddressOfFunctionResolver` to handle this for us.
http://reviews.llvm.org/D15590
Files:
include/clang/Sema/Overload.h
include/clang/Sema/Sema.h
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
test/CodeGenCXX/pass-object-size.cpp
test/Sema/pass-object-size.c
test/SemaCXX/pass-object-size.cpp
Index: test/SemaCXX/pass-object-size.cpp
===================================================================
--- test/SemaCXX/pass-object-size.cpp
+++ test/SemaCXX/pass-object-size.cpp
@@ -120,3 +120,17 @@
(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}}
+}
+}
Index: test/Sema/pass-object-size.c
===================================================================
--- test/Sema/pass-object-size.c
+++ test/Sema/pass-object-size.c
@@ -33,7 +33,7 @@
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 @@
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}}
}
Index: test/CodeGenCXX/pass-object-size.cpp
===================================================================
--- test/CodeGenCXX/pass-object-size.cpp
+++ test/CodeGenCXX/pass-object-size.cpp
@@ -25,3 +25,21 @@
// 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);
+}
+}
Index: lib/Sema/SemaOverload.cpp
===================================================================
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -9597,6 +9597,13 @@
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;
+ }
}
}
@@ -11197,6 +11204,17 @@
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
@@ -11209,15 +11227,21 @@
MultiExprArg Args,
SourceLocation RParenLoc,
Expr *ExecConfig,
- bool AllowTypoCorrection) {
+ bool AllowTypoCorrection,
+ bool CalleesAddressIsTaken) {
OverloadCandidateSet CandidateSet(Fn->getExprLoc(),
OverloadCandidateSet::CSK_Normal);
ExprResult result;
if (buildOverloadedCallSet(S, Fn, ULE, Args, LParenLoc, &CandidateSet,
&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);
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -4939,8 +4939,10 @@
OverloadExpr *ovl = find.Expression;
if (isa<UnresolvedLookupExpr>(ovl)) {
UnresolvedLookupExpr *ULE = cast<UnresolvedLookupExpr>(ovl);
- return BuildOverloadedCallExpr(S, Fn, ULE, LParenLoc, ArgExprs,
- RParenLoc, ExecConfig);
+ return BuildOverloadedCallExpr(
+ S, Fn, ULE, LParenLoc, ArgExprs, RParenLoc, ExecConfig,
+ /*AllowTypoCorrection=*/true,
+ /*CalleesAddressIsTaken=*/find.IsAddressOfOperand);
} else {
return BuildCallToMemberFunction(S, Fn, LParenLoc, ArgExprs,
RParenLoc);
@@ -4957,10 +4959,14 @@
Expr *NakedFn = Fn->IgnoreParens();
+ bool CallingNDeclIndirectly = false;
NamedDecl *NDecl = nullptr;
- if (UnaryOperator *UnOp = dyn_cast<UnaryOperator>(NakedFn))
- if (UnOp->getOpcode() == UO_AddrOf)
+ if (UnaryOperator *UnOp = dyn_cast<UnaryOperator>(NakedFn)) {
+ if (UnOp->getOpcode() == UO_AddrOf) {
+ CallingNDeclIndirectly = true;
NakedFn = UnOp->getSubExpr()->IgnoreParens();
+ }
+ }
if (isa<DeclRefExpr>(NakedFn)) {
NDecl = cast<DeclRefExpr>(NakedFn)->getDecl();
@@ -4982,6 +4988,11 @@
NDecl = cast<MemberExpr>(NakedFn)->getMemberDecl();
if (FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(NDecl)) {
+ if (CallingNDeclIndirectly &&
+ !checkAddressOfFunctionIsAvailable(FD, /*Complain=*/true,
+ Fn->getLocStart()))
+ return ExprError();
+
if (FD->hasAttr<EnableIfAttr>()) {
if (const EnableIfAttr *Attr = CheckEnableIf(FD, ArgExprs, true)) {
Diag(Fn->getLocStart(),
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -2546,7 +2546,8 @@
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,
Index: include/clang/Sema/Overload.h
===================================================================
--- include/clang/Sema/Overload.h
+++ include/clang/Sema/Overload.h
@@ -570,8 +570,8 @@
/// 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 @@
/// 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).
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
