r264975 - [Sema] Fix PR27122: ICE with enable_if+ill-formed call.

George Burgess IV via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 30 17:16:25 PDT 2016


Author: gbiv
Date: Wed Mar 30 19:16:25 2016
New Revision: 264975

URL: http://llvm.org/viewvc/llvm-project?rev=264975&view=rev
Log:
[Sema] Fix PR27122: ICE with enable_if+ill-formed call.

In some cases, when we encounter a direct function call with an
incorrect number of arguments, we'll emit a diagnostic, and pretend that
the call to the function was valid. For example, in C:

int foo();
int a = foo(1);

Prior to this patch, we'd get an ICE if foo had an enable_if attribute,
because CheckEnableIf assumes that the number of arguments it gets
passed is valid for the function it's passed. Now, we check that the
number of args looks valid prior to checking enable_if conditions.

This fix was not done inside of CheckEnableIf because the problem
presently can only occur in one caller of CheckEnableIf (ActOnCallExpr).
Additionally, checking inside of CheckEnableIf would make us emit
multiple diagnostics for the same error (one "enable_if failed", one
"you gave this function the wrong number of arguments"), which seems
worse than just complaining about the latter.

Modified:
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/Sema/enable_if.c
    cfe/trunk/test/SemaCXX/enable_if.cpp

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=264975&r1=264974&r2=264975&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Mar 30 19:16:25 2016
@@ -5044,6 +5044,14 @@ static FunctionDecl *rewriteBuiltinFunct
   return OverloadDecl;
 }
 
+static bool isNumberOfArgsValidForCall(Sema &S, const FunctionDecl *Callee,
+                                       std::size_t NumArgs) {
+  if (S.TooManyArguments(Callee->getNumParams(), NumArgs,
+                         /*PartialOverloading=*/false))
+    return Callee->isVariadic();
+  return Callee->getMinRequiredArguments() <= NumArgs;
+}
+
 /// ActOnCallExpr - Handle a call to Fn with the specified array of arguments.
 /// This provides the location of the left/right parens and a list of comma
 /// locations.
@@ -5175,7 +5183,14 @@ Sema::ActOnCallExpr(Scope *S, Expr *Fn,
                                            Fn->getLocStart()))
       return ExprError();
 
-    if (FD->hasAttr<EnableIfAttr>()) {
+    // CheckEnableIf assumes that the we're passing in a sane number of args for
+    // FD, but that doesn't always hold true here. This is because, in some
+    // cases, we'll emit a diag about an ill-formed function call, but then
+    // we'll continue on as if the function call wasn't ill-formed. So, if the
+    // number of args looks incorrect, don't do enable_if checks; we should've
+    // already emitted an error about the bad call.
+    if (FD->hasAttr<EnableIfAttr>() &&
+        isNumberOfArgsValidForCall(*this, FD, ArgExprs.size())) {
       if (const EnableIfAttr *Attr = CheckEnableIf(FD, ArgExprs, true)) {
         Diag(Fn->getLocStart(),
              isa<CXXMethodDecl>(FD) ?

Modified: cfe/trunk/test/Sema/enable_if.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/enable_if.c?rev=264975&r1=264974&r2=264975&view=diff
==============================================================================
--- cfe/trunk/test/Sema/enable_if.c (original)
+++ cfe/trunk/test/Sema/enable_if.c Wed Mar 30 19:16:25 2016
@@ -142,4 +142,11 @@ void test8() {
   void (*p1)(int) = &f4; // expected-error{{cannot take address of function 'f4' becuase it has one or more non-tautological enable_if conditions}}
   void (*p2)(int) = f4; // expected-error{{cannot take address of function 'f4' becuase it has one or more non-tautological enable_if conditions}}
 }
+
+void regular_enable_if(int a) __attribute__((enable_if(a, ""))); // expected-note 3{{declared here}}
+void PR27122_ext() {
+  regular_enable_if(0, 2); // expected-error{{too many arguments}}
+  regular_enable_if(1, 2); // expected-error{{too many arguments}}
+  regular_enable_if(); // expected-error{{too few arguments}}
+}
 #endif

Modified: cfe/trunk/test/SemaCXX/enable_if.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enable_if.cpp?rev=264975&r1=264974&r2=264975&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/enable_if.cpp (original)
+++ cfe/trunk/test/SemaCXX/enable_if.cpp Wed Mar 30 19:16:25 2016
@@ -346,3 +346,43 @@ void testIt() {
   auto CRef = (NoMatchTy)&foo; // expected-error{{address of overloaded function 'foo' does not match required type 'void ()'}}
 }
 }
+
+namespace PR27122 {
+// (slightly reduced) code that motivated the bug...
+namespace ns {
+void Function(int num)
+  __attribute__((enable_if(num != 0, "")));
+void Function(int num, int a0)
+  __attribute__((enable_if(num != 1, "")));
+}  // namespace ns
+
+using ns::Function; // expected-note 3{{declared here}}
+void Run() {
+  Functioon(0); // expected-error{{use of undeclared identifier}} expected-error{{too few arguments}}
+  Functioon(0, 1); // expected-error{{use of undeclared identifier}}
+  Functioon(0, 1, 2); // expected-error{{use of undeclared identifier}}
+}
+
+// Extra tests
+void regularEnableIf(int a) __attribute__((enable_if(a, ""))); // expected-note 3{{declared here}} expected-note 3{{candidate function not viable}}
+void runRegularEnableIf() {
+  regularEnableIf(0, 2); // expected-error{{no matching function}}
+  regularEnableIf(1, 2); // expected-error{{no matching function}}
+  regularEnableIf(); // expected-error{{no matching function}}
+
+  // Test without getting overload resolution involved
+  ::PR27122::regularEnableIf(0, 2); // expected-error{{too many arguments}}
+  ::PR27122::regularEnableIf(1, 2); // expected-error{{too many arguments}}
+  ::PR27122::regularEnableIf(); // expected-error{{too few arguments}}
+}
+
+struct Foo {
+  void bar(int i) __attribute__((enable_if(i, ""))); // expected-note 2{{declared here}}
+};
+
+void runFoo() {
+  Foo f;
+  f.bar(); // expected-error{{too few arguments}}
+  f.bar(1, 2); // expected-error{{too many arguments}}
+}
+}




More information about the cfe-commits mailing list