public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
@ 2015-06-22  0:36 Martin Sebor
  2015-06-22 14:11 ` Joseph Myers
                   ` (2 more replies)
  0 siblings, 3 replies; 34+ messages in thread
From: Martin Sebor @ 2015-06-22  0:36 UTC (permalink / raw)
  To: Gcc Patch List

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

Attached is a patch to reject C and C++ constructs that result
in obtaining a pointer (or a reference in C++) to a builtin
function.  These constructs are currently silently accepted by
GCC and, in most cases(*), result in a linker error.  The patch
brings GCC on par with Clang by rejecting all such constructs.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Okay to commit to trunk?

Martin

[*] Besides rejecting constructs that lead to linker errors
the patch leads to rejecting also some benign constructs (that
are also rejected by Clang).  For example, the program below
currently compiles and links successfully with GCC 5.1 is
rejected with the patch applied.  That's intended (but I'm
open to arguments against it).

$ cat /build/tmp/u.cpp && /build/gcc-66516/gcc/xg++ 
-B/build/gcc-66516/gcc -Wall -c -std=c++11 /build/tmp/u.cpp
static constexpr int (&r)(int) = __builtin_ffs;

int main () { return r (0); }
/build/tmp/u.cpp:1:34: error: invalid initialization of a reference with 
a builtin function
  static constexpr int (&r)(int) = __builtin_ffs;
                                   ^

[-- Attachment #2: gcc-66516.patch --]
[-- Type: text/x-patch, Size: 11324 bytes --]

2015-06-21  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* c/c-typeck.c (default_function_array_conversion): Reject
	converting a builtin function to a pointer.
	(parser_build_unary_op): Reject taking the address of a builtin
	function.
	* cp/call.c (convert_like_real): Reject converting a builtin function
	to a pointer.
	(initialize_reference): Reject initializing a reference with a builtin
	function.
	* cp/typeck.c (cp_build_addr_expr_strict): Reject taking the address
	of a builtin function.
	(build_reinterpret_cast_1): Reject casting a builtin function to
	a pointer.
	(convert_for_initialization): Reject initializing a pointer with
	the a builtin function.

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 636e0bb..637a292 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -58,6 +58,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "cilk.h"
 #include "gomp-constants.h"

+#include <stdlib.h>
+
 /* Possible cases of implicit bad conversions.  Used to select
    diagnostic messages in convert_for_assignment.  */
 enum impl_conv {
@@ -1940,6 +1942,12 @@ default_function_array_conversion (location_t loc, struct c_expr exp)
       }
       break;
     case FUNCTION_TYPE:
+      if (DECL_IS_BUILTIN (exp.value))
+	{
+	  error_at (loc, "converting builtin function to a pointer");
+	  exp.value = error_mark_node;
+	}
+      else
 	exp.value = function_to_pointer_conversion (loc, exp.value);
       break;
     default:
@@ -3384,7 +3392,14 @@ parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg)
   result.original_code = code;
   result.original_type = NULL;

-  if (TREE_OVERFLOW_P (result.value) && !TREE_OVERFLOW_P (arg.value))
+  if (code == ADDR_EXPR
+      && TREE_CODE (TREE_TYPE (arg.value)) == FUNCTION_TYPE
+      && DECL_IS_BUILTIN (arg.value))
+    {
+      error_at (loc, "taking address of a builtin function");
+      result.value = error_mark_node;
+    }
+  else if (TREE_OVERFLOW_P (result.value) && !TREE_OVERFLOW_P (arg.value))
     overflow_warning (loc, result.value);

   return result;
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 2d90ed9..df4cc77 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6570,6 +6570,13 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
 	    expr = build_target_expr_with_type (expr, type, complain);
 	  }

+	if (TREE_CODE (TREE_TYPE (expr)) == FUNCTION_TYPE
+	    && DECL_P (expr) && DECL_IS_BUILTIN (expr))
+	  {
+	    error_at (input_location, "taking address of a builtin function");
+	    return error_mark_node;
+	  }
+
 	/* Take the address of the thing to which we will bind the
 	   reference.  */
 	expr = cp_build_addr_expr (expr, complain);
@@ -9768,8 +9775,18 @@ initialize_reference (tree type, tree expr,
     }

   if (conv->kind == ck_ref_bind)
+    {
+      if (TREE_CODE (TREE_TYPE (expr)) == FUNCTION_TYPE
+	  && DECL_P (expr) && DECL_IS_BUILTIN (expr))
+	{
+	  error_at (input_location, "invalid initialization of a reference "
+		    "with a builtin function");
+	  return error_mark_node;
+	}
+
       /* Perform the conversion.  */
       expr = convert_like (conv, expr, complain);
+    }
   else if (conv->kind == ck_ambig)
     /* We gave an error in build_user_type_conversion_1.  */
     expr = error_mark_node;
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 5b09b73..fbea052 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -5621,6 +5621,13 @@ cp_build_addr_expr (tree arg, tsubst_flags_t complain)
 static tree
 cp_build_addr_expr_strict (tree arg, tsubst_flags_t complain)
 {
+  if (TREE_CODE (TREE_TYPE (arg)) == FUNCTION_TYPE
+      && DECL_P (arg) && DECL_IS_BUILTIN (arg))
+    {
+      error_at (input_location, "taking address of a builtin function");
+      return error_mark_node;
+    }
+
   return cp_build_addr_expr_1 (arg, 1, complain);
 }

@@ -6860,6 +6867,14 @@ build_reinterpret_cast_1 (tree type, tree expr, bool c_cast_p,
 	  || VOID_TYPE_P (TREE_TYPE (type))))
     return convert_member_func_to_ptr (type, expr, complain);

+  if (TREE_CODE (intype) == FUNCTION_TYPE
+      && DECL_P (expr) && DECL_IS_BUILTIN (expr))
+    {
+      error_at (EXPR_LOC_OR_LOC (expr, input_location),
+		"taking address of a builtin function");
+      return error_mark_node;
+    }
+
   /* If the cast is not to a reference type, the lvalue-to-rvalue,
      array-to-pointer, and function-to-pointer conversions are
      performed.  */
@@ -8307,6 +8322,13 @@ convert_for_initialization (tree exp, tree type, tree rhs, int flags,
       || (TREE_CODE (rhs) == TREE_LIST && TREE_VALUE (rhs) == error_mark_node))
     return error_mark_node;

+  if (TREE_CODE (TREE_TYPE (rhs)) == FUNCTION_TYPE
+      && DECL_P (rhs) && DECL_IS_BUILTIN (rhs))
+    {
+      error_at (input_location, "taking address of a builtin function");
+      return error_mark_node;
+    }
+
   if ((TREE_CODE (TREE_TYPE (rhs)) == ARRAY_TYPE
        && TREE_CODE (type) != ARRAY_TYPE
        && (TREE_CODE (type) != REFERENCE_TYPE

2015-06-21  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* gcc.dg/addr_builtin-pr66516.c: New test.
	* g++.dg/addr_builtin-pr66516.C: New test.

diff --git a/gcc/testsuite/g++.dg/addr_builtin-pr66516.C b/gcc/testsuite/g++.dg/addr_builtin-pr66516.C
new file mode 100644
index 0000000..38f1a51
--- /dev/null
+++ b/gcc/testsuite/g++.dg/addr_builtin-pr66516.C
@@ -0,0 +1,70 @@
+/* { dg-do compile } */
+
+/* Verify that taking the address of a builtin function generates
+   a compilation error.  */
+
+#if 201103L <= __cplusplus
+
+bool func (void)
+{
+  /* Taking the address of builtin constants is valid.  */
+  return &__func__ == &__func__;             /* { dg-bogus "builtin" } */
+}
+
+#endif
+
+/* Calling the builtins is valid.  */
+int foo (int i)
+{
+  const void* p = 0;
+
+  switch (i & 0xf) {
+  case 0: i = __builtin_expect (i, 0);       /* { dg-bogus "builtin" } */
+  case 1: p = __builtin_return_address (0);  /* { dg-bogus "builtin" } */
+  case 2: i = __builtin_strlen ("");         /* { dg-bogus "builtin" } */
+  case 3: p = __builtin_FILE ();             /* { dg-bogus "builtin" } */
+  case 4: p = __builtin_FUNCTION ();         /* { dg-bogus "builtin" } */
+  case 5: i = __builtin_LINE ();             /* { dg-bogus "builtin" } */
+  }
+  return p ? *(int*)p : i;
+}
+
+void* const addr[] = {
+  (void*)__builtin_expect,          /* { dg-error "builtin" } */
+  (void*)__builtin_return_address,  /* { dg-error "builtin" } */
+  (void*)__atomic_load,             /* { dg-error "builtin" } */
+  (void*)__builtin_add_overflow,    /* { dg-error "builtin" } */
+  (void*)__builtin_constant_p,      /* { dg-error "builtin" } */
+  (void*)__builtin_nanf,            /* { dg-error "builtin" } */
+  (void*)__builtin_strlen,          /* { dg-error "builtin" } */
+  (void*)__builtin_unreachable,     /* { dg-error "builtin" } */
+  (void*)__builtin_FILE,            /* { dg-error "builtin" } */
+  (void*)__builtin_FUNCTION,        /* { dg-error "builtin" } */
+  (void*)__builtin_LINE,            /* { dg-error "builtin" } */
+
+  (void*)&__builtin_expect,         /* { dg-error "builtin" } */
+  (void*)&__builtin_return_address, /* { dg-error "builtin" } */
+  (void*)&__atomic_load,            /* { dg-error "builtin" } */
+  (void*)&__builtin_add_overflow,   /* { dg-error "builtin" } */
+  (void*)&__builtin_constant_p,     /* { dg-error "builtin" } */
+  (void*)&__builtin_nanf,           /* { dg-error "builtin" } */
+  (void*)&__builtin_strlen,         /* { dg-error "builtin" } */
+  (void*)&__builtin_unreachable,    /* { dg-error "builtin" } */
+  (void*)&__builtin_FILE,           /* { dg-error "builtin" } */
+  (void*)&__builtin_FUNCTION,       /* { dg-error "builtin" } */
+  (void*)&__builtin_LINE,           /* { dg-error "builtin" } */
+
+  reinterpret_cast<void*>(__builtin_expect),       /* { dg-error "builtin" } */
+  reinterpret_cast<void*>(__builtin_return_address), /* { dg-error "builtin" } */
+  reinterpret_cast<void*>(__atomic_load),          /* { dg-error "builtin" } */
+  reinterpret_cast<void*>(__builtin_add_overflow), /* { dg-error "builtin" } */
+  reinterpret_cast<void*>(__builtin_constant_p),   /* { dg-error "builtin" } */
+  reinterpret_cast<void*>(__builtin_nanf),         /* { dg-error "builtin" } */
+  reinterpret_cast<void*>(__builtin_strlen),       /* { dg-error "builtin" } */
+  reinterpret_cast<void*>(__builtin_unreachable),  /* { dg-error "builtin" } */
+  reinterpret_cast<void*>(__builtin_FILE),         /* { dg-error "builtin" } */
+  reinterpret_cast<void*>(__builtin_FUNCTION),     /* { dg-error "builtin" } */
+  reinterpret_cast<void*>(__builtin_LINE),         /* { dg-error "builtin" } */
+
+  0
+};
diff --git a/gcc/testsuite/gcc.dg/addr_builtin-pr66516.c b/gcc/testsuite/gcc.dg/addr_builtin-pr66516.c
new file mode 100644
index 0000000..b0b480e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/addr_builtin-pr66516.c
@@ -0,0 +1,59 @@
+/* { dg-do compile } */
+
+/* Verify that taking the address of a builtin function generates
+   a compilation error.  */
+
+#if 199901L <= __STDC_VERSION__
+
+int func (void)
+{
+  /* Taking the address of builtin constants is valid.  */
+  return &__func__ == &__func__;                /* { dg-bogus "builtin" } */
+}
+
+#endif
+
+
+/* Calling the builtins is valid.  */
+int foo (int i)
+{
+  const void* p = 0;
+
+  switch (i & 0xf) {
+  case 0: i = __builtin_expect (i, 0);       /* { dg-bogus "builtin" } */
+  case 1: p = __builtin_return_address (0);  /* { dg-bogus "builtin" } */
+  case 2: i = __builtin_strlen ("");         /* { dg-bogus "builtin" } */
+  case 3: p = __builtin_FILE ();             /* { dg-bogus "builtin" } */
+  case 4: p = __builtin_FUNCTION ();         /* { dg-bogus "builtin" } */
+  case 5: i = __builtin_LINE ();             /* { dg-bogus "builtin" } */
+  }
+  return p ? *(int*)p : i;
+}
+
+void* const addr[] = {
+  (void*)__builtin_expect,          /* { dg-error "builtin" } */
+  (void*)__builtin_return_address,  /* { dg-error "builtin" } */
+  (void*)__atomic_load,             /* { dg-error "builtin" } */
+  (void*)__builtin_add_overflow,    /* { dg-error "builtin" } */
+  (void*)__builtin_constant_p,      /* { dg-error "builtin" } */
+  (void*)__builtin_nanf,            /* { dg-error "builtin" } */
+  (void*)__builtin_strlen,          /* { dg-error "builtin" } */
+  (void*)__builtin_unreachable,     /* { dg-error "builtin" } */
+  (void*)__builtin_FILE,            /* { dg-error "builtin" } */
+  (void*)__builtin_FUNCTION,        /* { dg-error "builtin" } */
+  (void*)__builtin_LINE,            /* { dg-error "builtin" } */
+
+  (void*)&__builtin_expect,         /* { dg-error "builtin" } */
+  (void*)&__builtin_return_address, /* { dg-error "builtin" } */
+  (void*)&__atomic_load,            /* { dg-error "builtin" } */
+  (void*)&__builtin_add_overflow,   /* { dg-error "builtin" } */
+  (void*)&__builtin_constant_p,     /* { dg-error "builtin" } */
+  (void*)&__builtin_nanf,           /* { dg-error "builtin" } */
+  (void*)&__builtin_strlen,         /* { dg-error "builtin" } */
+  (void*)&__builtin_unreachable,    /* { dg-error "builtin" } */
+  (void*)&__builtin_FILE,           /* { dg-error "builtin" } */
+  (void*)&__builtin_FUNCTION,       /* { dg-error "builtin" } */
+  (void*)&__builtin_LINE,           /* { dg-error "builtin" } */
+
+  0
+};

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-06-22  0:36 [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function Martin Sebor
@ 2015-06-22 14:11 ` Joseph Myers
  2015-06-22 14:41 ` Marek Polacek
  2015-06-22 19:45 ` Marek Polacek
  2 siblings, 0 replies; 34+ messages in thread
From: Joseph Myers @ 2015-06-22 14:11 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Gcc Patch List

On Sun, 21 Jun 2015, Martin Sebor wrote:

> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 636e0bb..637a292 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -58,6 +58,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "cilk.h"
>  #include "gomp-constants.h"
> 
> +#include <stdlib.h>

Included from system.h, don't include it explicitly in source files.

> +      if (DECL_IS_BUILTIN (exp.value))
> +	{
> +	  error_at (loc, "converting builtin function to a pointer");

Say "built-in" (see codingconventions.html).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-06-22  0:36 [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function Martin Sebor
  2015-06-22 14:11 ` Joseph Myers
@ 2015-06-22 14:41 ` Marek Polacek
  2015-06-23  4:38   ` Martin Sebor
  2015-06-22 19:45 ` Marek Polacek
  2 siblings, 1 reply; 34+ messages in thread
From: Marek Polacek @ 2015-06-22 14:41 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Gcc Patch List

On Sun, Jun 21, 2015 at 05:05:14PM -0600, Martin Sebor wrote:
> Attached is a patch to reject C and C++ constructs that result
> in obtaining a pointer (or a reference in C++) to a builtin
> function.  These constructs are currently silently accepted by
> GCC and, in most cases(*), result in a linker error.  The patch
> brings GCC on par with Clang by rejecting all such constructs.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
 
It seems like this patch regresess pr59630.c testcase; I don't see
the testcase being addressed in this patch.

> 2015-06-21  Martin Sebor  <msebor@redhat.com>
> 
> 	PR c/66516
> 	* c/c-typeck.c (default_function_array_conversion): Reject
> 	converting a builtin function to a pointer.
> 	(parser_build_unary_op): Reject taking the address of a builtin
> 	function.
> 	* cp/call.c (convert_like_real): Reject converting a builtin function
> 	to a pointer.
> 	(initialize_reference): Reject initializing a reference with a builtin
> 	function.
> 	* cp/typeck.c (cp_build_addr_expr_strict): Reject taking the address
> 	of a builtin function.
> 	(build_reinterpret_cast_1): Reject casting a builtin function to
> 	a pointer.
> 	(convert_for_initialization): Reject initializing a pointer with
> 	the a builtin function.
 
Please no c/ and cp/ prefixes.

> +#include <stdlib.h>

As Joseph already pointed out, this is redundant.

> @@ -3384,7 +3392,14 @@ parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg)
>    result.original_code = code;
>    result.original_type = NULL;
> 
> -  if (TREE_OVERFLOW_P (result.value) && !TREE_OVERFLOW_P (arg.value))
> +  if (code == ADDR_EXPR
> +      && TREE_CODE (TREE_TYPE (arg.value)) == FUNCTION_TYPE
> +      && DECL_IS_BUILTIN (arg.value))
> +    {
> +      error_at (loc, "taking address of a builtin function");
> +      result.value = error_mark_node;
> +    }
> +  else if (TREE_OVERFLOW_P (result.value) && !TREE_OVERFLOW_P (arg.value))
>      overflow_warning (loc, result.value);

It seems like you can move the new hunk a bit above so that we don't call
build_unary_op in a case when taking the address of a built-in function.

Unfortunately, it doesn't seem possible to do this error in build_unary_op
or in function_to_pointer_conversion :(.

	Marek

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-06-22  0:36 [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function Martin Sebor
  2015-06-22 14:11 ` Joseph Myers
  2015-06-22 14:41 ` Marek Polacek
@ 2015-06-22 19:45 ` Marek Polacek
  2 siblings, 0 replies; 34+ messages in thread
From: Marek Polacek @ 2015-06-22 19:45 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Gcc Patch List

On Sun, Jun 21, 2015 at 05:05:14PM -0600, Martin Sebor wrote:
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/addr_builtin-pr66516.c
> @@ -0,0 +1,59 @@
> +/* { dg-do compile } */

One more nit: I think I'd prefer naming the test addr-builtin-1.c
and then putting /* PR c/66516 */ on the first line of the test.

	Marek

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-06-22 14:41 ` Marek Polacek
@ 2015-06-23  4:38   ` Martin Sebor
  2015-06-23 10:29     ` Marek Polacek
  0 siblings, 1 reply; 34+ messages in thread
From: Martin Sebor @ 2015-06-23  4:38 UTC (permalink / raw)
  To: Marek Polacek; +Cc: Gcc Patch List

> It seems like this patch regresess pr59630.c testcase; I don't see
> the testcase being addressed in this patch.

Thanks for the review and for pointing out this regression!
I missed it among all the C test suite failures (I see 157
of them in 24 distinct tests on x86_64.)

pr59630 is marked ice-on-valid-code even though the call via
the converted pointer is clearly invalid (UB). What's more
relevant, though, is that the test case is one of those that
(while they both compile and link with the unpatched GCC) are
not intended to compile with the patch (and don't compile with
Clang).

In this simple case, the call to __builtin_abs(0) is folded
into the constant 0, but in more involved cases GCC emits
a call to abs. It's not clear to me from the manual or from
the builtin tests I've seen whether this is by design or
an accident of the implementation

Is it intended that programs be able to take the address of
the builtins that correspond to libc functions and make calls
to the underlying libc functions via such pointers? (If so,
the patch will need some tweaking.)

>
> Please no c/ and cp/ prefixes.

Sure, let me fix that in the next patch once the question
above has been settled.

>
>> +#include <stdlib.h>
>
> As Joseph already pointed out, this is redundant.

Yes, that was an accidental vestige of some debugging code I had
added. I'll take it out.

>
>> @@ -3384,7 +3392,14 @@ parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg)
>>     result.original_code = code;
>>     result.original_type = NULL;
>>
>> -  if (TREE_OVERFLOW_P (result.value) && !TREE_OVERFLOW_P (arg.value))
>> +  if (code == ADDR_EXPR
>> +      && TREE_CODE (TREE_TYPE (arg.value)) == FUNCTION_TYPE
>> +      && DECL_IS_BUILTIN (arg.value))
>> +    {
>> +      error_at (loc, "taking address of a builtin function");
>> +      result.value = error_mark_node;
>> +    }
>> +  else if (TREE_OVERFLOW_P (result.value) && !TREE_OVERFLOW_P (arg.value))
>>       overflow_warning (loc, result.value);
>
> It seems like you can move the new hunk a bit above so that we don't call
> build_unary_op in a case when taking the address of a built-in function.

Yes, that should work.

>
> Unfortunately, it doesn't seem possible to do this error in build_unary_op
> or in function_to_pointer_conversion :(.

Right. I couldn't find a way to do it because it gets called
for function calls too.

> One more nit: I think I'd prefer naming the test addr-builtin-1.c
> and then putting /* PR c/66516 */ on the first line of the test.

Will do.

Martin

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-06-23  4:38   ` Martin Sebor
@ 2015-06-23 10:29     ` Marek Polacek
  2015-06-23 10:39       ` Jakub Jelinek
  0 siblings, 1 reply; 34+ messages in thread
From: Marek Polacek @ 2015-06-23 10:29 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Gcc Patch List

On Mon, Jun 22, 2015 at 07:59:20PM -0600, Martin Sebor wrote:
> >It seems like this patch regresess pr59630.c testcase; I don't see
> >the testcase being addressed in this patch.
> 
> Thanks for the review and for pointing out this regression!
> I missed it among all the C test suite failures (I see 157
> of them in 24 distinct tests on x86_64.)

You might want to use contrib/test_summary and then compare its
outputs.

> pr59630 is marked ice-on-valid-code even though the call via
> the converted pointer is clearly invalid (UB). What's more
> relevant, though, is that the test case is one of those that
> (while they both compile and link with the unpatched GCC) are
> not intended to compile with the patch (and don't compile with
> Clang).

Right, just turn dg-warning into dg-error.

> In this simple case, the call to __builtin_abs(0) is folded
> into the constant 0, but in more involved cases GCC emits
> a call to abs. It's not clear to me from the manual or from
> the builtin tests I've seen whether this is by design or
> an accident of the implementation
> 
> Is it intended that programs be able to take the address of
> the builtins that correspond to libc functions and make calls
> to the underlying libc functions via such pointers? (If so,
> the patch will need some tweaking.)

I don't think so, at least clang doesn't allow e.g.
size_t (*fp) (const char *) = __builtin_strlen;

	Marek

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-06-23 10:29     ` Marek Polacek
@ 2015-06-23 10:39       ` Jakub Jelinek
  2015-06-23 15:12         ` Martin Sebor
  0 siblings, 1 reply; 34+ messages in thread
From: Jakub Jelinek @ 2015-06-23 10:39 UTC (permalink / raw)
  To: Marek Polacek; +Cc: Martin Sebor, Gcc Patch List

On Tue, Jun 23, 2015 at 12:18:30PM +0200, Marek Polacek wrote:
> > Is it intended that programs be able to take the address of
> > the builtins that correspond to libc functions and make calls
> > to the underlying libc functions via such pointers? (If so,
> > the patch will need some tweaking.)
> 
> I don't think so, at least clang doesn't allow e.g.
> size_t (*fp) (const char *) = __builtin_strlen;

Well, clang is irrelevant here, __builtin_strlen etc. is a GNU
extension, so it matters what we decide about it.  As this used to work
for decades (if the builtin function has a libc fallback), suddenly
rejecting it could break various programs that e.g. just
#define strlen __builtin_strlen
or similar.  Can't we really reject it just for the functions
that don't have a unique fallback?

	Jakub

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-06-23 10:39       ` Jakub Jelinek
@ 2015-06-23 15:12         ` Martin Sebor
  2015-06-29  7:37           ` Martin Sebor
  0 siblings, 1 reply; 34+ messages in thread
From: Martin Sebor @ 2015-06-23 15:12 UTC (permalink / raw)
  To: Jakub Jelinek, Marek Polacek; +Cc: Martin Sebor, Gcc Patch List

On 06/23/2015 04:29 AM, Jakub Jelinek wrote:
> On Tue, Jun 23, 2015 at 12:18:30PM +0200, Marek Polacek wrote:
>>> Is it intended that programs be able to take the address of
>>> the builtins that correspond to libc functions and make calls
>>> to the underlying libc functions via such pointers? (If so,
>>> the patch will need some tweaking.)
>>
>> I don't think so, at least clang doesn't allow e.g.
>> size_t (*fp) (const char *) = __builtin_strlen;
>
> Well, clang is irrelevant here, __builtin_strlen etc. is a GNU
> extension, so it matters what we decide about it.  As this used to work
> for decades (if the builtin function has a libc fallback), suddenly
> rejecting it could break various programs that e.g. just
> #define strlen __builtin_strlen
> or similar.  Can't we really reject it just for the functions
> that don't have a unique fallback?

Let me look into it.

Martin

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-06-23 15:12         ` Martin Sebor
@ 2015-06-29  7:37           ` Martin Sebor
  2015-07-02 14:20             ` Joseph Myers
  0 siblings, 1 reply; 34+ messages in thread
From: Martin Sebor @ 2015-06-29  7:37 UTC (permalink / raw)
  To: Jakub Jelinek, Marek Polacek; +Cc: Gcc Patch List, Joseph S. Myers

[-- Attachment #1: Type: text/plain, Size: 359 bytes --]

Attached is a rewrite of the patch to enforce that GCC builtin
functions with no library equivalents are only used to make
calls or cast to void (or in sizeof and _Alignof expressions
as a GCC extension). This version of the patch also addresses
the requests made in responses to the first patch.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Martin

[-- Attachment #2: gcc-66516.patch --]
[-- Type: text/x-patch, Size: 30553 bytes --]

2015-06-28  Martin Sebor  <msebor@redhat.com>

	pr c/66516
	* tree.h (DECL_IS_GCC_BUILTIN): New macro.
	* doc/extend.texi (Other Builtins): Document when the address of
	a builtin function can be taken.

2015-06-28  Martin Sebor  <msebor@redhat.com>

	pr c/66516
	* c-tree.h (c_validate_addressable): New function.
	* c-typeck.c (convert_arguments, parser_build_unary_op): Call it.
	(build_conditional_expr, c_cast_expr, convert_for_assignment): Same.
	(build_binary_op, c_objc_common_truthvalue_conversion): Same.
	(c_validate_addressable): Define function.

2015-06-28  Martin Sebor  <msebor@redhat.com>

	pr c/66516
	* call.c (build_conditional_expr_1): Call c_validate_addressable.
	(convert_arg_to_ellipsis, convert_for_arg_passing): Same.
	* cp-tree.h (cp_validate_addressable): New function.
	* pt.c (convert_template_argument): Call it.
	* typeck.c (cp_build_binary_op, cp_build_addr_expr_strict): Same.
	(cp_build_unary_op, build_static_cast_1, build_reinterpret_cast_1):
	Same.
	(cp_build_c_cast, convert_for_assignment, convert_for_initialization):
	Same.
	(cp_validate_addressable): Define function.

2015-06-28  Martin Sebor  <msebor@redhat.com>

	pr c/66516
	* g++.dg/addr_builtin-1.C: New test.
	* gcc.dg/addr_builtin-1.c: New test.
	* gcc.dg/lto/pr54702_1.c: Add a missing include directive.

diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 28b58c6..4219129 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -655,6 +655,7 @@ extern tree c_finish_transaction (location_t, tree, int);
 extern bool c_tree_equal (tree, tree);
 extern tree c_build_function_call_vec (location_t, vec<location_t>, tree,
 				       vec<tree, va_gc> *, vec<tree, va_gc> *);
+extern bool c_validate_addressable (const_tree, location_t = UNKNOWN_LOCATION);

 /* Set to 0 at beginning of a function definition, set to 1 if
    a return statement that specifies a return value is seen.  */
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 8e2696a..5fd3669 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -3340,6 +3340,10 @@ convert_arguments (location_t loc, vec<location_t> arg_loc, tree typelist,
 	  error (invalid_func_diag);
 	  return -1;
 	}
+      else if (TREE_CODE (val) == ADDR_EXPR && !c_validate_addressable (val))
+	{
+	  return -1;
+	}
       else
 	/* Convert `short' and `char' to full-size `int'.  */
 	parmval = default_conversion (val);
@@ -3376,13 +3380,18 @@ struct c_expr
 parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg)
 {
   struct c_expr result;
-
-  result.value = build_unary_op (loc, code, arg.value, 0);
   result.original_code = code;
   result.original_type = NULL;

+  if (c_validate_addressable (arg.value))
+    {
+      result.value = build_unary_op (loc, code, arg.value, 0);
+
       if (TREE_OVERFLOW_P (result.value) && !TREE_OVERFLOW_P (arg.value))
 	overflow_warning (loc, result.value);
+    }
+  else
+      result.value = error_mark_node;

   return result;
 }
@@ -4477,11 +4486,22 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
       || TREE_CODE (TREE_TYPE (op2)) == ERROR_MARK)
     return error_mark_node;

+  if (TREE_CODE (TREE_TYPE (ifexp)) == POINTER_TYPE
+      && !c_validate_addressable (ifexp,
+				  EXPR_LOCATION (TREE_OPERAND (ifexp, 0))))
+    return error_mark_node;
+
   type1 = TREE_TYPE (op1);
   code1 = TREE_CODE (type1);
   type2 = TREE_TYPE (op2);
   code2 = TREE_CODE (type2);

+  if (code1 == POINTER_TYPE && !c_validate_addressable (op1))
+    return error_mark_node;
+
+  if (code2 == POINTER_TYPE && !c_validate_addressable (op2))
+    return error_mark_node;
+
   /* C90 does not permit non-lvalue arrays in conditional expressions.
      In C99 they will be pointers by now.  */
   if (code1 == ARRAY_TYPE || code2 == ARRAY_TYPE)
@@ -5220,6 +5240,10 @@ c_cast_expr (location_t loc, struct c_type_name *type_name, tree expr)
   type = groktypename (type_name, &type_expr, &type_expr_const);
   warn_strict_prototypes = saved_wsp;

+  if (TREE_CODE (expr) == ADDR_EXPR && !VOID_TYPE_P (type)
+      && !c_validate_addressable (expr))
+    return error_mark_node;
+
   ret = build_c_cast (loc, type, expr);
   if (type_expr)
     {
@@ -5859,6 +5883,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
   rhs = require_complete_type (rhs);
   if (rhs == error_mark_node)
     return error_mark_node;
+
+  if (coder == POINTER_TYPE && !c_validate_addressable (rhs))
+    return error_mark_node;
+
   /* A non-reference type can convert to a reference.  This handles
      va_start, va_copy and possibly port built-ins.  */
   if (codel == REFERENCE_TYPE && coder != REFERENCE_TYPE)
@@ -10336,6 +10364,14 @@ build_binary_op (location_t location, enum tree_code code,
   if (code0 == ERROR_MARK || code1 == ERROR_MARK)
     return error_mark_node;

+  if (code0 == POINTER_TYPE
+      && !c_validate_addressable (op0, EXPR_LOCATION (orig_op0)))
+    return error_mark_node;
+
+  if (code1 == POINTER_TYPE
+      && !c_validate_addressable (op1, EXPR_LOCATION (orig_op1)))
+    return error_mark_node;
+
   if ((invalid_op_diag
        = targetm.invalid_binary_op (code, type0, type1)))
     {
@@ -11313,6 +11349,11 @@ c_objc_common_truthvalue_conversion (location_t location, tree expr)
       error_at (location, "void value not ignored as it ought to be");
       return error_mark_node;

+    case POINTER_TYPE:
+      if (!c_validate_addressable (expr))
+	return error_mark_node;
+      break;
+
     case FUNCTION_TYPE:
       gcc_unreachable ();

@@ -12865,3 +12906,33 @@ cilk_install_body_with_frame_cleanup (tree fndecl, tree body, void *w)
   append_to_statement_list (build_stmt (EXPR_LOCATION (body), TRY_FINALLY_EXPR,
 				       	body_list, dtor), &list);
 }
+
+/* For EXPR that is an ADDR_EXPR or whose type is a FUNCTION_TYPE,
+   determines whether its operand can have its address taken issues
+   an error pointing to the location LOC.
+   Operands that cannot have their address taken are builtin functions
+   that have no library fallback (no other kinds of expressions are
+   considered).
+   Returns true when the expression can have its address taken and
+   false otherwise.  */
+bool
+c_validate_addressable (const_tree expr, location_t loc /* = UNKNOWN_LOCATION */)
+{
+  if (TREE_CODE (expr) == ADDR_EXPR)
+    expr = TREE_OPERAND (expr, 0);
+
+  if (TREE_TYPE (expr)
+      && TREE_CODE (TREE_TYPE (expr)) == FUNCTION_TYPE
+      && DECL_P (expr) && DECL_IS_GCC_BUILTIN (expr))
+    {
+      if (loc == UNKNOWN_LOCATION)
+	loc = EXPR_LOC_OR_LOC (expr, input_location);
+      /* Reject arguments that are builtin functions with
+	 no library fallback.  */
+      error_at (loc, "builtin functions must be directly called");
+
+      return false;
+    }
+
+  return true;
+}
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index b846919..9bbde27 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -4662,6 +4662,15 @@ build_conditional_expr_1 (location_t loc, tree arg1, tree arg2, tree arg3,
       return fold_build3 (VEC_COND_EXPR, arg2_type, arg1, arg2, arg3);
     }

+  if (TREE_CODE (arg1) == FUNCTION_DECL && !cp_validate_addressable (arg1))
+    return error_mark_node;
+
+  if (TREE_CODE (arg2) == FUNCTION_DECL && !cp_validate_addressable (arg2))
+    return error_mark_node;
+
+  if (TREE_CODE (arg3) == FUNCTION_DECL && !cp_validate_addressable (arg3))
+    return error_mark_node;
+
   /* [expr.cond]

      The first expression is implicitly converted to bool (clause
@@ -6603,12 +6612,17 @@ convert_arg_to_ellipsis (tree arg, tsubst_flags_t complain)
   tree arg_type;
   location_t loc = EXPR_LOC_OR_LOC (arg, input_location);

+  if (TREE_CODE (TREE_TYPE (arg)) == FUNCTION_TYPE
+      && !cp_validate_addressable (arg))
+      return error_mark_node;
+
   /* [expr.call]

      The lvalue-to-rvalue, array-to-pointer, and function-to-pointer
      standard conversions are performed.  */
   arg = decay_conversion (arg, complain);
   arg_type = TREE_TYPE (arg);
+
   /* [expr.call]

      If the argument has integral or enumeration type that is subject
@@ -6884,6 +6898,10 @@ convert_for_arg_passing (tree type, tree val, tsubst_flags_t complain)
 	   && COMPLETE_TYPE_P (type)
 	   && tree_int_cst_lt (TYPE_SIZE (type), TYPE_SIZE (integer_type_node)))
     val = cp_perform_integral_promotions (val, complain);
+  else if (TREE_CODE (val) == ADDR_EXPR
+	   && !cp_validate_addressable (val, complain))
+    return error_mark_node;
+
   if ((complain & tf_warning)
       && warn_suggest_attribute_format)
     {
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index e8cc38f..98e9a63 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6298,6 +6298,9 @@ extern bool check_raw_literal_operator		(const_tree decl);
 extern bool check_literal_operator_args		(const_tree, bool *, bool *);
 extern void maybe_warn_about_useless_cast       (tree, tree, tsubst_flags_t);
 extern tree cp_perform_integral_promotions      (tree, tsubst_flags_t);
+extern bool cp_validate_addressable             (const_tree,
+                                                 tsubst_flags_t = tf_error,
+                                                 location_t = UNKNOWN_LOCATION);

 /* in typeck2.c */
 extern void require_complete_eh_spec_types	(tree, tree);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 874f29f..3f9f49f 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -6827,6 +6827,26 @@ convert_template_argument (tree parm,
       else if (val == error_mark_node && (complain & tf_error))
 	error ("could not convert template argument %qE to %qT",  orig_arg, t);

+      if (TREE_CODE (val) == ADDR_EXPR
+          && !cp_validate_addressable (TREE_OPERAND (val, 0), complain))
+        {
+          /* Reject template arguments that are pointers to builtin
+             functions with no library fallbacks.  */
+          return error_mark_node;
+        }
+
+      if (INDIRECT_REF_P (val))
+        {
+          /* Reject template arguments that are references to builtin
+             functions with no library fallbacks.  */
+          const_tree inner = TREE_OPERAND (val, 0);
+          if (TREE_CODE (TREE_TYPE (inner)) == REFERENCE_TYPE
+              && TREE_CODE (TREE_TYPE (TREE_TYPE (inner))) == FUNCTION_TYPE
+              && TREE_CODE (TREE_TYPE (inner)) == REFERENCE_TYPE
+              && !cp_validate_addressable (TREE_OPERAND (inner, 0), complain))
+              return error_mark_node;
+        }
+
       if (TREE_CODE (val) == SCOPE_REF)
 	{
 	  /* Strip typedefs from the SCOPE_REF.  */
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 2d03d75..02aab3d 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4021,6 +4021,14 @@ cp_build_binary_op (location_t location,
   if (code0 == ERROR_MARK || code1 == ERROR_MARK)
     return error_mark_node;

+  if (code0 == POINTER_TYPE
+      && !cp_validate_addressable (op0, complain, EXPR_LOCATION (orig_op0)))
+    return error_mark_node;
+
+  if (code1 == POINTER_TYPE
+      && !cp_validate_addressable (op1, complain, EXPR_LOCATION (orig_op1)))
+    return error_mark_node;
+
   if ((invalid_op_diag
        = targetm.invalid_binary_op (code, type0, type1)))
     {
@@ -5651,6 +5659,9 @@ cp_build_addr_expr (tree arg, tsubst_flags_t complain)
 static tree
 cp_build_addr_expr_strict (tree arg, tsubst_flags_t complain)
 {
+  if (!cp_validate_addressable (arg, complain))
+      return error_mark_node;
+
   return cp_build_addr_expr_1 (arg, 1, complain);
 }

@@ -5688,6 +5699,9 @@ cp_build_unary_op (enum tree_code code, tree xarg, int noconvert,
       return error_mark_node;
     }

+  if (!cp_validate_addressable (arg, complain))
+    return error_mark_node;
+
   switch (code)
     {
     case UNARY_PLUS_EXPR:
@@ -6596,6 +6610,10 @@ build_static_cast_1 (tree type, tree expr, bool c_cast_p,
   if (abstract_virtuals_error_sfinae (ACU_CAST, type, complain))
     return error_mark_node;

+  if (TREE_CODE (intype) == FUNCTION_TYPE
+      && !cp_validate_addressable (expr, complain))
+      return error_mark_node;
+
   /* [expr.static.cast]

      An expression e can be explicitly converted to a type T using a
@@ -6863,6 +6881,9 @@ build_reinterpret_cast_1 (tree type, tree expr, bool c_cast_p,
 	warning (0, "casting %qT to %qT does not dereference pointer",
 		 intype, type);

+      if (!cp_validate_addressable (expr, complain))
+	  return error_mark_node;
+
       expr = cp_build_addr_expr (expr, complain);

       if (warn_strict_aliasing > 2)
@@ -6890,6 +6911,10 @@ build_reinterpret_cast_1 (tree type, tree expr, bool c_cast_p,
 	  || VOID_TYPE_P (TREE_TYPE (type))))
     return convert_member_func_to_ptr (type, expr, complain);

+  if (TREE_CODE (intype) == FUNCTION_TYPE
+      && !cp_validate_addressable (expr, complain, input_location))
+      return error_mark_node;
+
   /* If the cast is not to a reference type, the lvalue-to-rvalue,
      array-to-pointer, and function-to-pointer conversions are
      performed.  */
@@ -7271,6 +7296,10 @@ cp_build_c_cast (tree type, tree expr, tsubst_flags_t complain)
     warning_at (input_location, OPT_Wint_to_pointer_cast,
 		"cast to pointer from integer of different size");

+  if (FUNCTION_POINTER_TYPE_P (type)
+      && !cp_validate_addressable (value, complain))
+    return error_mark_node;
+
   /* A C-style cast can be a const_cast.  */
   result = build_const_cast_1 (type, value, complain & tf_warning,
 			       &valid_p);
@@ -8135,6 +8164,9 @@ convert_for_assignment (tree type, tree rhs,
       return error_mark_node;
     }

+  if (coder == FUNCTION_TYPE && !cp_validate_addressable (rhs, complain))
+      return error_mark_node;
+
   if (c_dialect_objc ())
     {
       int parmno;
@@ -8337,6 +8369,10 @@ convert_for_initialization (tree exp, tree type, tree rhs, int flags,
       || (TREE_CODE (rhs) == TREE_LIST && TREE_VALUE (rhs) == error_mark_node))
     return error_mark_node;

+  if (TREE_CODE (TREE_TYPE (rhs)) == FUNCTION_TYPE
+      && !cp_validate_addressable (rhs, complain, input_location))
+      return error_mark_node;
+
   if ((TREE_CODE (TREE_TYPE (rhs)) == ARRAY_TYPE
        && TREE_CODE (type) != ARRAY_TYPE
        && (TREE_CODE (type) != REFERENCE_TYPE
@@ -9358,3 +9394,40 @@ check_literal_operator_args (const_tree decl,
       return true;
     }
 }
+
+/* For EXPR that is an ADDR_EXPR or whose type is a FUNCTION_TYPE,
+   determines whether its operand can have its address taken and,
+   when COMPLAIN & tf_error is non-zero, issues an error pointing
+   to the location LOC.
+   Operands that cannot have their address taken are builtin functions
+   that have no library fallback (no other kinds of expressions are
+   considered).
+   Returns true when the expression can have its address taken and
+   false otherwise.  */
+bool
+cp_validate_addressable (const_tree expr, tsubst_flags_t complain,
+			 location_t loc /* = UNKNOWN_LOCATION */)
+{
+  if (TREE_CODE (expr) == ADDR_EXPR)
+    expr = TREE_OPERAND (expr, 0);
+
+  if (TREE_TYPE (expr)
+      && TREE_CODE (TREE_TYPE (expr)) == FUNCTION_TYPE
+      && DECL_P (expr) && DECL_IS_GCC_BUILTIN (expr)
+      && !DECL_IS_OPERATOR_NEW (expr)
+      && (!DECL_NAME (expr)
+	  || !NEW_DELETE_OPNAME_P (DECL_NAME (expr))))
+    {
+      if (complain & tf_error) {
+	if (loc == UNKNOWN_LOCATION)
+	  loc = EXPR_LOC_OR_LOC (expr, input_location);
+	/* Reject arguments that are builtin functions with
+	   no library fallback.  */
+	error_at (loc, "builtin functions must be directly called");
+      }
+
+      return false;
+    }
+
+  return true;
+}
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 8258000..0473797 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -10080,14 +10080,22 @@ recommend general use of these functions.

 The remaining functions are provided for optimization purposes.

+With the exception of built-ins that have library equivalents such as
+the standard C library functions discussed below, or that expand to
+library calls, GCC built-in functions are always expanded inline and
+thus do not have corresponding entry points and their address cannot
+be obtained.  Attempting to use them in an expression other than
+a function call results in a compile-time error.
+
 @opindex fno-builtin
 GCC includes built-in versions of many of the functions in the standard
-C library.  The versions prefixed with @code{__builtin_} are always
-treated as having the same meaning as the C library function even if you
-specify the @option{-fno-builtin} option.  (@pxref{C Dialect Options})
-Many of these functions are only optimized in certain cases; if they are
-not optimized in a particular case, a call to the library function is
-emitted.
+C library.  These functions come in two forms: one whose names are prefixed
+with the @code{__builtin_} prefix, and the other without.  Both forms have
+the same type (including prototype), the same address (when their address is
+taken), and the same meaning as the C library functions even if you specify
+the @option{-fno-builtin} option @pxref{C Dialect Options}).  Many of these
+functions are only optimized in certain cases; if they are not optimized in
+particular case, a call to the library function is emitted.

 @opindex ansi
 @opindex std
diff --git a/gcc/testsuite/g++.dg/addr_builtin-1.C b/gcc/testsuite/g++.dg/addr_builtin-1.C
new file mode 100644
index 0000000..5e78b35
--- /dev/null
+++ b/gcc/testsuite/g++.dg/addr_builtin-1.C
@@ -0,0 +1,188 @@
+/* PR66516 - missing diagnostic on taking the address of a builtin function */
+/* { dg-do compile } */
+/* { dg-options "-Wno-error=pedantic" } */
+
+typedef void (F)();
+typedef __UINTPTR_TYPE__ uintptr_t;
+
+// Utility function to test passing builtin functions as an ordinary
+// argument and via the ellipsis.
+static void func_arg (F*, ...);
+
+// Utility templates to test specializing templates on pointers and
+// references to builtin functions.
+template <F*> struct TestPointer { };
+template <F&> struct TestReference { };
+
+// Utility function with which, along with the builtin function,
+// to instantiate the C98 multi-parameter or C11 variadic tempates
+// below.
+void f () { }
+
+#if 201103 <= __cplusplus
+
+template <F*...> struct TestPointers { };
+template <F&...> struct TestReferences { };
+
+#else
+
+template <F* = &f, F* = &f> struct TestPointers { };
+template <F& = f, F& = f> struct TestReferences { };
+
+#endif
+
+static F* test_taking_address_of_gcc_builtin ()
+{
+  enum UINTPTR_E { e = ~(uintptr_t)0 };
+
+  F *p;
+  void *q;
+  uintptr_t a;
+
+  __builtin_trap ();                 // { dg-bogus "builtin" }
+  (void)__builtin_trap;              // { dg-bogus "builtin" }
+  __builtin_trap;                    // { dg-bogus "builtin" }
+
+  // Address operator.
+  p = &__builtin_trap;               // { dg-error "builtin" }
+
+  // Unary NOT.
+  a = !__builtin_trap;               // { dg-error "builtin" }
+
+  // Casts.
+  p = (F*)__builtin_trap;            // { dg-error "builtin" }
+
+  p = &(F&)__builtin_trap;            // { dg-error "builtin" }
+
+  p = &reinterpret_cast<F&>(__builtin_trap);   // { dg-error "builtin" }
+  p = &static_cast<F&>(__builtin_trap);        // { dg-error "builtin" }
+
+  p = reinterpret_cast<F*>(__builtin_trap);    // { dg-error "builtin" }
+  p = static_cast<F*>(__builtin_trap);         // { dg-error "builtin" }
+
+  a = reinterpret_cast<uintptr_t>(__builtin_trap);  // { dg-error "builtin" }
+  a = static_cast<uintptr_t>(__builtin_trap);       // { dg-error "builtin" }
+
+  a = reinterpret_cast<UINTPTR_E>(__builtin_trap);  // { dg-error "builtin" }
+  a = static_cast<UINTPTR_E>(__builtin_trap);       // { dg-error "builtin" }
+
+  // Additive operator.  Ill-formed but allowed with -fpermissive.
+  p = __builtin_trap + 0;            // { dg-error "builtin" }
+  p = __builtin_trap - 0;            // { dg-error "builtin" }
+  a = __builtin_trap - p;            // { dg-error "builtin" }
+  a = p - __builtin_trap;            // { dg-error "builtin" }
+
+  // Relational operators.  Ill-formed but allowed with -fpermissive.
+  a = __builtin_trap < p;            // { dg-error "builtin" }
+  a = p < __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap > p;            // { dg-error "builtin" }
+  a = p > __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap > p;            // { dg-error "builtin" }
+  a = p > __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  // Equality operators.
+  a = __builtin_trap == p;           // { dg-error "builtin" }
+  a = p == __builtin_trap;           // { dg-error "builtin" }
+  a = __builtin_trap != p;           // { dg-error "builtin" }
+  a = p != __builtin_trap;           // { dg-error "builtin" }
+
+  // Logical AND and OR.
+  a = __builtin_trap && p;           // { dg-error "builtin" }
+  a = p && __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap || p;           // { dg-error "builtin" }
+  a = p || __builtin_trap;           // { dg-error "builtin" }
+
+  // Conditional operator.
+  a = __builtin_trap ? 1 : 0;        // { dg-error "builtin" }
+  p = a ? __builtin_trap : 0;        // { dg-error "builtin" }
+  p = a ? 0 : __builtin_trap;        // { dg-error "builtin" }
+
+  // Assignment operator.
+  p = __builtin_trap;                // { dg-error "builtin" }
+
+  // Passing as an argument.
+  func_arg (__builtin_trap);         // { dg-error "builtin" }
+
+  // Passing through ellipsis.
+  func_arg (0, __builtin_trap);      // { dg-error "builtin" }
+
+  {
+    // Template specialization.
+    TestPointer<__builtin_trap> tp;         // { dg-error "builtin" }
+    TestReference<__builtin_trap> tr;       // { dg-error "builtin" }
+
+    TestPointers<__builtin_trap> tp1;       // { dg-error "builtin" }
+    TestReferences<__builtin_trap> tr1;     // { dg-error "builtin" }
+
+    TestPointers<f, __builtin_trap> tp2;    // { dg-error "builtin" }
+    TestReferences<f, __builtin_trap> tr2;  // { dg-error "builtin" }
+
+    TestPointers<__builtin_trap, f> tp3;    // { dg-error "builtin" }
+    TestReferences<__builtin_trap, f> tr3;  // { dg-error "builtin" }
+  }
+
+  return __builtin_trap;                    // { dg-error "builtin" }
+
+  (void)a;
+  (void)p;
+  (void)q;
+}
+
+// Operators new and delete are treated as GCC builtins with no library
+// fallbacks yet they exist in the runtime library and programs must be
+// able to take their address.
+void test_taking_address_of_op_new_and_delete ()
+{
+  typedef __SIZE_TYPE__ size_t;
+
+  typedef void* (OpNew) (size_t);
+  typedef void (OpDelete) (void*);
+
+  OpNew &newr = operator new;           // { dg-bogus "builtin" }
+  OpNew &newra = operator new[];        // { dg-bogus "builtin" }
+  OpNew *newp = &operator new;          // { dg-bogus "builtin" }
+  newp = &operator new[];               // { dg-bogus "builtin" }
+
+  OpDelete &delr = operator delete;     // { dg-bogus "builtin" }
+  OpDelete &delra = operator delete[];  // { dg-bogus "builtin" }
+  OpDelete *delp = &operator delete;    // { dg-bogus "builtin" }
+  delp = &operator delete[];            // { dg-bogus "builtin" }
+
+  (void)newr;
+  (void)newp;
+  (void)delr;
+  (void)delp;
+}
+
+// Creating a reference to or taking the address of a builtin with
+// a library "fallback" must be allowed.
+void test_taking_address_of_library_builtin ()
+{
+  {
+    typedef int F (int);
+    F &r = __builtin_abs;               // { dg-bogus "builtin" }
+    F *p = &__builtin_abs;              // { dg-bogus "builtin" }
+    (void)p;
+    (void)r;
+  }
+  {
+    typedef __SIZE_TYPE__ size_t;
+    typedef size_t F (const char*);
+    F &r = __builtin_strlen;            // { dg-bogus "builtin" }
+    F *p = &__builtin_strlen;           // { dg-bogus "builtin" }
+    (void)p;
+    (void)p;
+  }
+}
diff --git a/gcc/testsuite/gcc.dg/addr_builtin-1.c b/gcc/testsuite/gcc.dg/addr_builtin-1.c
new file mode 100644
index 0000000..1dbd871
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/addr_builtin-1.c
@@ -0,0 +1,127 @@
+/* PR66516 - missing diagnostic on taking the address of a builtin function */
+/* { dg-do compile } */
+/* { dg-options "-Wno-error=pedantic" } */
+
+typedef void (F)(void);
+typedef __UINTPTR_TYPE__ uintptr_t;
+
+// Utility function to test passing builtin functions as an ordinary
+// argument and via the ellipsis.
+static void func_arg (F *p, ...) { (void)p; }
+
+static F* test_taking_address_of_gcc_builtin (void)
+{
+    enum UINTPTR_E { e = ~(uintptr_t)0 };
+
+    F *p;
+    void *q;
+    uintptr_t a;
+
+    // Call, cast to void, and id are allowed.
+    __builtin_trap ();                 // { dg-bogus "builtin" }
+    (*__builtin_trap)();               // { dg-bogus "builtin" }
+    (void)__builtin_trap;              // { dg-bogus "builtin" }
+    __builtin_trap;                    // { dg-bogus "builtin" }
+
+    // Address and indirection operators.
+    p = &__builtin_trap;               // { dg-error "builtin" }
+    p = *__builtin_trap;               // { dg-error "builtin" }
+    p = &*__builtin_trap;              // { dg-error "builtin" }
+
+    // Unary NOT.
+    a = !__builtin_trap;               // { dg-error "builtin" }
+
+    // Sizeof and _Alignof are disallowed by C but allowed by GCC.
+    a = sizeof __builtin_trap;         // { dg-bogus "builtin" }
+    a = _Alignof __builtin_trap;       // { dg-bogus "builtin" }
+
+    // Casts.
+    p = (F*)__builtin_trap;            // { dg-error "builtin" }
+    a = (uintptr_t)__builtin_trap;     // { dg-error "builtin" }
+
+    // Additive operator.
+    p = __builtin_trap + 0;            // { dg-error "builtin" }
+    p = __builtin_trap - 0;            // { dg-error "builtin" }
+    a = __builtin_trap - p;            // { dg-error "builtin" }
+    a = p - __builtin_trap;            // { dg-error "builtin" }
+
+    // Relational operators.
+    a = __builtin_trap < p;            // { dg-error "builtin" }
+    a = p < __builtin_trap;            // { dg-error "builtin" }
+
+    a = __builtin_trap <= p;           // { dg-error "builtin" }
+    a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+    a = __builtin_trap > p;            // { dg-error "builtin" }
+    a = p > __builtin_trap;            // { dg-error "builtin" }
+
+    a = __builtin_trap > p;            // { dg-error "builtin" }
+    a = p > __builtin_trap;            // { dg-error "builtin" }
+
+    a = __builtin_trap <= p;           // { dg-error "builtin" }
+    a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+    a = __builtin_trap <= p;           // { dg-error "builtin" }
+    a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+    // Equality operators.
+    a = __builtin_trap == p;           // { dg-error "builtin" }
+    a = p == __builtin_trap;           // { dg-error "builtin" }
+    a = __builtin_trap != p;           // { dg-error "builtin" }
+    a = p != __builtin_trap;           // { dg-error "builtin" }
+
+    // Logical AND and OR.
+    a = __builtin_trap && p;           // { dg-error "builtin" }
+    a = p && __builtin_trap;           // { dg-error "builtin" }
+
+    a = __builtin_trap || p;           // { dg-error "builtin" }
+    a = p || __builtin_trap;           // { dg-error "builtin" }
+
+    // Conditional operator.
+    a = __builtin_trap ? 1 : 0;        // { dg-error "builtin" }
+    p = a ? __builtin_trap : 0;        // { dg-error "builtin" }
+    p = a ? 0 : __builtin_trap;        // { dg-error "builtin" }
+
+    // Assignment operator.
+    p = __builtin_trap;                // { dg-error "builtin" }
+
+    q = __builtin_trap;                // { dg-error "builtin" }
+    a = __builtin_trap;                // { dg-error "builtin" }
+
+    // Passing as an argument.
+    func_arg (__builtin_trap);         // { dg-error "builtin" }
+
+    // Passing through the ellipsis.
+    func_arg (0, __builtin_trap);      // { dg-error "builtin" }
+
+    // Return statement.
+    return __builtin_trap;             // { dg-error "builtin" }
+
+    (void)a;
+    (void)p;
+    (void)q;
+}
+
+// Taking the address of a builtin with a library "fallback" must be
+// allowed.
+void test_taking_address_of_library_builtin (void)
+{
+  {
+    typedef int F (int);
+    F *p = &__builtin_abs;              // { dg-bogus "builtin" }
+    (void)p;
+  }
+  {
+    typedef __SIZE_TYPE__ size_t;
+    typedef void* F (void*, const void*, size_t);
+    F *p = &*__builtin_memcpy;          // { dg-bogus "builtin" }
+    (void)p;
+  }
+
+  {
+    typedef __SIZE_TYPE__ size_t;
+    typedef size_t F (const char*);
+    F *p = &__builtin_strlen;           // { dg-bogus "builtin" }
+    (void)p;
+  }
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr54702_1.c b/gcc/testsuite/gcc.dg/lto/pr54702_1.c
index 2afb0fb..73c5693 100644
--- a/gcc/testsuite/gcc.dg/lto/pr54702_1.c
+++ b/gcc/testsuite/gcc.dg/lto/pr54702_1.c
@@ -1,3 +1,5 @@
+#include <stdlib.h>
+
 int *b;
 void *d;
 int c;
diff --git a/gcc/tree.h b/gcc/tree.h
index 250f99d..e8c846a 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -2086,12 +2086,19 @@ extern machine_mode element_mode (const_tree t);
 #define DECL_SOURCE_FILE(NODE) LOCATION_FILE (DECL_SOURCE_LOCATION (NODE))
 #define DECL_SOURCE_LINE(NODE) LOCATION_LINE (DECL_SOURCE_LOCATION (NODE))
 #define DECL_SOURCE_COLUMN(NODE) LOCATION_COLUMN (DECL_SOURCE_LOCATION (NODE))
-/* This accessor returns TRUE if the decl it operates on was created
+
+/* This accessor returns TRUE if the DECL it operates on was created
    by a front-end or back-end rather than by user code.  In this case
    builtin-ness is indicated by source location.  */
 #define DECL_IS_BUILTIN(DECL) \
   (LOCATION_LOCUS (DECL_SOURCE_LOCATION (DECL)) <= BUILTINS_LOCATION)

+/* Returns TRUE if the DECL it operates on is a GCC builtin that has
+   no library fallback.  This includes builtins defined using the
+   DEF_SYNC_BUILTIN macro (see builtins.def).  */
+#define DECL_IS_GCC_BUILTIN(DECL) \
+  (DECL_IS_BUILTIN (DECL) && !DECL_ASSEMBLER_NAME_SET_P (DECL))
+
 /*  For FIELD_DECLs, this is the RECORD_TYPE, UNION_TYPE, or
     QUAL_UNION_TYPE node that the field is a member of.  For VAR_DECL,
     PARM_DECL, FUNCTION_DECL, LABEL_DECL, RESULT_DECL, and CONST_DECL

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-06-29  7:37           ` Martin Sebor
@ 2015-07-02 14:20             ` Joseph Myers
  2015-07-04 22:32               ` Martin Sebor
  0 siblings, 1 reply; 34+ messages in thread
From: Joseph Myers @ 2015-07-02 14:20 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

On Sun, 28 Jun 2015, Martin Sebor wrote:

> 2015-06-28  Martin Sebor  <msebor@redhat.com>
> 
> 	pr c/66516
> 	* c-tree.h (c_validate_addressable): New function.
> 	* c-typeck.c (convert_arguments, parser_build_unary_op): Call it.
> 	(build_conditional_expr, c_cast_expr, convert_for_assignment): Same.
> 	(build_binary_op, c_objc_common_truthvalue_conversion): Same.
> 	(c_validate_addressable): Define function.

I don't think c_validate_addressable is a good name - given that it's 
called for lots of things that aren't addressable, in contexts in which 
there is no need for them to be addressable, and doesn't do checks of 
addressability in contexts where they are actually needed and done 
elsewhere (e.g. checks for not taking the address of a register variable).  
The question seems to be something more like: is the expression used as an 
operand something it's OK to use as an operand at all?

What is the logic for the list of functions above being a complete list of 
the places that need changes?

> @@ -4477,11 +4486,22 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
>        || TREE_CODE (TREE_TYPE (op2)) == ERROR_MARK)
>      return error_mark_node;
> 
> +  if (TREE_CODE (TREE_TYPE (ifexp)) == POINTER_TYPE
> +      && !c_validate_addressable (ifexp,
> +				  EXPR_LOCATION (TREE_OPERAND (ifexp, 0))))
> +    return error_mark_node;

How can ifexp be of pointer type?  It's undergone truthvalue conversion 
and should always be of type int at this point (but in any case, you can't 
refer to TREE_OPERAND (ifexp, 0) without knowing what sort of expression 
it is).

> +/* For EXPR that is an ADDR_EXPR or whose type is a FUNCTION_TYPE,
> +   determines whether its operand can have its address taken issues
> +   an error pointing to the location LOC.
> +   Operands that cannot have their address taken are builtin functions
> +   that have no library fallback (no other kinds of expressions are
> +   considered).
> +   Returns true when the expression can have its address taken and
> +   false otherwise.  */

Apart from the naming issue, the comment says nothing about the semantics 
of the function for an argument that's not of that form.

> +      error_at (loc, "builtin functions must be directly called");

"built-in" (see codingconventions.html).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-07-02 14:20             ` Joseph Myers
@ 2015-07-04 22:32               ` Martin Sebor
  2015-07-14  3:37                 ` [PING] " Martin Sebor
  0 siblings, 1 reply; 34+ messages in thread
From: Martin Sebor @ 2015-07-04 22:32 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

[-- Attachment #1: Type: text/plain, Size: 2128 bytes --]

> I don't think c_validate_addressable is a good name - given that it's
> called for lots of things that aren't addressable, in contexts in which
> there is no need for them to be addressable, and doesn't do checks of
> addressability in contexts where they are actually needed and done
> elsewhere (e.g. checks for not taking the address of a register variable).
> The question seems to be something more like: is the expression used as an
> operand something it's OK to use as an operand at all?

Thank you for the review.

I've changed the name to c_reject_gcc_builtin. If you would prefer
a different name still please suggest one. I'm not partial to any
one in particular.

> What is the logic for the list of functions above being a complete list of
> the places that need changes?

The logic by which I arrived at the changes was by constructing
test cases exercising expressions where a function is a valid
operand and where its address might need to be obtained when
it's not available, and stepping through the code and modifying
it until I found a suitable place to change to reject it.

> How can ifexp be of pointer type?  It's undergone truthvalue conversion
> and should always be of type int at this point (but in any case, you can't
> refer to TREE_OPERAND (ifexp, 0) without knowing what sort of expression
> it is).

I've removed the redundant test from this function.

>
>> +/* For EXPR that is an ADDR_EXPR or whose type is a FUNCTION_TYPE,
>> +   determines whether its operand can have its address taken issues
>> +   an error pointing to the location LOC.
>> +   Operands that cannot have their address taken are builtin functions
>> +   that have no library fallback (no other kinds of expressions are
>> +   considered).
>> +   Returns true when the expression can have its address taken and
>> +   false otherwise.  */
>
> Apart from the naming issue, the comment says nothing about the semantics
> of the function for an argument that's not of that form.

I've reworded the comment to hopefully make the semantics of
the function more clear.

Attached is an updated patch with these changes.

Martin

[-- Attachment #2: gcc-66516.patch --]
[-- Type: text/x-patch, Size: 29639 bytes --]

gcc/ChangeLog

2015-07-04  Martin Sebor  <msebor@redhat.com>

	pr c/66516
	* tree.h (DECL_IS_GCC_BUILTIN): New macro.
	* doc/extend.texi (Other Builtins): Document when the address of
	a built-in function can be taken.

gcc/c/ChangeLog

2015-07-04  Martin Sebor  <msebor@redhat.com>

	pr c/66516
	* c-tree.h (c_reject_gcc_builtin): New function.
	* c-typeck.c (convert_arguments, parser_build_unary_op): Call it.
	(build_conditional_expr, c_cast_expr, convert_for_assignment): Same.
	(build_binary_op, c_objc_common_truthvalue_conversion): Same.
	(c_reject_gcc_builtin): Define function.

gcc/cp/ChangeLog

2015-07-04  Martin Sebor  <msebor@redhat.com>

	pr c/66516
	* cp-tree.h (cp_reject_gcc_builtin): New function.
	* call.c (build_conditional_expr_1): Call it.
	(convert_arg_to_ellipsis, convert_for_arg_passing): Same.
	* pt.c (convert_template_argument): Same.
	* typeck.c (cp_build_binary_op, cp_build_addr_expr_strict): Same.
	(cp_build_unary_op, build_static_cast_1, build_reinterpret_cast_1):
	Same.
	(cp_build_c_cast, convert_for_assignment, convert_for_initialization):
	Same.
	(cp_reject_gcc_builtin): Define function.

gcc/testsuite/ChangeLog

2015-07-04  Martin Sebor  <msebor@redhat.com>

	pr c/66516
	* g++.dg/addr_builtin-1.C: New test.
	* gcc.dg/addr_builtin-1.c: New test.

diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 28b58c6..6a492c8 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -655,6 +655,7 @@ extern tree c_finish_transaction (location_t, tree, int);
 extern bool c_tree_equal (tree, tree);
 extern tree c_build_function_call_vec (location_t, vec<location_t>, tree,
 				       vec<tree, va_gc> *, vec<tree, va_gc> *);
+extern bool c_reject_gcc_builtin (const_tree, location_t = UNKNOWN_LOCATION);

 /* Set to 0 at beginning of a function definition, set to 1 if
    a return statement that specifies a return value is seen.  */
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 636e0bb..d27ace2 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -3343,6 +3343,10 @@ convert_arguments (location_t loc, vec<location_t> arg_loc, tree typelist,
 	  error (invalid_func_diag);
 	  return -1;
 	}
+      else if (TREE_CODE (val) == ADDR_EXPR && c_reject_gcc_builtin (val))
+	{
+	  return -1;
+	}
       else
 	/* Convert `short' and `char' to full-size `int'.  */
 	parmval = default_conversion (val);
@@ -3380,12 +3384,20 @@ parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg)
 {
   struct c_expr result;

-  result.value = build_unary_op (loc, code, arg.value, 0);
   result.original_code = code;
   result.original_type = NULL;

+  if (c_reject_gcc_builtin (arg.value))
+    {
+      result.value = error_mark_node;
+    }
+  else
+    {
+      result.value = build_unary_op (loc, code, arg.value, 0);
+
       if (TREE_OVERFLOW_P (result.value) && !TREE_OVERFLOW_P (arg.value))
 	overflow_warning (loc, result.value);
+    }

   return result;
 }
@@ -4486,6 +4498,12 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
   type2 = TREE_TYPE (op2);
   code2 = TREE_CODE (type2);

+  if (code1 == POINTER_TYPE && c_reject_gcc_builtin (op1))
+    return error_mark_node;
+
+  if (code2 == POINTER_TYPE && c_reject_gcc_builtin (op2))
+    return error_mark_node;
+
   /* C90 does not permit non-lvalue arrays in conditional expressions.
      In C99 they will be pointers by now.  */
   if (code1 == ARRAY_TYPE || code2 == ARRAY_TYPE)
@@ -5224,6 +5242,10 @@ c_cast_expr (location_t loc, struct c_type_name *type_name, tree expr)
   type = groktypename (type_name, &type_expr, &type_expr_const);
   warn_strict_prototypes = saved_wsp;

+  if (TREE_CODE (expr) == ADDR_EXPR && !VOID_TYPE_P (type)
+      && c_reject_gcc_builtin (expr))
+    return error_mark_node;
+
   ret = build_c_cast (loc, type, expr);
   if (type_expr)
     {
@@ -5863,6 +5885,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
   rhs = require_complete_type (rhs);
   if (rhs == error_mark_node)
     return error_mark_node;
+
+  if (coder == POINTER_TYPE && c_reject_gcc_builtin (rhs))
+    return error_mark_node;
+
   /* A non-reference type can convert to a reference.  This handles
      va_start, va_copy and possibly port built-ins.  */
   if (codel == REFERENCE_TYPE && coder != REFERENCE_TYPE)
@@ -10341,6 +10367,14 @@ build_binary_op (location_t location, enum tree_code code,
   if (code0 == ERROR_MARK || code1 == ERROR_MARK)
     return error_mark_node;

+  if (code0 == POINTER_TYPE
+      && c_reject_gcc_builtin (op0, EXPR_LOCATION (orig_op0)))
+    return error_mark_node;
+
+  if (code1 == POINTER_TYPE
+      && c_reject_gcc_builtin (op1, EXPR_LOCATION (orig_op1)))
+    return error_mark_node;
+
   if ((invalid_op_diag
        = targetm.invalid_binary_op (code, type0, type1)))
     {
@@ -11318,6 +11352,11 @@ c_objc_common_truthvalue_conversion (location_t location, tree expr)
       error_at (location, "void value not ignored as it ought to be");
       return error_mark_node;

+    case POINTER_TYPE:
+      if (c_reject_gcc_builtin (expr))
+	return error_mark_node;
+      break;
+
     case FUNCTION_TYPE:
       gcc_unreachable ();

@@ -12870,3 +12909,30 @@ cilk_install_body_with_frame_cleanup (tree fndecl, tree body, void *w)
   append_to_statement_list (build_stmt (EXPR_LOCATION (body), TRY_FINALLY_EXPR,
 				       	body_list, dtor), &list);
 }
+
+/* For an EXPR of a FUNCTION_TYPE that references a GCC built-in function
+   with no library fallback or for an ADDR_EXPR whose operand is such type
+   issues an error pointing to the location LOC.
+   Returns true when the expression has been diagnosed and false
+   otherwise.  */
+bool
+c_reject_gcc_builtin (const_tree expr, location_t loc /* = UNKNOWN_LOCATION */)
+{
+  if (TREE_CODE (expr) == ADDR_EXPR)
+    expr = TREE_OPERAND (expr, 0);
+
+  if (TREE_TYPE (expr)
+      && TREE_CODE (TREE_TYPE (expr)) == FUNCTION_TYPE
+      && DECL_P (expr) && DECL_IS_GCC_BUILTIN (expr))
+    {
+      if (loc == UNKNOWN_LOCATION)
+	loc = EXPR_LOC_OR_LOC (expr, input_location);
+      /* Reject arguments that are builtin functions with
+	 no library fallback.  */
+      error_at (loc, "builtin functions must be directly called");
+
+      return true;
+    }
+
+  return false;
+}
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 2d90ed9..75a1892 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -4689,6 +4689,15 @@ build_conditional_expr_1 (location_t loc, tree arg1, tree arg2, tree arg3,
       return fold_build3 (VEC_COND_EXPR, arg2_type, arg1, arg2, arg3);
     }

+  if (TREE_CODE (arg1) == FUNCTION_DECL && cp_reject_gcc_builtin (arg1))
+    return error_mark_node;
+
+  if (TREE_CODE (arg2) == FUNCTION_DECL && cp_reject_gcc_builtin (arg2))
+    return error_mark_node;
+
+  if (TREE_CODE (arg3) == FUNCTION_DECL && cp_reject_gcc_builtin (arg3))
+    return error_mark_node;
+
   /* [expr.cond]

      The first expression is implicitly converted to bool (clause
@@ -6628,12 +6637,17 @@ convert_arg_to_ellipsis (tree arg, tsubst_flags_t complain)
   tree arg_type;
   location_t loc = EXPR_LOC_OR_LOC (arg, input_location);

+  if (TREE_CODE (TREE_TYPE (arg)) == FUNCTION_TYPE
+      && cp_reject_gcc_builtin (arg))
+      return error_mark_node;
+
   /* [expr.call]

      The lvalue-to-rvalue, array-to-pointer, and function-to-pointer
      standard conversions are performed.  */
   arg = decay_conversion (arg, complain);
   arg_type = TREE_TYPE (arg);
+
   /* [expr.call]

      If the argument has integral or enumeration type that is subject
@@ -6909,6 +6923,10 @@ convert_for_arg_passing (tree type, tree val, tsubst_flags_t complain)
 	   && COMPLETE_TYPE_P (type)
 	   && tree_int_cst_lt (TYPE_SIZE (type), TYPE_SIZE (integer_type_node)))
     val = cp_perform_integral_promotions (val, complain);
+  else if (TREE_CODE (val) == ADDR_EXPR
+	   && cp_reject_gcc_builtin (val, complain))
+    return error_mark_node;
+
   if ((complain & tf_warning)
       && warn_suggest_attribute_format)
     {
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 1eac636..d32ba06 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6293,6 +6293,9 @@ extern bool check_raw_literal_operator		(const_tree decl);
 extern bool check_literal_operator_args		(const_tree, bool *, bool *);
 extern void maybe_warn_about_useless_cast       (tree, tree, tsubst_flags_t);
 extern tree cp_perform_integral_promotions      (tree, tsubst_flags_t);
+extern bool cp_reject_gcc_builtin             (const_tree,
+                                                 tsubst_flags_t = tf_error,
+                                                 location_t = UNKNOWN_LOCATION);

 /* in typeck2.c */
 extern void require_complete_eh_spec_types	(tree, tree);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 7f04fe6..d657eb1 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -1053,7 +1053,7 @@ retrieve_specialization (tree tmpl, tree args, hashval_t hash)
 		  ? TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (tmpl))
 		  : template_class_depth (DECL_CONTEXT (tmpl))));

-#ifdef ENABLE_CHECKING
+#if 0 // def ENABLE_CHECKING
   /* We should have gone through coerce_template_parms by now.  */
   ++processing_template_decl;
   if (!any_dependent_template_arguments_p (args))
@@ -6805,6 +6805,26 @@ convert_template_argument (tree parm,
       else if (val == error_mark_node && (complain & tf_error))
 	error ("could not convert template argument %qE to %qT",  orig_arg, t);

+      if (TREE_CODE (val) == ADDR_EXPR
+          && cp_reject_gcc_builtin (TREE_OPERAND (val, 0), complain))
+        {
+          /* Reject template arguments that are pointers to builtin
+             functions with no library fallbacks.  */
+          return error_mark_node;
+        }
+
+      if (INDIRECT_REF_P (val))
+        {
+          /* Reject template arguments that are references to builtin
+             functions with no library fallbacks.  */
+          const_tree inner = TREE_OPERAND (val, 0);
+          if (TREE_CODE (TREE_TYPE (inner)) == REFERENCE_TYPE
+              && TREE_CODE (TREE_TYPE (TREE_TYPE (inner))) == FUNCTION_TYPE
+              && TREE_CODE (TREE_TYPE (inner)) == REFERENCE_TYPE
+              && cp_reject_gcc_builtin (TREE_OPERAND (inner, 0), complain))
+              return error_mark_node;
+        }
+
       if (TREE_CODE (val) == SCOPE_REF)
 	{
 	  /* Strip typedefs from the SCOPE_REF.  */
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 5b09b73..5cba479 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4025,6 +4025,14 @@ cp_build_binary_op (location_t location,
   if (code0 == ERROR_MARK || code1 == ERROR_MARK)
     return error_mark_node;

+  if (code0 == POINTER_TYPE
+      && cp_reject_gcc_builtin (op0, complain, EXPR_LOCATION (orig_op0)))
+    return error_mark_node;
+
+  if (code1 == POINTER_TYPE
+      && cp_reject_gcc_builtin (op1, complain, EXPR_LOCATION (orig_op1)))
+    return error_mark_node;
+
   if ((invalid_op_diag
        = targetm.invalid_binary_op (code, type0, type1)))
     {
@@ -5621,6 +5629,9 @@ cp_build_addr_expr (tree arg, tsubst_flags_t complain)
 static tree
 cp_build_addr_expr_strict (tree arg, tsubst_flags_t complain)
 {
+  if (cp_reject_gcc_builtin (arg, complain))
+      return error_mark_node;
+
   return cp_build_addr_expr_1 (arg, 1, complain);
 }

@@ -5658,6 +5669,9 @@ cp_build_unary_op (enum tree_code code, tree xarg, int noconvert,
       return error_mark_node;
     }

+  if (cp_reject_gcc_builtin (arg, complain))
+    return error_mark_node;
+
   switch (code)
     {
     case UNARY_PLUS_EXPR:
@@ -6566,6 +6580,10 @@ build_static_cast_1 (tree type, tree expr, bool c_cast_p,
   if (abstract_virtuals_error_sfinae (ACU_CAST, type, complain))
     return error_mark_node;

+  if (TREE_CODE (intype) == FUNCTION_TYPE
+      && cp_reject_gcc_builtin (expr, complain))
+      return error_mark_node;
+
   /* [expr.static.cast]

      An expression e can be explicitly converted to a type T using a
@@ -6833,6 +6851,9 @@ build_reinterpret_cast_1 (tree type, tree expr, bool c_cast_p,
 	warning (0, "casting %qT to %qT does not dereference pointer",
 		 intype, type);

+      if (cp_reject_gcc_builtin (expr, complain))
+	  return error_mark_node;
+
       expr = cp_build_addr_expr (expr, complain);

       if (warn_strict_aliasing > 2)
@@ -6860,6 +6881,10 @@ build_reinterpret_cast_1 (tree type, tree expr, bool c_cast_p,
 	  || VOID_TYPE_P (TREE_TYPE (type))))
     return convert_member_func_to_ptr (type, expr, complain);

+  if (TREE_CODE (intype) == FUNCTION_TYPE
+      && cp_reject_gcc_builtin (expr, complain, input_location))
+      return error_mark_node;
+
   /* If the cast is not to a reference type, the lvalue-to-rvalue,
      array-to-pointer, and function-to-pointer conversions are
      performed.  */
@@ -7241,6 +7266,10 @@ cp_build_c_cast (tree type, tree expr, tsubst_flags_t complain)
     warning_at (input_location, OPT_Wint_to_pointer_cast,
 		"cast to pointer from integer of different size");

+  if (FUNCTION_POINTER_TYPE_P (type)
+      && cp_reject_gcc_builtin (value, complain))
+    return error_mark_node;
+
   /* A C-style cast can be a const_cast.  */
   result = build_const_cast_1 (type, value, complain & tf_warning,
 			       &valid_p);
@@ -8105,6 +8134,9 @@ convert_for_assignment (tree type, tree rhs,
       return error_mark_node;
     }

+  if (coder == FUNCTION_TYPE && cp_reject_gcc_builtin (rhs, complain))
+      return error_mark_node;
+
   if (c_dialect_objc ())
     {
       int parmno;
@@ -8307,6 +8339,10 @@ convert_for_initialization (tree exp, tree type, tree rhs, int flags,
       || (TREE_CODE (rhs) == TREE_LIST && TREE_VALUE (rhs) == error_mark_node))
     return error_mark_node;

+  if (TREE_CODE (TREE_TYPE (rhs)) == FUNCTION_TYPE
+      && cp_reject_gcc_builtin (rhs, complain, input_location))
+      return error_mark_node;
+
   if ((TREE_CODE (TREE_TYPE (rhs)) == ARRAY_TYPE
        && TREE_CODE (type) != ARRAY_TYPE
        && (TREE_CODE (type) != REFERENCE_TYPE
@@ -9328,3 +9364,37 @@ check_literal_operator_args (const_tree decl,
       return true;
     }
 }
+
+/* For an EXPR of a FUNCTION_TYPE that references a GCC built-in function
+   with no library fallback or for an ADDR_EXPR whose operand is such type
+   and, when COMPLAIN & tf_error is non-zero,  issues an error pointing to
+   the location LOC.
+   Returns true when the expression would have been diagnosed if COMPLAIN
+   & tf_error had been non-zero and false otherwise.  */
+bool
+cp_reject_gcc_builtin (const_tree expr, tsubst_flags_t complain,
+		       location_t loc /* = UNKNOWN_LOCATION */)
+{
+  if (TREE_CODE (expr) == ADDR_EXPR)
+    expr = TREE_OPERAND (expr, 0);
+
+  if (TREE_TYPE (expr)
+      && TREE_CODE (TREE_TYPE (expr)) == FUNCTION_TYPE
+      && DECL_P (expr) && DECL_IS_GCC_BUILTIN (expr)
+      && !DECL_IS_OPERATOR_NEW (expr)
+      && (!DECL_NAME (expr)
+	  || !NEW_DELETE_OPNAME_P (DECL_NAME (expr))))
+    {
+      if (complain & tf_error) {
+	if (loc == UNKNOWN_LOCATION)
+	  loc = EXPR_LOC_OR_LOC (expr, input_location);
+	/* Reject arguments that are builtin functions with
+	   no library fallback.  */
+	error_at (loc, "builtin functions must be directly called");
+      }
+
+      return true;
+    }
+
+  return false;
+}
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 9ad2b68..8d0c2b1 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -10080,14 +10080,22 @@ recommend general use of these functions.

 The remaining functions are provided for optimization purposes.

+With the exception of built-ins that have library equivalents such as
+the standard C library functions discussed below, or that expand to
+library calls, GCC built-in functions are always expanded inline and
+thus do not have corresponding entry points and their address cannot
+be obtained.  Attempting to use them in an expression other than
+a function call results in a compile-time error.
+
 @opindex fno-builtin
 GCC includes built-in versions of many of the functions in the standard
-C library.  The versions prefixed with @code{__builtin_} are always
-treated as having the same meaning as the C library function even if you
-specify the @option{-fno-builtin} option.  (@pxref{C Dialect Options})
-Many of these functions are only optimized in certain cases; if they are
-not optimized in a particular case, a call to the library function is
-emitted.
+C library.  These functions come in two forms: one whose names are prefixed
+with the @code{__builtin_} prefix, and the other without.  Both forms have
+the same type (including prototype), the same address (when their address is
+taken), and the same meaning as the C library functions even if you specify
+the @option{-fno-builtin} option @pxref{C Dialect Options}).  Many of these
+functions are only optimized in certain cases; if they are not optimized in
+particular case, a call to the library function is emitted.

 @opindex ansi
 @opindex std
diff --git a/gcc/testsuite/g++.dg/addr_builtin-1.C b/gcc/testsuite/g++.dg/addr_builtin-1.C
new file mode 100644
index 0000000..5e78b35
--- /dev/null
+++ b/gcc/testsuite/g++.dg/addr_builtin-1.C
@@ -0,0 +1,188 @@
+/* PR66516 - missing diagnostic on taking the address of a built-in function */
+/* { dg-do compile } */
+/* { dg-options "-Wno-error=pedantic" } */
+
+typedef void (F)();
+typedef __UINTPTR_TYPE__ uintptr_t;
+
+// Utility function to test passing built-in functions as an ordinary
+// argument and via the ellipsis.
+static void func_arg (F*, ...);
+
+// Utility templates to test specializing templates on pointers and
+// references to built-in functions.
+template <F*> struct TestPointer { };
+template <F&> struct TestReference { };
+
+// Utility function with which, along with the built-in function,
+// to instantiate the C98 multi-parameter or C11 variadic tempates
+// below.
+void f () { }
+
+#if 201103 <= __cplusplus
+
+template <F*...> struct TestPointers { };
+template <F&...> struct TestReferences { };
+
+#else
+
+template <F* = &f, F* = &f> struct TestPointers { };
+template <F& = f, F& = f> struct TestReferences { };
+
+#endif
+
+static F* test_taking_address_of_gcc_builtin ()
+{
+  enum UINTPTR_E { e = ~(uintptr_t)0 };
+
+  F *p;
+  void *q;
+  uintptr_t a;
+
+  __builtin_trap ();                 // { dg-bogus "builtin" }
+  (void)__builtin_trap;              // { dg-bogus "builtin" }
+  __builtin_trap;                    // { dg-bogus "builtin" }
+
+  // Address operator.
+  p = &__builtin_trap;               // { dg-error "builtin" }
+
+  // Unary NOT.
+  a = !__builtin_trap;               // { dg-error "builtin" }
+
+  // Casts.
+  p = (F*)__builtin_trap;            // { dg-error "builtin" }
+
+  p = &(F&)__builtin_trap;            // { dg-error "builtin" }
+
+  p = &reinterpret_cast<F&>(__builtin_trap);   // { dg-error "builtin" }
+  p = &static_cast<F&>(__builtin_trap);        // { dg-error "builtin" }
+
+  p = reinterpret_cast<F*>(__builtin_trap);    // { dg-error "builtin" }
+  p = static_cast<F*>(__builtin_trap);         // { dg-error "builtin" }
+
+  a = reinterpret_cast<uintptr_t>(__builtin_trap);  // { dg-error "builtin" }
+  a = static_cast<uintptr_t>(__builtin_trap);       // { dg-error "builtin" }
+
+  a = reinterpret_cast<UINTPTR_E>(__builtin_trap);  // { dg-error "builtin" }
+  a = static_cast<UINTPTR_E>(__builtin_trap);       // { dg-error "builtin" }
+
+  // Additive operator.  Ill-formed but allowed with -fpermissive.
+  p = __builtin_trap + 0;            // { dg-error "builtin" }
+  p = __builtin_trap - 0;            // { dg-error "builtin" }
+  a = __builtin_trap - p;            // { dg-error "builtin" }
+  a = p - __builtin_trap;            // { dg-error "builtin" }
+
+  // Relational operators.  Ill-formed but allowed with -fpermissive.
+  a = __builtin_trap < p;            // { dg-error "builtin" }
+  a = p < __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap > p;            // { dg-error "builtin" }
+  a = p > __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap > p;            // { dg-error "builtin" }
+  a = p > __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  // Equality operators.
+  a = __builtin_trap == p;           // { dg-error "builtin" }
+  a = p == __builtin_trap;           // { dg-error "builtin" }
+  a = __builtin_trap != p;           // { dg-error "builtin" }
+  a = p != __builtin_trap;           // { dg-error "builtin" }
+
+  // Logical AND and OR.
+  a = __builtin_trap && p;           // { dg-error "builtin" }
+  a = p && __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap || p;           // { dg-error "builtin" }
+  a = p || __builtin_trap;           // { dg-error "builtin" }
+
+  // Conditional operator.
+  a = __builtin_trap ? 1 : 0;        // { dg-error "builtin" }
+  p = a ? __builtin_trap : 0;        // { dg-error "builtin" }
+  p = a ? 0 : __builtin_trap;        // { dg-error "builtin" }
+
+  // Assignment operator.
+  p = __builtin_trap;                // { dg-error "builtin" }
+
+  // Passing as an argument.
+  func_arg (__builtin_trap);         // { dg-error "builtin" }
+
+  // Passing through ellipsis.
+  func_arg (0, __builtin_trap);      // { dg-error "builtin" }
+
+  {
+    // Template specialization.
+    TestPointer<__builtin_trap> tp;         // { dg-error "builtin" }
+    TestReference<__builtin_trap> tr;       // { dg-error "builtin" }
+
+    TestPointers<__builtin_trap> tp1;       // { dg-error "builtin" }
+    TestReferences<__builtin_trap> tr1;     // { dg-error "builtin" }
+
+    TestPointers<f, __builtin_trap> tp2;    // { dg-error "builtin" }
+    TestReferences<f, __builtin_trap> tr2;  // { dg-error "builtin" }
+
+    TestPointers<__builtin_trap, f> tp3;    // { dg-error "builtin" }
+    TestReferences<__builtin_trap, f> tr3;  // { dg-error "builtin" }
+  }
+
+  return __builtin_trap;                    // { dg-error "builtin" }
+
+  (void)a;
+  (void)p;
+  (void)q;
+}
+
+// Operators new and delete are treated as GCC builti-ns with no library
+// fallbacks yet they exist in the runtime library and programs must be
+// able to take their address.
+void test_taking_address_of_op_new_and_delete ()
+{
+  typedef __SIZE_TYPE__ size_t;
+
+  typedef void* (OpNew) (size_t);
+  typedef void (OpDelete) (void*);
+
+  OpNew &newr = operator new;           // { dg-bogus "builtin" }
+  OpNew &newra = operator new[];        // { dg-bogus "builtin" }
+  OpNew *newp = &operator new;          // { dg-bogus "builtin" }
+  newp = &operator new[];               // { dg-bogus "builtin" }
+
+  OpDelete &delr = operator delete;     // { dg-bogus "builtin" }
+  OpDelete &delra = operator delete[];  // { dg-bogus "builtin" }
+  OpDelete *delp = &operator delete;    // { dg-bogus "builtin" }
+  delp = &operator delete[];            // { dg-bogus "builtin" }
+
+  (void)newr;
+  (void)newp;
+  (void)delr;
+  (void)delp;
+}
+
+// Creating a reference to or taking the address of a built-in with
+// a library "fallback" must be allowed.
+void test_taking_address_of_library_builtin ()
+{
+  {
+    typedef int F (int);
+    F &r = __builtin_abs;               // { dg-bogus "builtin" }
+    F *p = &__builtin_abs;              // { dg-bogus "builtin" }
+    (void)p;
+    (void)r;
+  }
+  {
+    typedef __SIZE_TYPE__ size_t;
+    typedef size_t F (const char*);
+    F &r = __builtin_strlen;            // { dg-bogus "builtin" }
+    F *p = &__builtin_strlen;           // { dg-bogus "builtin" }
+    (void)p;
+    (void)p;
+  }
+}
diff --git a/gcc/testsuite/gcc.dg/addr_builtin-1.c b/gcc/testsuite/gcc.dg/addr_builtin-1.c
new file mode 100644
index 0000000..42346ef
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/addr_builtin-1.c
@@ -0,0 +1,117 @@
+/* PR66516 - missing diagnostic on taking the address of a built-in function */
+/* { dg-do compile } */
+/* { dg-options "-Wno-error=pedantic" } */
+
+typedef void (F)(void);
+typedef __UINTPTR_TYPE__ uintptr_t;
+
+// Utility function to test passing built-in functions as an ordinary
+// argument and via the ellipsis.
+static void func_arg (F *p, ...) { (void)p; }
+
+static F* test_taking_address_of_gcc_builtin (void)
+{
+    enum UINTPTR_E { e = ~(uintptr_t)0 };
+
+    F *p;
+    void *q;
+    uintptr_t a;
+
+    // Call, cast to void, and id are allowed.
+    __builtin_trap ();                 // { dg-bogus "builtin" }
+    (void)__builtin_trap;              // { dg-bogus "builtin" }
+    __builtin_trap;                    // { dg-bogus "builtin" }
+
+    // Address operator.
+    p = &__builtin_trap;               // { dg-error "builtin" }
+
+    // Unary NOT.
+    a = !__builtin_trap;               // { dg-error "builtin" }
+
+    // Sizeof and _Alignof are disallowed by C but allowed by GCC.
+    a = sizeof __builtin_trap;         // { dg-bogus "builtin" }
+    a = _Alignof __builtin_trap;       // { dg-bogus "builtin" }
+
+    // Casts.
+    p = (F*)__builtin_trap;            // { dg-error "builtin" }
+    a = (uintptr_t)__builtin_trap;     // { dg-error "builtin" }
+
+    // Additive operator.
+    p = __builtin_trap + 0;            // { dg-error "builtin" }
+    p = __builtin_trap - 0;            // { dg-error "builtin" }
+    a = __builtin_trap - p;            // { dg-error "builtin" }
+    a = p - __builtin_trap;            // { dg-error "builtin" }
+
+    // Relational operators.
+    a = __builtin_trap < p;            // { dg-error "builtin" }
+    a = p < __builtin_trap;            // { dg-error "builtin" }
+
+    a = __builtin_trap <= p;           // { dg-error "builtin" }
+    a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+    a = __builtin_trap > p;            // { dg-error "builtin" }
+    a = p > __builtin_trap;            // { dg-error "builtin" }
+
+    a = __builtin_trap > p;            // { dg-error "builtin" }
+    a = p > __builtin_trap;            // { dg-error "builtin" }
+
+    a = __builtin_trap <= p;           // { dg-error "builtin" }
+    a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+    a = __builtin_trap <= p;           // { dg-error "builtin" }
+    a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+    // Equality operators.
+    a = __builtin_trap == p;           // { dg-error "builtin" }
+    a = p == __builtin_trap;           // { dg-error "builtin" }
+    a = __builtin_trap != p;           // { dg-error "builtin" }
+    a = p != __builtin_trap;           // { dg-error "builtin" }
+
+    // Logical AND and OR.
+    a = __builtin_trap && p;           // { dg-error "builtin" }
+    a = p && __builtin_trap;           // { dg-error "builtin" }
+
+    a = __builtin_trap || p;           // { dg-error "builtin" }
+    a = p || __builtin_trap;           // { dg-error "builtin" }
+
+    // Conditional operator.
+    a = __builtin_trap ? 1 : 0;        // { dg-error "builtin" }
+    p = a ? __builtin_trap : 0;        // { dg-error "builtin" }
+    p = a ? 0 : __builtin_trap;        // { dg-error "builtin" }
+
+    // Assignment operator.
+    p = __builtin_trap;                // { dg-error "builtin" }
+
+    q = __builtin_trap;                // { dg-error "builtin" }
+    a = __builtin_trap;                // { dg-error "builtin" }
+
+    // Passing as an argument.
+    func_arg (__builtin_trap);         // { dg-error "builtin" }
+
+    // Passing through the ellipsis.
+    func_arg (0, __builtin_trap);      // { dg-error "builtin" }
+
+    // Return statement.
+    return __builtin_trap;             // { dg-error "builtin" }
+
+    (void)a;
+    (void)p;
+    (void)q;
+}
+
+// Taking the address of a built-in with a library "fallback" must be
+// allowed.
+void test_taking_address_of_library_builtin (void)
+{
+  {
+    typedef int F (int);
+    F *p = &__builtin_abs;              // { dg-bogus "builtin" }
+    (void)p;
+  }
+  {
+    typedef __SIZE_TYPE__ size_t;
+    typedef size_t F (const char*);
+    F *p = &__builtin_strlen;           // { dg-bogus "builtin" }
+    (void)p;
+  }
+}
diff --git a/gcc/tree.h b/gcc/tree.h
index ca5e681..17663d9 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -2086,12 +2086,19 @@ extern machine_mode element_mode (const_tree t);
 #define DECL_SOURCE_FILE(NODE) LOCATION_FILE (DECL_SOURCE_LOCATION (NODE))
 #define DECL_SOURCE_LINE(NODE) LOCATION_LINE (DECL_SOURCE_LOCATION (NODE))
 #define DECL_SOURCE_COLUMN(NODE) LOCATION_COLUMN (DECL_SOURCE_LOCATION (NODE))
-/* This accessor returns TRUE if the decl it operates on was created
+
+/* This accessor returns TRUE if the DECL it operates on was created
    by a front-end or back-end rather than by user code.  In this case
    builtin-ness is indicated by source location.  */
 #define DECL_IS_BUILTIN(DECL) \
   (LOCATION_LOCUS (DECL_SOURCE_LOCATION (DECL)) <= BUILTINS_LOCATION)

+/* Returns TRUE if the DECL it operates on is a GCC builtin that has
+   no library fallback.  This includes built-ins defined using the
+   DEF_SYNC_BUILTIN macro (see builtins.def).  */
+#define DECL_IS_GCC_BUILTIN(DECL) \
+  (DECL_IS_BUILTIN (DECL) && !DECL_ASSEMBLER_NAME_SET_P (DECL))
+
 /*  For FIELD_DECLs, this is the RECORD_TYPE, UNION_TYPE, or
     QUAL_UNION_TYPE node that the field is a member of.  For VAR_DECL,
     PARM_DECL, FUNCTION_DECL, LABEL_DECL, RESULT_DECL, and CONST_DECL

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-07-04 22:32               ` Martin Sebor
@ 2015-07-14  3:37                 ` Martin Sebor
  2015-07-14 15:01                   ` Jason Merrill
  0 siblings, 1 reply; 34+ messages in thread
From: Martin Sebor @ 2015-07-14  3:37 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List, Jason Merrill

[CC Jason since the patch also touches the C++ front end]

The updated patch is here:
   https://gcc.gnu.org/ml/gcc-patches/2015-07/msg00258.html

Thanks
Martin

On 07/04/2015 04:32 PM, Martin Sebor wrote:
>> I don't think c_validate_addressable is a good name - given that it's
>> called for lots of things that aren't addressable, in contexts in which
>> there is no need for them to be addressable, and doesn't do checks of
>> addressability in contexts where they are actually needed and done
>> elsewhere (e.g. checks for not taking the address of a register
>> variable).
>> The question seems to be something more like: is the expression used
>> as an
>> operand something it's OK to use as an operand at all?
>
> Thank you for the review.
>
> I've changed the name to c_reject_gcc_builtin. If you would prefer
> a different name still please suggest one. I'm not partial to any
> one in particular.
>
>> What is the logic for the list of functions above being a complete
>> list of
>> the places that need changes?
>
> The logic by which I arrived at the changes was by constructing
> test cases exercising expressions where a function is a valid
> operand and where its address might need to be obtained when
> it's not available, and stepping through the code and modifying
> it until I found a suitable place to change to reject it.
>
>> How can ifexp be of pointer type?  It's undergone truthvalue conversion
>> and should always be of type int at this point (but in any case, you
>> can't
>> refer to TREE_OPERAND (ifexp, 0) without knowing what sort of expression
>> it is).
>
> I've removed the redundant test from this function.
>
>>
>>> +/* For EXPR that is an ADDR_EXPR or whose type is a FUNCTION_TYPE,
>>> +   determines whether its operand can have its address taken issues
>>> +   an error pointing to the location LOC.
>>> +   Operands that cannot have their address taken are builtin functions
>>> +   that have no library fallback (no other kinds of expressions are
>>> +   considered).
>>> +   Returns true when the expression can have its address taken and
>>> +   false otherwise.  */
>>
>> Apart from the naming issue, the comment says nothing about the semantics
>> of the function for an argument that's not of that form.
>
> I've reworded the comment to hopefully make the semantics of
> the function more clear.
>
> Attached is an updated patch with these changes.
>
> Martin

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-07-14  3:37                 ` [PING] " Martin Sebor
@ 2015-07-14 15:01                   ` Jason Merrill
  2015-07-14 15:07                     ` Jason Merrill
  2015-07-14 18:04                     ` Martin Sebor
  0 siblings, 2 replies; 34+ messages in thread
From: Jason Merrill @ 2015-07-14 15:01 UTC (permalink / raw)
  To: Martin Sebor, Joseph Myers; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

I wonder if it would make sense to handle this when we actually try to 
emit a reference to one of these functions in the back end, rather than 
in many places through the front end.

If it's going to stay in the front end, the C and C++ front-ends should 
share an implementation of this function, in c-common.c.

Most of the calls in the C++ front end can be replaced by a single call 
in mark_rvalue_use.

> -#ifdef ENABLE_CHECKING
> +#if 0 // def ENABLE_CHECKING

This change is unrelated.

> +#define DECL_IS_GCC_BUILTIN(DECL) \

This macro name could be clearer.  DECL_IS_ONLY_BUILTIN? 
DECL_IS_NOFALLBACK_BUILTIN?

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-07-14 15:01                   ` Jason Merrill
@ 2015-07-14 15:07                     ` Jason Merrill
  2015-07-14 18:04                     ` Martin Sebor
  1 sibling, 0 replies; 34+ messages in thread
From: Jason Merrill @ 2015-07-14 15:07 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

I wonder if it would make sense to handle this when we actually try to 
emit a reference to one of these functions in the back end, rather than 
in many places through the front end.

If it's going to stay in the front end, the C and C++ front-ends should 
share an implementation of this function, in c-common.c.

Most of the calls in the C++ front end can be replaced by a single call 
in mark_rvalue_use.

> -#ifdef ENABLE_CHECKING
> +#if 0 // def ENABLE_CHECKING

This change is unrelated.

> +#define DECL_IS_GCC_BUILTIN(DECL) \

This macro name could be clearer.  DECL_IS_ONLY_BUILTIN? 
DECL_IS_NOFALLBACK_BUILTIN?

Jason


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-07-14 15:01                   ` Jason Merrill
  2015-07-14 15:07                     ` Jason Merrill
@ 2015-07-14 18:04                     ` Martin Sebor
  2015-07-29  7:27                       ` Jason Merrill
  1 sibling, 1 reply; 34+ messages in thread
From: Martin Sebor @ 2015-07-14 18:04 UTC (permalink / raw)
  To: Jason Merrill, Joseph Myers; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

On 07/14/2015 09:01 AM, Jason Merrill wrote:
> I wonder if it would make sense to handle this when we actually try to
> emit a reference to one of these functions in the back end, rather than
> in many places through the front end.
>
> If it's going to stay in the front end, the C and C++ front-ends should
> share an implementation of this function, in c-common.c.

Thanks for the comments and the suggestion. It would certainly
be much cleaner if it could be detected in one place. Let me
investigate the back end option.

FWIW, my initial attempt at a patch did have a single function
with common logic, until it became clear that the C++ conditions
the function tested were slightly different (and more involved
due to the checks for operators new and delete) than those in
C. But since they don't seem to be incompatible with one another,
as long as no one minds that when called from the C front end
the function ends up doing some unnecessary work I can merge
them back (if the back end idea doesn't pan out).

>
> Most of the calls in the C++ front end can be replaced by a single call
> in mark_rvalue_use.
>
>> -#ifdef ENABLE_CHECKING
>> +#if 0 // def ENABLE_CHECKING
>
> This change is unrelated.

Yes, sorry about that.

>
>> +#define DECL_IS_GCC_BUILTIN(DECL) \
>
> This macro name could be clearer.  DECL_IS_ONLY_BUILTIN?
> DECL_IS_NOFALLBACK_BUILTIN?

I derived DECL_IS_GCC_BUILTIN from the DEF_GCC_BUILTIN macro
in builtins.def used to define such builtins. There are no
DECL_IS_XXX_BUILTINs for the other DEF_XXX_BUILTIN macros yet
but I imagine there could be and it seems that keeping the
names consistent would make the two sets of macros easier to
understand and work with. But if you don't think this is
important I'm fine renaming the macro.

Martin

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-07-14 18:04                     ` Martin Sebor
@ 2015-07-29  7:27                       ` Jason Merrill
  2015-07-29 19:07                         ` Martin Sebor
  2015-08-28 20:10                         ` Martin Sebor
  0 siblings, 2 replies; 34+ messages in thread
From: Jason Merrill @ 2015-07-29  7:27 UTC (permalink / raw)
  To: Martin Sebor, Joseph Myers; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

Sorry about the slow response on IRC today, I got distracted onto 
another issue and forgot to check back.  What I started to write:

> I'm exploring your suggestion to see if the back end could emit the diagnostics. But I'm not sure it has sufficient context (location information) to point to the line of code that uses the function.

Hmm, that's a good point.  I think it would make sense for the ADDR_EXPR 
to carry location information as long as we're dealing with trees, but I 
suspect we don't currently set the location of an ADDR_EXPR.  So that 
would need to be fixed as part of this approach.

> I suspect the back end or even the middle end route isn't going to work even if there was enough context to diagnose the problem expressions because some of them will have been optimized away by then (e.g., 'if (& __builtin_foo != 0)' is optimized into if (1) by gimple).

I was thinking that if they're optimized away, they aren't problematic 
anymore; that was part of the attraction for me of handling this lower down.

> The second question is about your suggestion to consolidate the code into mark_rvalue_use. The problem I'm running into there is that mark_rvalue_use is called for calls to builtins as well as for other uses and doesn't have enough context to tell one from the other.

Ah, true.  But special-casing call uses is still fewer places than 
special-casing all non-call uses.

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-07-29  7:27                       ` Jason Merrill
@ 2015-07-29 19:07                         ` Martin Sebor
  2015-08-03 23:02                           ` Martin Sebor
  2015-08-28 20:10                         ` Martin Sebor
  1 sibling, 1 reply; 34+ messages in thread
From: Martin Sebor @ 2015-07-29 19:07 UTC (permalink / raw)
  To: Jason Merrill, Joseph Myers; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

On 07/28/2015 09:38 PM, Jason Merrill wrote:
> Sorry about the slow response on IRC today, I got distracted onto
> another issue and forgot to check back.  What I started to write:

No problem.

>
>> I'm exploring your suggestion to see if the back end could emit the
>> diagnostics. But I'm not sure it has sufficient context (location
>> information) to point to the line of code that uses the function.
>
> Hmm, that's a good point.  I think it would make sense for the ADDR_EXPR
> to carry location information as long as we're dealing with trees, but I
> suspect we don't currently set the location of an ADDR_EXPR.  So that
> would need to be fixed as part of this approach.

Okay, let me look into this.

>
>> I suspect the back end or even the middle end route isn't going to
>> work even if there was enough context to diagnose the problem
>> expressions because some of them will have been optimized away by then
>> (e.g., 'if (& __builtin_foo != 0)' is optimized into if (1) by gimple).
>
> I was thinking that if they're optimized away, they aren't problematic
> anymore; that was part of the attraction for me of handling this lower
> down.

Yes, they're not problematic in that they don't cause linker unsats.
I have a few concerns with the approach of accepting or rejecting
constructs based on optimization (e.g., that it might lead to
similar problems as with -Wmaybe-uninitialized; or that it could
be perceived as inconsistent). But it may not be as bad as it seems
-- let me look into it if only as a learning exercise and see how
it goes.

>
>> The second question is about your suggestion to consolidate the code
>> into mark_rvalue_use. The problem I'm running into there is that
>> mark_rvalue_use is called for calls to builtins as well as for other
>> uses and doesn't have enough context to tell one from the other.
>
> Ah, true.  But special-casing call uses is still fewer places than
> special-casing all non-call uses.

This will be moot if I can implement it the middle-end. If not,
I'll give it a try to see if this alternative ends up reducing
the footprint of the patch.

Thanks
Martin

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-07-29 19:07                         ` Martin Sebor
@ 2015-08-03 23:02                           ` Martin Sebor
  2015-08-04 15:04                             ` Jason Merrill
  2015-08-04 15:19                             ` Joseph Myers
  0 siblings, 2 replies; 34+ messages in thread
From: Martin Sebor @ 2015-08-03 23:02 UTC (permalink / raw)
  To: Jason Merrill, Joseph Myers; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

>>> I suspect the back end or even the middle end route isn't going to
>>> work even if there was enough context to diagnose the problem
>>> expressions because some of them will have been optimized away by then
>>> (e.g., 'if (& __builtin_foo != 0)' is optimized into if (1) by gimple).
>>
>> I was thinking that if they're optimized away, they aren't problematic
>> anymore; that was part of the attraction for me of handling this lower
>> down.
>
> Yes, they're not problematic in that they don't cause linker unsats.
> I have a few concerns with the approach of accepting or rejecting
> constructs based on optimization (e.g., that it might lead to
> similar problems as with -Wmaybe-uninitialized; or that it could
> be perceived as inconsistent). But it may not be as bad as it seems
> -- let me look into it if only as a learning exercise and see how
> it goes.

I've prototyped this approach in a couple places in the middle
end. Both implementations are very simple and work great when
the code isn't optimized away. The problem is that the
optimizations done at various points between the front end and
the final gimplification make the diagnostics inconsistent.

For instance, with F being the type of the builtin, this is
diagnosed in the first prototype:

   F* foo () { if (0) return __builtin_trap; return 0; }

but this isn't:

   F* bar () { return 0 ? __builtin_trap : 0; }

because the ternary ?: expression is folded into a constant by
the front end even before it reaches the gimplifier, while the
if-else statement isn't folded until the control flow graph is
built. (As an aside: I'm wondering why that is. Why have the
front end do any optimization at all if the middle can and
does them too, and better?)

Diagnosing neither of these cases might perhaps seem like the
way to go since they are both safe (don't result in a linker
error). I implemented this approach in the second prototype in
a later pass but that exposed even more inconsistencies as even
more code is optimized (for instance, exception handling).

I think this sort of inconsistency in diagnosing semantically
equivalent language constructs would be viewed as arbitrary
and be unhelpful to users. It's too bad because otherwise this
solution would have been quite elegant and simple (touching
just one function).

Unless you have some other ideas I'm going to abandon this
approach see if the original patch can be simplified by
consoldating some of the handling into mark_rvalue_use.

Martin

PS Another problem, one that in my view sinks the middle end
approach, is the fact that references in unused static inline
functions aren't diagnosed either, even if calling the inline
function would result in taking the address of the builtin
function. E.g., this isn't diagnosed:

   static inline F* baz (void) { return __builtin_trap; }

unless baz() is called. This would make the feature largely
ineffective in C++ template libraries.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-08-03 23:02                           ` Martin Sebor
@ 2015-08-04 15:04                             ` Jason Merrill
  2015-08-04 15:58                               ` Jeff Law
  2015-08-04 15:19                             ` Joseph Myers
  1 sibling, 1 reply; 34+ messages in thread
From: Jason Merrill @ 2015-08-04 15:04 UTC (permalink / raw)
  To: Martin Sebor, Joseph Myers; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

On 08/03/2015 07:02 PM, Martin Sebor wrote:
> I've prototyped this approach in a couple places in the middle
> end. Both implementations are very simple and work great when
> the code isn't optimized away. The problem is that the
> optimizations done at various points between the front end and
> the final gimplification make the diagnostics inconsistent.
>
> For instance, with F being the type of the builtin, this is
> diagnosed in the first prototype:
>
>    F* foo () { if (0) return __builtin_trap; return 0; }
>
> but this isn't:
>
>    F* bar () { return 0 ? __builtin_trap : 0; }
>
> because the ternary ?: expression is folded into a constant by
> the front end even before it reaches the gimplifier, while the
> if-else statement isn't folded until the control flow graph is
> built. (As an aside: I'm wondering why that is. Why have the
> front end do any optimization at all if the middle can and
> does them too, and better?)

This is largely historical baggage, I think, from days where computers 
had less memory and we were trying to do as much processing as possible 
immediately.

The c++-delayed-folding branch delays folding the ?: expression until 
the end of the function, at which point we can see better what context 
the function is being used in, which could simplify your patch.

But your question leads me to wonder if we need to do front end folding 
there, either...

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-08-03 23:02                           ` Martin Sebor
  2015-08-04 15:04                             ` Jason Merrill
@ 2015-08-04 15:19                             ` Joseph Myers
  1 sibling, 0 replies; 34+ messages in thread
From: Joseph Myers @ 2015-08-04 15:19 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Jason Merrill, Jakub Jelinek, Marek Polacek, Gcc Patch List

On Mon, 3 Aug 2015, Martin Sebor wrote:

> because the ternary ?: expression is folded into a constant by
> the front end even before it reaches the gimplifier, while the
> if-else statement isn't folded until the control flow graph is
> built. (As an aside: I'm wondering why that is. Why have the
> front end do any optimization at all if the middle can and
> does them too, and better?)

Well, in C, if an expression is an integer constant expression, then in 
certain contexts its folded value needs to be known for checking lots of 
other constraints that should be diagnosed in the front end, such as on 
bit-field widths and array sizes - and then it needs to be known for 
layout so that sizeof and offsetof (which can be used in other integer 
constant expressions) can be computed.  And in other contexts (pointers 
conditional expressions), the value of an integer constant expression 
affects the type of the containing expression, so types cannot be 
determined without folding (with consequent effects on other diagnostics).  
And then certain expressions that aren't integer constant expressions but 
can be folded to them in fact get used (e.g. in the Linux kernel) in 
contexts requiring integer constant expressions, so also need folding.  
And then a wider range of expressions need folding in static initializers 
so the front end can diagnose whether an initializer is valid or not, 
while allowing extensions that again are used in practice (though there's 
a possibility such diagnostics for initializers could be left until after 
some middle-end optimization).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-08-04 15:04                             ` Jason Merrill
@ 2015-08-04 15:58                               ` Jeff Law
  0 siblings, 0 replies; 34+ messages in thread
From: Jeff Law @ 2015-08-04 15:58 UTC (permalink / raw)
  To: Jason Merrill, Martin Sebor, Joseph Myers
  Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

On 08/04/2015 09:04 AM, Jason Merrill wrote:
>
> This is largely historical baggage, I think, from days where computers
> had less memory and we were trying to do as much processing as possible
> immediately.
Right.  Also note the early folding was from a time before we had the 
gimple optimization pipeline -- the only significant optimizations we 
did on trees was the (excessive) fold-const.c stuff.


>
> The c++-delayed-folding branch delays folding the ?: expression until
> the end of the function, at which point we can see better what context
> the function is being used in, which could simplify your patch.
Right.   And that's a design direction we want to take with folding in 
general -- delay it until the transition to gimple/ssa.  That way what 
we get out of the front-ends looks much more like the original source.

jeff

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-07-29  7:27                       ` Jason Merrill
  2015-07-29 19:07                         ` Martin Sebor
@ 2015-08-28 20:10                         ` Martin Sebor
  2015-08-28 20:49                           ` Joseph Myers
  1 sibling, 1 reply; 34+ messages in thread
From: Martin Sebor @ 2015-08-28 20:10 UTC (permalink / raw)
  To: Jason Merrill, Joseph Myers; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

[-- Attachment #1: Type: text/plain, Size: 2707 bytes --]

>> The second question is about your suggestion to consolidate the code
>> into mark_rvalue_use. The problem I'm running into there is that
>> mark_rvalue_use is called for calls to builtins as well as for other
>> uses and doesn't have enough context to tell one from the other.
>
> Ah, true.  But special-casing call uses is still fewer places than
> special-casing all non-call uses.

Sorry it's taken me so long to get back to this.

Changing the patch to issue the diagnostic in mark_rvalue_use instead
of anywhere in expr.c or call.c caused a false positive for calls to
builtin functions, and a number of false negatives (e.g., for the
address-of expression and for reinterpret_cast<F&>(builtin)).

To avoid the false positive I added a new argument to both
mark_rvalue_use and decay_conversion (which calls mark_rvalue_use
when called from build_addr_func) to avoid diagnosing calls to
builtins.

To avoid the false negatives, we still need to retain some calls to
cp_reject_gcc_builtin from functions other than mark_rvalue_use.

I also removed the DECL_IS_GCC_BUILTIN macro introduced in the first
patch and replaced its uses with a somewhat simplified expression
that's the same between the C and C++ front ends.

Finally, I merged the c_ and cp_reject_gcc_builtin functions into
a single reject_gcc_builtin function and simplified the logic to
avoid testing for operators new and delete by name. I removed from
the C++ version the checks for the type substitution flags since
(IIUC) they don't come into play here (all uses of the builtins
can be diagnosed, regardless of the type substitution context).

I ran into one regression in the gcc.dg/lto/pr54702_1.c test.
The file takes the address of malloc without declaring it, and
after calling it first. The code is invalid but GCC compiles it
due to a bug. I raised it in c/67386 -- missing diagnostic on
a use of an undeclared function, and suppressed the error by
adding a declaration to the test. I mention it here because
the error that GCC issues otherwise (without the declaration)
is one about __builtin_malloc not being directly called. I'm
sure the error will change to "'malloc' undeclared" once PR67386
is fixed but until then, in this case, the error is rather cryptic.
I haven't spent too much time trying to fix it (it seems to have
to do with the undeclared malloc being treated as a builtin without
a library fallback).

Let me know if this is closer to what you were suggesting or if
you would like to see some other changes (or prefer the original
approach).

I tested the patch by bootstrapping on 86_64 and on powerpc64le
and running tests on the latter. I see the following difference
in the test results

Thanks
Martin




[-- Attachment #2: gcc-66516-5.patch --]
[-- Type: text/x-patch, Size: 26647 bytes --]

gcc/ChangeLog
2015-08-27  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* doc/extend.texi (Other Builtins): Document when the address
	of a builtin function can be taken.

gcc/c-family/ChangeLog
2015-08-27  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* c-common.h (reject_gcc_builtin): Declare new function.
	* c-common.c (reject_gcc_builtin): Define it.

gcc/c/ChangeLog
2015-08-27  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* c/c-typeck.c (convert_arguments, parser_build_unary_op)
	(build_conditional_expr, c_cast_expr, convert_for_assignment)
	(build_binary_op, _objc_common_truthvalue_conversion): Call
	reject_gcc_builtin.

gcc/cp/ChangeLog
2015-08-27  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* cp/cp-tree.h (mark_rvalue_use, decay_conversion): Add new
	argument(s).
	* cp/expr.c (mark_rvalue_use): Use new argument.
	* cp/call.c (build_addr_func): Call decay_conversion with new
	argument.
	* cp/pt.c (convert_template_argument): Call reject_gcc_builtin.
	* cp/typeck.c (decay_conversion): Use new argument.

gcc/testsuite/ChangeLog
2015-08-27  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* g++.dg/addr_builtin-1.C: New test.
	* gcc.dg/addr_builtin-1.c: New test.
	* gcc.dg/lto/pr54702_1.c: Declare malloc before taking its address.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 7691035..8fda350 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -12882,4 +12882,38 @@ pointer_to_zero_sized_aggr_p (tree t)
   return (TYPE_SIZE (t) && integer_zerop (TYPE_SIZE (t)));
 }

+/* For an EXPR of a FUNCTION_TYPE that references a GCC built-in function
+   with no library fallback or for an ADDR_EXPR whose operand is such type
+   issues an error pointing to the location LOC.
+   Returns true when the expression has been diagnosed and false
+   otherwise.  */
+bool
+reject_gcc_builtin (const_tree expr, location_t loc /* = UNKNOWN_LOCATION */)
+{
+  if (TREE_CODE (expr) == ADDR_EXPR)
+    expr = TREE_OPERAND (expr, 0);
+
+  if (TREE_TYPE (expr)
+      && TREE_CODE (TREE_TYPE (expr)) == FUNCTION_TYPE
+      && DECL_P (expr)
+      /* The intersection of DECL_BUILT_IN and DECL_IS_BUILTIN avoids
+	 false positives for user-declared built-ins such as abs or
+	 strlen, and for C++ operators new and delete.  */
+      && DECL_BUILT_IN (expr)
+      && DECL_IS_BUILTIN (expr)
+      && !DECL_ASSEMBLER_NAME_SET_P (expr))
+    {
+      if (loc == UNKNOWN_LOCATION)
+	loc = EXPR_LOC_OR_LOC (expr, input_location);
+
+      /* Reject arguments that are builtin functions with
+	 no library fallback.  */
+      error_at (loc, "builtin function %qE must be directly called", expr);
+
+      return true;
+    }
+
+  return false;
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index be63cd2..559a561 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1437,5 +1437,6 @@ extern bool contains_cilk_spawn_stmt (tree);
 extern tree cilk_for_number_of_iterations (tree);
 extern bool check_no_cilk (tree, const char *, const char *,
 		           location_t loc = UNKNOWN_LOCATION);
+extern bool reject_gcc_builtin (const_tree, location_t = UNKNOWN_LOCATION);

 #endif /* ! GCC_C_COMMON_H */
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index e8c8189..d3ea1c0 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -3339,6 +3339,10 @@ convert_arguments (location_t loc, vec<location_t> arg_loc, tree typelist,
 	  error (invalid_func_diag);
 	  return -1;
 	}
+      else if (TREE_CODE (val) == ADDR_EXPR && reject_gcc_builtin (val))
+	{
+	  return -1;
+	}
       else
 	/* Convert `short' and `char' to full-size `int'.  */
 	parmval = default_conversion (val);
@@ -3376,12 +3380,20 @@ parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg)
 {
   struct c_expr result;

-  result.value = build_unary_op (loc, code, arg.value, 0);
   result.original_code = code;
   result.original_type = NULL;

+  if (reject_gcc_builtin (arg.value))
+    {
+      result.value = error_mark_node;
+    }
+  else
+    {
+      result.value = build_unary_op (loc, code, arg.value, 0);
+
       if (TREE_OVERFLOW_P (result.value) && !TREE_OVERFLOW_P (arg.value))
         overflow_warning (loc, result.value);
+    }

   return result;
 }
@@ -4484,6 +4496,12 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
   type2 = TREE_TYPE (op2);
   code2 = TREE_CODE (type2);

+  if (code1 == POINTER_TYPE && reject_gcc_builtin (op1))
+    return error_mark_node;
+
+  if (code2 == POINTER_TYPE && reject_gcc_builtin (op2))
+    return error_mark_node;
+
   /* C90 does not permit non-lvalue arrays in conditional expressions.
      In C99 they will be pointers by now.  */
   if (code1 == ARRAY_TYPE || code2 == ARRAY_TYPE)
@@ -5222,6 +5240,10 @@ c_cast_expr (location_t loc, struct c_type_name *type_name, tree expr)
   type = groktypename (type_name, &type_expr, &type_expr_const);
   warn_strict_prototypes = saved_wsp;

+  if (TREE_CODE (expr) == ADDR_EXPR && !VOID_TYPE_P (type)
+      && reject_gcc_builtin (expr))
+    return error_mark_node;
+
   ret = build_c_cast (loc, type, expr);
   if (type_expr)
     {
@@ -5861,6 +5883,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
   rhs = require_complete_type (rhs);
   if (rhs == error_mark_node)
     return error_mark_node;
+
+  if (coder == POINTER_TYPE && reject_gcc_builtin (rhs))
+    return error_mark_node;
+
   /* A non-reference type can convert to a reference.  This handles
      va_start, va_copy and possibly port built-ins.  */
   if (codel == REFERENCE_TYPE && coder != REFERENCE_TYPE)
@@ -10350,6 +10376,14 @@ build_binary_op (location_t location, enum tree_code code,
   if (code0 == ERROR_MARK || code1 == ERROR_MARK)
     return error_mark_node;

+  if (code0 == POINTER_TYPE
+      && reject_gcc_builtin (op0, EXPR_LOCATION (orig_op0)))
+    return error_mark_node;
+
+  if (code1 == POINTER_TYPE
+      && reject_gcc_builtin (op1, EXPR_LOCATION (orig_op1)))
+    return error_mark_node;
+
   if ((invalid_op_diag
        = targetm.invalid_binary_op (code, type0, type1)))
     {
@@ -11330,6 +11364,11 @@ c_objc_common_truthvalue_conversion (location_t location, tree expr)
       error_at (location, "void value not ignored as it ought to be");
       return error_mark_node;

+    case POINTER_TYPE:
+      if (reject_gcc_builtin (expr))
+	return error_mark_node;
+      break;
+
     case FUNCTION_TYPE:
       gcc_unreachable ();

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 8d4a9e2..c3dd7f78 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -288,7 +288,7 @@ build_addr_func (tree function, tsubst_flags_t complain)
       function = build_address (function);
     }
   else
-    function = decay_conversion (function, complain);
+    function = decay_conversion (function, complain, false);

   return function;
 }
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 4dee60c..784a616 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5787,7 +5787,9 @@ extern tree create_try_catch_expr               (tree, tree);

 /* in expr.c */
 extern tree cplus_expand_constant		(tree);
-extern tree mark_rvalue_use			(tree);
+extern tree mark_rvalue_use			(tree,
+                                                 location_t = UNKNOWN_LOCATION,
+                                                 bool = true);
 extern tree mark_lvalue_use			(tree);
 extern tree mark_type_use			(tree);
 extern void mark_exp_read			(tree);
@@ -6461,7 +6463,9 @@ extern tree cxx_alignas_expr                    (tree);
 extern tree cxx_sizeof_nowarn                   (tree);
 extern tree is_bitfield_expr_with_lowered_type  (const_tree);
 extern tree unlowered_expr_type                 (const_tree);
-extern tree decay_conversion			(tree, tsubst_flags_t);
+extern tree decay_conversion			(tree,
+                                                 tsubst_flags_t,
+                                                 bool = true);
 extern tree build_class_member_access_expr      (tree, tree, tree, bool,
 						 tsubst_flags_t);
 extern tree finish_class_member_access_expr     (tree, tree, bool,
diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
index 6d2d658..21354dd 100644
--- a/gcc/cp/expr.c
+++ b/gcc/cp/expr.c
@@ -91,18 +91,24 @@ cplus_expand_constant (tree cst)
   return cst;
 }

-/* Called whenever an expression is used
-   in a rvalue context.  */
-
+/* Called whenever an expression is used in an rvalue context.  DIAG
+   is true if the expression should be checked to make sure it doesn't
+   make it possible to obtain the address of a GCC builtin function
+   with no library fallback (or any of its bits, such as in a conversion
+   to bool).  */
 tree
-mark_rvalue_use (tree expr)
+mark_rvalue_use (tree expr,
+		 location_t loc /* = UNKNOWN_LOCATION */,
+		 bool diag /* = true */)
 {
+  if (diag && reject_gcc_builtin (expr, loc))
+    return error_mark_node;
+
   mark_exp_read (expr);
   return expr;
 }

-/* Called whenever an expression is used
-   in a lvalue context.  */
+/* Called whenever an expression is used in an lvalue context.  */

 tree
 mark_lvalue_use (tree expr)
@@ -159,4 +165,3 @@ mark_exp_read (tree exp)
       break;
     }
 }
-
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index fb7b9d2..ec32c5a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -7199,6 +7199,18 @@ convert_template_argument (tree parm,
       else if (val == error_mark_node && (complain & tf_error))
 	error ("could not convert template argument %qE to %qT",  orig_arg, t);

+      if (INDIRECT_REF_P (val))
+        {
+          /* Reject template arguments that are references to built-in
+             functions with no library fallbacks.  */
+          const_tree inner = TREE_OPERAND (val, 0);
+          if (TREE_CODE (TREE_TYPE (inner)) == REFERENCE_TYPE
+              && TREE_CODE (TREE_TYPE (TREE_TYPE (inner))) == FUNCTION_TYPE
+              && TREE_CODE (TREE_TYPE (inner)) == REFERENCE_TYPE
+              && reject_gcc_builtin (TREE_OPERAND (inner, 0)))
+              return error_mark_node;
+        }
+
       if (TREE_CODE (val) == SCOPE_REF)
 	{
 	  /* Strip typedefs from the SCOPE_REF.  */
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 83fd34c..42e8cf8 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -1911,7 +1911,9 @@ unlowered_expr_type (const_tree exp)
    that the return value is no longer an lvalue.  */

 tree
-decay_conversion (tree exp, tsubst_flags_t complain)
+decay_conversion (tree exp,
+		  tsubst_flags_t complain,
+		  bool diag /* = true */)
 {
   tree type;
   enum tree_code code;
@@ -1921,7 +1923,7 @@ decay_conversion (tree exp, tsubst_flags_t complain)
   if (type == error_mark_node)
     return error_mark_node;

-  exp = mark_rvalue_use (exp);
+  exp = mark_rvalue_use (exp, loc, diag);

   exp = resolve_nondeduced_context (exp);
   if (type_unknown_p (exp))
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index dba8b43..23e6a76 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -10316,14 +10316,22 @@ recommend general use of these functions.

 The remaining functions are provided for optimization purposes.

+With the exception of built-ins that have library equivalents such as
+the standard C library functions discussed below, or that expand to
+library calls, GCC built-in functions are always expanded inline and
+thus do not have corresponding entry points and their address cannot
+be obtained.  Attempting to use them in an expression other than
+a function call results in a compile-time error.
+
 @opindex fno-builtin
 GCC includes built-in versions of many of the functions in the standard
-C library.  The versions prefixed with @code{__builtin_} are always
-treated as having the same meaning as the C library function even if you
-specify the @option{-fno-builtin} option.  (@pxref{C Dialect Options})
-Many of these functions are only optimized in certain cases; if they are
-not optimized in a particular case, a call to the library function is
-emitted.
+C library.  These functions come in two forms: one whose names start with
+the @code{__builtin_} prefix, and the other without.  Both forms have the
+same type (including prototype), the same address (when their address is
+taken), and the same meaning as the C library functions even if you specify
+the @option{-fno-builtin} option @pxref{C Dialect Options}).  Many of these
+functions are only optimized in certain cases; if they are not optimized in
+a particular case, a call to the library function is emitted.

 @opindex ansi
 @opindex std
diff --git a/gcc/testsuite/g++.dg/addr_builtin-1.C b/gcc/testsuite/g++.dg/addr_builtin-1.C
new file mode 100644
index 0000000..6848260
--- /dev/null
+++ b/gcc/testsuite/g++.dg/addr_builtin-1.C
@@ -0,0 +1,272 @@
+// PR66516 - missing diagnostic on taking the address of a builtin function
+// { dg-do compile }
+// { dg-options "-Wno-error=pedantic" }
+
+namespace std {
+  // Define type_info type to be able to use typeid in tests without
+  // having to include <typeinfo>.
+  struct type_info {
+      const char *name_;
+
+      explicit type_info (const char *s): name_ (s) { }
+      const char* name() const { return name_; }
+  };
+}
+
+// Extern "C" since builtin functions used in tests have C linkage.
+extern "C" {
+
+typedef void (F)();
+typedef __UINTPTR_TYPE__ uintptr_t;
+
+// Utility function to test passing built-in functions as an ordinary
+// argument and via the ellipsis.
+static void func_arg (F*, ...);
+
+}   // extern "C"
+
+
+// Utility templates to test specializing templates on pointers and
+// references to built-in functions.
+template <F*> struct TestPointer { };
+template <F&> struct TestReference { };
+
+// Utility function with which, along with the built-in function,
+// to instantiate the C98 multi-parameter or C11 variadic tempates
+// below.
+void f () { }
+
+#if 201103 <= __cplusplus
+
+template <F*...> struct TestPointers { };
+template <F&...> struct TestReferences { };
+
+#else
+
+template <F* = &f, F* = &f> struct TestPointers { };
+template <F& = f, F& = f> struct TestReferences { };
+
+#endif
+
+static F* test_taking_address_of_gcc_builtin ()
+{
+  enum UINTPTR_E { e = ~(uintptr_t)0 };
+
+  F *p;
+  void *q;
+  uintptr_t a;
+
+  __builtin_trap ();                           // okay
+  (void)__builtin_trap;                        // okay
+  __builtin_trap;                              // okay (if pointless)
+
+  {
+    typedef __typeof__ (__builtin_trap) F;     // okay
+  }
+
+#if 201103 <= __cplusplus
+  {
+    typedef decltype (__builtin_trap) F;       // okay
+
+    a = noexcept (&__builtin_trap);
+  }
+#endif
+
+  // Address and indirection operators.
+  p = &__builtin_trap;                         // { dg-error "builtin" }
+  p = *__builtin_trap;                         // { dg-error "builtin" }
+
+  // Unary NOT.
+  a = !__builtin_trap;                         // { dg-error "builtin|unary" }
+
+  // Casts.
+  p = (F*)__builtin_trap;                      // { dg-error "builtin" }
+
+  p = &(F&)__builtin_trap;                     // { dg-error "builtin" }
+
+  p = &reinterpret_cast<F&>(__builtin_trap);   // { dg-error "builtin" }
+  p = &static_cast<F&>(__builtin_trap);        // { dg-error "builtin" }
+
+  p = reinterpret_cast<F*>(__builtin_trap);    // { dg-error "builtin" }
+  p = static_cast<F*>(__builtin_trap);         // { dg-error "builtin" }
+
+  a = reinterpret_cast<uintptr_t>(__builtin_trap);  // { dg-error "builtin" }
+
+  // Expect a diagnostic for an invalid static_cast of a function
+  // to uintptr_t, rather than one for the argument being a built-in
+  // function, since the former is more relevant than the latter.
+  a = static_cast<uintptr_t>(__builtin_trap);       // { dg-error "invalid" }
+
+  a = reinterpret_cast<UINTPTR_E>(__builtin_trap);  // { dg-error "builtin" }
+  a = static_cast<UINTPTR_E>(__builtin_trap);       // { dg-error "builtin" }
+
+  // Additive operator.  Ill-formed but allowed with -fpermissive.
+  p = __builtin_trap + 0;            // { dg-error "builtin" }
+  p = __builtin_trap - 0;            // { dg-error "builtin" }
+  a = __builtin_trap - p;            // { dg-error "builtin" }
+  a = p - __builtin_trap;            // { dg-error "builtin" }
+
+  // Relational operators.  Ill-formed but allowed with -fpermissive.
+  a = __builtin_trap < p;            // { dg-error "builtin" }
+  a = p < __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap > p;            // { dg-error "builtin" }
+  a = p > __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap > p;            // { dg-error "builtin" }
+  a = p > __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  // Equality operators.
+  a = __builtin_trap == p;           // { dg-error "builtin" }
+  a = p == __builtin_trap;           // { dg-error "builtin" }
+  a = __builtin_trap != p;           // { dg-error "builtin" }
+  a = p != __builtin_trap;           // { dg-error "builtin" }
+
+  // Logical AND and OR.
+  a = __builtin_trap && p;           // { dg-error "builtin" }
+  a = p && __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap || p;           // { dg-error "builtin" }
+  a = p || __builtin_trap;           // { dg-error "builtin" }
+
+  // Conditional operator.
+  a = __builtin_trap ? 1 : 0;        // { dg-error "builtin" }
+  p = a ? __builtin_trap : 0;        // { dg-error "builtin" }
+  p = a ? 0 : __builtin_trap;        // { dg-error "builtin" }
+
+  // Assignment operator.
+  p = __builtin_trap;                // { dg-error "builtin" }
+
+  // Passing as an argument.
+  func_arg (__builtin_trap);         // { dg-error "builtin" }
+  func_arg (&__builtin_trap);        // { dg-error "builtin" }
+  func_arg (*__builtin_trap);        // { dg-error "builtin" }
+
+  // Passing through ellipsis.
+  func_arg (0, __builtin_trap);      // { dg-error "builtin" }
+  func_arg (0, &__builtin_trap);     // { dg-error "builtin" }
+  func_arg (0, *__builtin_trap);     // { dg-error "builtin" }
+
+  {
+    // Template specialization.
+    TestPointer<__builtin_trap> tp;         // { dg-error "builtin" }
+    TestReference<__builtin_trap> tr;       // { dg-error "builtin" }
+
+    TestPointers<__builtin_trap> tp1;       // { dg-error "builtin" }
+    TestReferences<__builtin_trap> tr1;     // { dg-error "builtin" }
+
+    TestPointers<f, __builtin_trap> tp2;    // { dg-error "builtin" }
+    TestReferences<f, __builtin_trap> tr2;  // { dg-error "builtin" }
+
+    TestPointers<__builtin_trap, f> tp3;    // { dg-error "builtin" }
+    TestReferences<__builtin_trap, f> tr3;  // { dg-error "builtin" }
+  }
+
+  try {
+      throw __builtin_trap;                 // { dg-error "builtin" }
+  }
+  catch (F) { }
+
+  return __builtin_trap;                    // { dg-error "builtin" }
+
+  (void)a;
+  (void)p;
+  (void)q;
+}
+
+// Make sure operators new and delete don't trigger false positives
+// (they return true from DECL_IS_BUILTIN(DECL) -- see tree.h).
+void test_taking_address_of_op_new_and_delete ()
+{
+  typedef __SIZE_TYPE__ size_t;
+
+  typedef void* (OpNew) (size_t);
+  typedef void (OpDelete) (void*);
+
+  OpNew &newr = operator new;
+  OpNew &newra = operator new[];
+  OpNew *newp = &operator new;
+  newp = &operator new[];
+
+  OpDelete &delr = operator delete;
+  OpDelete &delra = operator delete[];
+  OpDelete *delp = &operator delete;
+  delp = &operator delete[];
+
+  (void)newr;
+  (void)newp;
+  (void)delr;
+  (void)delp;
+}
+
+// Helper declaration to verify that it's possible to take the address
+// of a user-declared function that's also a GCC built-in.
+extern int abs (int);
+
+typedef __SIZE_TYPE__ size_t;
+extern size_t strlen (const char*);
+
+// Creating a reference to or taking the address of a built-in with
+// a library "fallback" must be allowed.
+void test_taking_address_of_library_builtin ()
+{
+  {
+    typedef int F (int);
+
+    F &r1 = __builtin_abs;
+    F &r2 = *__builtin_abs;
+    F *p = __builtin_abs;
+    p = &__builtin_abs;
+    p = *__builtin_abs;
+    (void)p;
+    (void)r1;
+    (void)r2;
+  }
+
+  {
+    typedef int F (int);
+
+    F &r1 = abs;
+    F &r2 = *abs;
+    F *p = abs;
+    p = &abs;
+    p = *abs;
+    (void)p;
+    (void)r1;
+    (void)r2;
+  }
+
+  {
+    typedef __SIZE_TYPE__ size_t;
+    typedef size_t F (const char*);
+    F &r1 = __builtin_strlen;
+    F &r2 = *__builtin_strlen;
+    F *p = __builtin_strlen;
+    p = &__builtin_strlen;
+    p = *__builtin_strlen;
+    (void)p;
+    (void)r1;
+    (void)r2;
+  }
+
+  {
+    typedef size_t F (const char*);
+    F &r1 = strlen;
+    F &r2 = *strlen;
+    F *p = strlen;
+    p = &strlen;
+    p = *strlen;
+    (void)p;
+    (void)r1;
+    (void)r2;
+  }
+}
diff --git a/gcc/testsuite/gcc.dg/addr_builtin-1.c b/gcc/testsuite/gcc.dg/addr_builtin-1.c
new file mode 100644
index 0000000..03394f2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/addr_builtin-1.c
@@ -0,0 +1,155 @@
+// PR66516 - missing diagnostic on taking the address of a builtin function
+// { dg-do compile }
+// { dg-options "-Wno-error=pedantic" }
+
+typedef void (F)(void);
+typedef __UINTPTR_TYPE__ uintptr_t;
+
+// Utility function to test passing built-in functions as an ordinary
+// argument and via the ellipsis.
+static void func_arg (F *p, ...) { (void)p; }
+
+static F* test_taking_address_of_gcc_builtin (void)
+{
+  enum UINTPTR_E { e = ~(uintptr_t)0 };
+
+  F *p;
+  void *q;
+  uintptr_t a;
+
+  // Call, cast to void, and id are allowed.
+  __builtin_trap ();
+  (void)__builtin_trap;
+  __builtin_trap;
+
+  {
+    typedef __typeof__ (__builtin_trap) F;     // okay
+  }
+
+  // Address and indirection operators.
+  p = &__builtin_trap;               // { dg-error "builtin" }
+  p = *__builtin_trap;               // { dg-error "builtin" }
+
+  // Unary NOT.
+  a = !__builtin_trap;               // { dg-error "builtin" }
+
+  // Sizeof and _Alignof are disallowed by C but allowed by GCC
+  // and there's no reason to reject built-ins as operands since
+  // doing so doesn't yield their address.
+  a = sizeof __builtin_trap;
+  a = _Alignof __builtin_trap;
+
+  // Casts.
+  p = (F*)__builtin_trap;            // { dg-error "builtin" }
+  a = (uintptr_t)__builtin_trap;     // { dg-error "builtin" }
+
+  // Additive operator.
+  p = __builtin_trap + 0;            // { dg-error "builtin" }
+  p = __builtin_trap - 0;            // { dg-error "builtin" }
+  a = __builtin_trap - p;            // { dg-error "builtin" }
+  a = p - __builtin_trap;            // { dg-error "builtin" }
+
+  // Relational operators.
+  a = __builtin_trap < p;            // { dg-error "builtin" }
+  a = p < __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap > p;            // { dg-error "builtin" }
+  a = p > __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap > p;            // { dg-error "builtin" }
+  a = p > __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  // Equality operators.
+  a = __builtin_trap == p;           // { dg-error "builtin" }
+  a = p == __builtin_trap;           // { dg-error "builtin" }
+  a = __builtin_trap != p;           // { dg-error "builtin" }
+  a = p != __builtin_trap;           // { dg-error "builtin" }
+
+  // Logical AND and OR.
+  a = __builtin_trap && p;           // { dg-error "builtin" }
+  a = p && __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap || p;           // { dg-error "builtin" }
+  a = p || __builtin_trap;           // { dg-error "builtin" }
+
+  // Conditional operator.
+  a = __builtin_trap ? 1 : 0;        // { dg-error "builtin" }
+  p = a ? __builtin_trap : 0;        // { dg-error "builtin" }
+  p = a ? 0 : __builtin_trap;        // { dg-error "builtin" }
+
+  // Assignment operator.
+  p = __builtin_trap;                // { dg-error "builtin" }
+
+  q = __builtin_trap;                // { dg-error "builtin" }
+  a = __builtin_trap;                // { dg-error "builtin" }
+
+  // Passing as an argument.
+  func_arg (__builtin_trap);         // { dg-error "builtin" }
+
+  // Passing through the ellipsis.
+  func_arg (0, __builtin_trap);      // { dg-error "builtin" }
+
+  // Return statement.
+  return __builtin_trap;             // { dg-error "builtin" }
+
+  (void)a;
+  (void)p;
+  (void)q;
+}
+
+// Helper declaration to verify that it's possible to take the address
+// of a user-declared function that's also a GCC built-in.
+extern int abs (int);
+
+extern __SIZE_TYPE__ strlen (const char*);
+
+// Taking the address of a builtin with a library "fallback" must be
+// allowed.
+void test_taking_address_of_library_builtin (void)
+{
+  {
+    typedef int F (int);
+
+    // compute the address of libc's abs (all expressions are valid)
+    F *p = __builtin_abs;
+    p = &__builtin_abs;
+    p = *__builtin_abs;
+    p = abs;
+    p = &abs;
+    p = *abs;
+    (void)p;
+  }
+
+  {
+    typedef __SIZE_TYPE__ size_t;
+    typedef size_t F (const char*);
+
+    // compute the address of libc's strlen
+    F *p = __builtin_strlen;
+    p = &__builtin_strlen;
+    p = *__builtin_strlen;
+
+    p = strlen;
+    p = &strlen;
+    p = *strlen;
+    (void)p;
+  }
+
+  {
+    typedef int F (int);
+
+    // compute the address of libc's isxxx functions
+    F *p = __builtin_isalnum;
+    p = &__builtin_isalpha;
+    p = *__builtin_iscntrl;
+  }
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr54702_1.c b/gcc/testsuite/gcc.dg/lto/pr54702_1.c
index 2afb0fb..4e8f06d 100644
--- a/gcc/testsuite/gcc.dg/lto/pr54702_1.c
+++ b/gcc/testsuite/gcc.dg/lto/pr54702_1.c
@@ -7,6 +7,10 @@ void f2 ()
   int *a = f1 (0);
 }

+/* A function doen't necessarily have to be declared to be called
+   but it must be declared in order for its address to be taken.  */
+extern void* malloc (__SIZE_TYPE__);
+
 int *f1 (j)
 {
   b = malloc (0);

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-08-28 20:10                         ` Martin Sebor
@ 2015-08-28 20:49                           ` Joseph Myers
  2015-08-29  0:57                             ` Martin Sebor
  0 siblings, 1 reply; 34+ messages in thread
From: Joseph Myers @ 2015-08-28 20:49 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Jason Merrill, Jakub Jelinek, Marek Polacek, Gcc Patch List

On Fri, 28 Aug 2015, Martin Sebor wrote:

> I ran into one regression in the gcc.dg/lto/pr54702_1.c test.
> The file takes the address of malloc without declaring it, and
> after calling it first. The code is invalid but GCC compiles it
> due to a bug. I raised it in c/67386 -- missing diagnostic on
> a use of an undeclared function, and suppressed the error by

But that PR isn't a bug - the code is working exactly as it's meant to (an 
implicit declaration acts exactly like an explicit declaration "int func 
();" in the nearest containing scope).  The declaration has an 
incompatible type, it's true, but GCC deliberately allows that with a 
warning.

What if (a) you use a built-in function that returns int, instead of 
malloc, and (b) use -std=gnu89, so the implicit declaration isn't even an 
extension?  Then you have something that's completely valid, including 
taking the address of the implicitly declared function.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-08-28 20:49                           ` Joseph Myers
@ 2015-08-29  0:57                             ` Martin Sebor
  2015-09-01 15:01                               ` Martin Sebor
  0 siblings, 1 reply; 34+ messages in thread
From: Martin Sebor @ 2015-08-29  0:57 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Jason Merrill, Jakub Jelinek, Marek Polacek, Gcc Patch List

On 08/28/2015 02:09 PM, Joseph Myers wrote:
> On Fri, 28 Aug 2015, Martin Sebor wrote:
>
>> I ran into one regression in the gcc.dg/lto/pr54702_1.c test.
>> The file takes the address of malloc without declaring it, and
>> after calling it first. The code is invalid but GCC compiles it
>> due to a bug. I raised it in c/67386 -- missing diagnostic on
>> a use of an undeclared function, and suppressed the error by
>
> But that PR isn't a bug - the code is working exactly as it's meant to (an
> implicit declaration acts exactly like an explicit declaration "int func
> ();" in the nearest containing scope).  The declaration has an
> incompatible type, it's true, but GCC deliberately allows that with a
> warning.
> What if (a) you use a built-in function that returns int, instead of
> malloc, and (b) use -std=gnu89, so the implicit declaration isn't even an
> extension?  Then you have something that's completely valid, including
> taking the address of the implicitly declared function.

In that case the patched GCC issues an error for taking the address
of the undeclared function as the test case below shows.

I was aware of the C90 implicit declaration rule but I interpreted
it as saying that the injected declaration is only in effect for
the call expression. Since no other tests broke, I assumed the one
that did was buggy. Anyway, after testing a few other compilers it
looks like they all also extend the implicit declaration through
the rest of the scope, so the patch will need further tweaking to
allow this corner case.

The problem is that DECL_IS_BUILTIN(expr) returns true for
an implicitly declared builtin function with a library fallback
but false for one that's been declared explicitly. I'll either
have to find some other test to determine that the implicitly
declared function has a fallback or fix whatever is causing
the macro to return the wrong value.

Martin

$ cat t.c && /build/gcc-66516/gcc/xgcc -B /build/gcc-66516/gcc 
-std=gnu89 t.cint (*p)(int);

void foo (void) {
     p = abs;
}

int bar (void) {
     int n = abs (0);
     p = abs;
     return n;
}

t.c: In function ‘foo’:
t.c:4:9: error: ‘abs’ undeclared (first use in this function)
      p = abs;
          ^
t.c:4:9: note: each undeclared identifier is reported only once for each 
function it appears in
t.c: In function ‘bar’:
t.c:9:5: error: builtin function ‘abs’ must be directly called
      p = abs;
      ^

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-08-29  0:57                             ` Martin Sebor
@ 2015-09-01 15:01                               ` Martin Sebor
  2015-09-01 17:29                                 ` Joseph Myers
  0 siblings, 1 reply; 34+ messages in thread
From: Martin Sebor @ 2015-09-01 15:01 UTC (permalink / raw)
  To: Joseph Myers, Jason Merrill; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

[-- Attachment #1: Type: text/plain, Size: 471 bytes --]

Attached is an updated patch that avoids diagnosing taking the address
of implicitly declared library builtins like abs, bootstrapped and
tested on ppc64le with no regressions.

The tweak below was added to reject_gcc_builtin make it possible.
Since the expression is in c-family/c-common.c, the more descriptive
C_DECL_IMPLICIT that exists for this purpose is not available (it's
defined in c/c-tree.h).

+      && (c_dialect_cxx () || !DECL_LANG_FLAG_2 (expr))

Martin

[-- Attachment #2: gcc-66516-6.patch --]
[-- Type: text/x-patch, Size: 28297 bytes --]

gcc/ChangeLog
2015-08-31  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* doc/extend.texi (Other Builtins): Document when the address
	of a builtin function can be taken.

gcc/c-family/ChangeLog
2015-08-31  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* c-common.h (reject_gcc_builtin): Declare new function.
	* c-common.c (reject_gcc_builtin): Define it.

gcc/c/ChangeLog
2015-08-31  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* c/c-typeck.c (convert_arguments, parser_build_unary_op)
	(build_conditional_expr, c_cast_expr, convert_for_assignment)
	(build_binary_op, _objc_common_truthvalue_conversion): Call
	reject_gcc_builtin.

gcc/cp/ChangeLog
2015-08-31  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* cp/cp-tree.h (mark_rvalue_use, decay_conversion): Add new
	argument(s).
	* cp/expr.c (mark_rvalue_use): Use new argument.
	* cp/call.c (build_addr_func): Call decay_conversion with new
	argument.
	* cp/pt.c (convert_template_argument): Call reject_gcc_builtin.
	* cp/typeck.c (decay_conversion): Use new argument.

gcc/testsuite/ChangeLog
2015-08-31  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* g++.dg/addr_builtin-1.C: New test.
	* gcc.dg/addr_builtin-1.c: New test.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 7691035..8cca668 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -12882,4 +12882,41 @@ pointer_to_zero_sized_aggr_p (tree t)
   return (TYPE_SIZE (t) && integer_zerop (TYPE_SIZE (t)));
 }

+/* For an EXPR of a FUNCTION_TYPE that references a GCC built-in function
+   with no library fallback or for an ADDR_EXPR whose operand is such type
+   issues an error pointing to the location LOC.
+   Returns true when the expression has been diagnosed and false
+   otherwise.  */
+bool
+reject_gcc_builtin (const_tree expr, location_t loc /* = UNKNOWN_LOCATION */)
+{
+  if (TREE_CODE (expr) == ADDR_EXPR)
+    expr = TREE_OPERAND (expr, 0);
+
+  if (TREE_TYPE (expr)
+      && TREE_CODE (TREE_TYPE (expr)) == FUNCTION_TYPE
+      && DECL_P (expr)
+      /* The intersection of DECL_BUILT_IN and DECL_IS_BUILTIN avoids
+	 false positives for user-declared built-ins such as abs or
+	 strlen, and for C++ operators new and delete.
+	 In C mode only, DECL_LANG_FLAG_2 avoids false positives for
+	 implicitly declared builtins with library fallbacks.  */
+      && DECL_BUILT_IN (expr)
+      && DECL_IS_BUILTIN (expr)
+      && (c_dialect_cxx () || !DECL_LANG_FLAG_2 (expr))
+      && !DECL_ASSEMBLER_NAME_SET_P (expr))
+    {
+      if (loc == UNKNOWN_LOCATION)
+	loc = EXPR_LOC_OR_LOC (expr, input_location);
+
+      /* Reject arguments that are builtin functions with
+	 no library fallback.  */
+      error_at (loc, "builtin function %qE must be directly called", expr);
+
+      return true;
+    }
+
+  return false;
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index be63cd2..559a561 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1437,5 +1437,6 @@ extern bool contains_cilk_spawn_stmt (tree);
 extern tree cilk_for_number_of_iterations (tree);
 extern bool check_no_cilk (tree, const char *, const char *,
 		           location_t loc = UNKNOWN_LOCATION);
+extern bool reject_gcc_builtin (const_tree, location_t = UNKNOWN_LOCATION);

 #endif /* ! GCC_C_COMMON_H */
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index e8c8189..ca4e3e2 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -3339,6 +3339,10 @@ convert_arguments (location_t loc, vec<location_t> arg_loc, tree typelist,
 	  error (invalid_func_diag);
 	  return -1;
 	}
+      else if (TREE_CODE (val) == ADDR_EXPR && reject_gcc_builtin (val))
+	{
+	  return -1;
+	}
       else
 	/* Convert `short' and `char' to full-size `int'.  */
 	parmval = default_conversion (val);
@@ -3376,12 +3380,20 @@ parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg)
 {
   struct c_expr result;

-  result.value = build_unary_op (loc, code, arg.value, 0);
   result.original_code = code;
   result.original_type = NULL;

+  if (reject_gcc_builtin (arg.value))
+    {
+      result.value = error_mark_node;
+    }
+  else
+    {
+      result.value = build_unary_op (loc, code, arg.value, 0);
+
   if (TREE_OVERFLOW_P (result.value) && !TREE_OVERFLOW_P (arg.value))
     overflow_warning (loc, result.value);
+    }

   return result;
 }
@@ -4484,6 +4496,12 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
   type2 = TREE_TYPE (op2);
   code2 = TREE_CODE (type2);

+  if (code1 == POINTER_TYPE && reject_gcc_builtin (op1))
+    return error_mark_node;
+
+  if (code2 == POINTER_TYPE && reject_gcc_builtin (op2))
+    return error_mark_node;
+
   /* C90 does not permit non-lvalue arrays in conditional expressions.
      In C99 they will be pointers by now.  */
   if (code1 == ARRAY_TYPE || code2 == ARRAY_TYPE)
@@ -5222,6 +5240,10 @@ c_cast_expr (location_t loc, struct c_type_name *type_name, tree expr)
   type = groktypename (type_name, &type_expr, &type_expr_const);
   warn_strict_prototypes = saved_wsp;

+  if (TREE_CODE (expr) == ADDR_EXPR && !VOID_TYPE_P (type)
+      && reject_gcc_builtin (expr))
+    return error_mark_node;
+
   ret = build_c_cast (loc, type, expr);
   if (type_expr)
     {
@@ -5861,6 +5883,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
   rhs = require_complete_type (rhs);
   if (rhs == error_mark_node)
     return error_mark_node;
+
+  if (coder == POINTER_TYPE && reject_gcc_builtin (rhs))
+    return error_mark_node;
+
   /* A non-reference type can convert to a reference.  This handles
      va_start, va_copy and possibly port built-ins.  */
   if (codel == REFERENCE_TYPE && coder != REFERENCE_TYPE)
@@ -10350,6 +10376,14 @@ build_binary_op (location_t location, enum tree_code code,
   if (code0 == ERROR_MARK || code1 == ERROR_MARK)
     return error_mark_node;

+  if (code0 == POINTER_TYPE
+      && reject_gcc_builtin (op0, EXPR_LOCATION (orig_op0)))
+    return error_mark_node;
+
+  if (code1 == POINTER_TYPE
+      && reject_gcc_builtin (op1, EXPR_LOCATION (orig_op1)))
+    return error_mark_node;
+
   if ((invalid_op_diag
        = targetm.invalid_binary_op (code, type0, type1)))
     {
@@ -11330,6 +11364,11 @@ c_objc_common_truthvalue_conversion (location_t location, tree expr)
       error_at (location, "void value not ignored as it ought to be");
       return error_mark_node;

+    case POINTER_TYPE:
+      if (reject_gcc_builtin (expr))
+	return error_mark_node;
+      break;
+
     case FUNCTION_TYPE:
       gcc_unreachable ();

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 8d4a9e2..c3dd7f78 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -288,7 +288,7 @@ build_addr_func (tree function, tsubst_flags_t complain)
       function = build_address (function);
     }
   else
-    function = decay_conversion (function, complain);
+    function = decay_conversion (function, complain, false);

   return function;
 }
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 4dee60c..784a616 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5787,7 +5787,9 @@ extern tree create_try_catch_expr               (tree, tree);

 /* in expr.c */
 extern tree cplus_expand_constant		(tree);
-extern tree mark_rvalue_use			(tree);
+extern tree mark_rvalue_use			(tree,
+                                                 location_t = UNKNOWN_LOCATION,
+                                                 bool = true);
 extern tree mark_lvalue_use			(tree);
 extern tree mark_type_use			(tree);
 extern void mark_exp_read			(tree);
@@ -6461,7 +6463,9 @@ extern tree cxx_alignas_expr                    (tree);
 extern tree cxx_sizeof_nowarn                   (tree);
 extern tree is_bitfield_expr_with_lowered_type  (const_tree);
 extern tree unlowered_expr_type                 (const_tree);
-extern tree decay_conversion			(tree, tsubst_flags_t);
+extern tree decay_conversion			(tree,
+                                                 tsubst_flags_t,
+                                                 bool = true);
 extern tree build_class_member_access_expr      (tree, tree, tree, bool,
 						 tsubst_flags_t);
 extern tree finish_class_member_access_expr     (tree, tree, bool,
diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
index 6d2d658..21354dd 100644
--- a/gcc/cp/expr.c
+++ b/gcc/cp/expr.c
@@ -91,18 +91,24 @@ cplus_expand_constant (tree cst)
   return cst;
 }

-/* Called whenever an expression is used
-   in a rvalue context.  */
-
+/* Called whenever an expression is used in an rvalue context.  DIAG
+   is true if the expression should be checked to make sure it doesn't
+   make it possible to obtain the address of a GCC builtin function
+   with no library fallback (or any of its bits, such as in a conversion
+   to bool).  */
 tree
-mark_rvalue_use (tree expr)
+mark_rvalue_use (tree expr,
+		 location_t loc /* = UNKNOWN_LOCATION */,
+		 bool diag /* = true */)
 {
+  if (diag && reject_gcc_builtin (expr, loc))
+    return error_mark_node;
+
   mark_exp_read (expr);
   return expr;
 }

-/* Called whenever an expression is used
-   in a lvalue context.  */
+/* Called whenever an expression is used in an lvalue context.  */

 tree
 mark_lvalue_use (tree expr)
@@ -159,4 +165,3 @@ mark_exp_read (tree exp)
       break;
     }
 }
-
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index fb7b9d2..ec32c5a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -7199,6 +7199,18 @@ convert_template_argument (tree parm,
       else if (val == error_mark_node && (complain & tf_error))
 	error ("could not convert template argument %qE to %qT",  orig_arg, t);

+      if (INDIRECT_REF_P (val))
+        {
+          /* Reject template arguments that are references to built-in
+             functions with no library fallbacks.  */
+          const_tree inner = TREE_OPERAND (val, 0);
+          if (TREE_CODE (TREE_TYPE (inner)) == REFERENCE_TYPE
+              && TREE_CODE (TREE_TYPE (TREE_TYPE (inner))) == FUNCTION_TYPE
+              && TREE_CODE (TREE_TYPE (inner)) == REFERENCE_TYPE
+              && reject_gcc_builtin (TREE_OPERAND (inner, 0)))
+              return error_mark_node;
+        }
+
       if (TREE_CODE (val) == SCOPE_REF)
 	{
 	  /* Strip typedefs from the SCOPE_REF.  */
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 83fd34c..42e8cf8 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -1911,7 +1911,9 @@ unlowered_expr_type (const_tree exp)
    that the return value is no longer an lvalue.  */

 tree
-decay_conversion (tree exp, tsubst_flags_t complain)
+decay_conversion (tree exp,
+		  tsubst_flags_t complain,
+		  bool diag /* = true */)
 {
   tree type;
   enum tree_code code;
@@ -1921,7 +1923,7 @@ decay_conversion (tree exp, tsubst_flags_t complain)
   if (type == error_mark_node)
     return error_mark_node;

-  exp = mark_rvalue_use (exp);
+  exp = mark_rvalue_use (exp, loc, diag);

   exp = resolve_nondeduced_context (exp);
   if (type_unknown_p (exp))
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index dba8b43..23e6a76 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -10316,14 +10316,22 @@ recommend general use of these functions.

 The remaining functions are provided for optimization purposes.

+With the exception of built-ins that have library equivalents such as
+the standard C library functions discussed below, or that expand to
+library calls, GCC built-in functions are always expanded inline and
+thus do not have corresponding entry points and their address cannot
+be obtained.  Attempting to use them in an expression other than
+a function call results in a compile-time error.
+
 @opindex fno-builtin
 GCC includes built-in versions of many of the functions in the standard
-C library.  The versions prefixed with @code{__builtin_} are always
-treated as having the same meaning as the C library function even if you
-specify the @option{-fno-builtin} option.  (@pxref{C Dialect Options})
-Many of these functions are only optimized in certain cases; if they are
-not optimized in a particular case, a call to the library function is
-emitted.
+C library.  These functions come in two forms: one whose names start with
+the @code{__builtin_} prefix, and the other without.  Both forms have the
+same type (including prototype), the same address (when their address is
+taken), and the same meaning as the C library functions even if you specify
+the @option{-fno-builtin} option @pxref{C Dialect Options}).  Many of these
+functions are only optimized in certain cases; if they are not optimized in
+a particular case, a call to the library function is emitted.

 @opindex ansi
 @opindex std
diff --git a/gcc/testsuite/g++.dg/addr_builtin-1.C b/gcc/testsuite/g++.dg/addr_builtin-1.C
new file mode 100644
index 0000000..6848260
--- /dev/null
+++ b/gcc/testsuite/g++.dg/addr_builtin-1.C
@@ -0,0 +1,272 @@
+// PR66516 - missing diagnostic on taking the address of a builtin function
+// { dg-do compile }
+// { dg-options "-Wno-error=pedantic" }
+
+namespace std {
+  // Define type_info type to be able to use typeid in tests without
+  // having to include <typeinfo>.
+  struct type_info {
+      const char *name_;
+
+      explicit type_info (const char *s): name_ (s) { }
+      const char* name() const { return name_; }
+  };
+}
+
+// Extern "C" since builtin functions used in tests have C linkage.
+extern "C" {
+
+typedef void (F)();
+typedef __UINTPTR_TYPE__ uintptr_t;
+
+// Utility function to test passing built-in functions as an ordinary
+// argument and via the ellipsis.
+static void func_arg (F*, ...);
+
+}   // extern "C"
+
+
+// Utility templates to test specializing templates on pointers and
+// references to built-in functions.
+template <F*> struct TestPointer { };
+template <F&> struct TestReference { };
+
+// Utility function with which, along with the built-in function,
+// to instantiate the C98 multi-parameter or C11 variadic tempates
+// below.
+void f () { }
+
+#if 201103 <= __cplusplus
+
+template <F*...> struct TestPointers { };
+template <F&...> struct TestReferences { };
+
+#else
+
+template <F* = &f, F* = &f> struct TestPointers { };
+template <F& = f, F& = f> struct TestReferences { };
+
+#endif
+
+static F* test_taking_address_of_gcc_builtin ()
+{
+  enum UINTPTR_E { e = ~(uintptr_t)0 };
+
+  F *p;
+  void *q;
+  uintptr_t a;
+
+  __builtin_trap ();                           // okay
+  (void)__builtin_trap;                        // okay
+  __builtin_trap;                              // okay (if pointless)
+
+  {
+    typedef __typeof__ (__builtin_trap) F;     // okay
+  }
+
+#if 201103 <= __cplusplus
+  {
+    typedef decltype (__builtin_trap) F;       // okay
+
+    a = noexcept (&__builtin_trap);
+  }
+#endif
+
+  // Address and indirection operators.
+  p = &__builtin_trap;                         // { dg-error "builtin" }
+  p = *__builtin_trap;                         // { dg-error "builtin" }
+
+  // Unary NOT.
+  a = !__builtin_trap;                         // { dg-error "builtin|unary" }
+
+  // Casts.
+  p = (F*)__builtin_trap;                      // { dg-error "builtin" }
+
+  p = &(F&)__builtin_trap;                     // { dg-error "builtin" }
+
+  p = &reinterpret_cast<F&>(__builtin_trap);   // { dg-error "builtin" }
+  p = &static_cast<F&>(__builtin_trap);        // { dg-error "builtin" }
+
+  p = reinterpret_cast<F*>(__builtin_trap);    // { dg-error "builtin" }
+  p = static_cast<F*>(__builtin_trap);         // { dg-error "builtin" }
+
+  a = reinterpret_cast<uintptr_t>(__builtin_trap);  // { dg-error "builtin" }
+
+  // Expect a diagnostic for an invalid static_cast of a function
+  // to uintptr_t, rather than one for the argument being a built-in
+  // function, since the former is more relevant than the latter.
+  a = static_cast<uintptr_t>(__builtin_trap);       // { dg-error "invalid" }
+
+  a = reinterpret_cast<UINTPTR_E>(__builtin_trap);  // { dg-error "builtin" }
+  a = static_cast<UINTPTR_E>(__builtin_trap);       // { dg-error "builtin" }
+
+  // Additive operator.  Ill-formed but allowed with -fpermissive.
+  p = __builtin_trap + 0;            // { dg-error "builtin" }
+  p = __builtin_trap - 0;            // { dg-error "builtin" }
+  a = __builtin_trap - p;            // { dg-error "builtin" }
+  a = p - __builtin_trap;            // { dg-error "builtin" }
+
+  // Relational operators.  Ill-formed but allowed with -fpermissive.
+  a = __builtin_trap < p;            // { dg-error "builtin" }
+  a = p < __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap > p;            // { dg-error "builtin" }
+  a = p > __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap > p;            // { dg-error "builtin" }
+  a = p > __builtin_trap;            // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin" }
+  a = p <= __builtin_trap;           // { dg-error "builtin" }
+
+  // Equality operators.
+  a = __builtin_trap == p;           // { dg-error "builtin" }
+  a = p == __builtin_trap;           // { dg-error "builtin" }
+  a = __builtin_trap != p;           // { dg-error "builtin" }
+  a = p != __builtin_trap;           // { dg-error "builtin" }
+
+  // Logical AND and OR.
+  a = __builtin_trap && p;           // { dg-error "builtin" }
+  a = p && __builtin_trap;           // { dg-error "builtin" }
+
+  a = __builtin_trap || p;           // { dg-error "builtin" }
+  a = p || __builtin_trap;           // { dg-error "builtin" }
+
+  // Conditional operator.
+  a = __builtin_trap ? 1 : 0;        // { dg-error "builtin" }
+  p = a ? __builtin_trap : 0;        // { dg-error "builtin" }
+  p = a ? 0 : __builtin_trap;        // { dg-error "builtin" }
+
+  // Assignment operator.
+  p = __builtin_trap;                // { dg-error "builtin" }
+
+  // Passing as an argument.
+  func_arg (__builtin_trap);         // { dg-error "builtin" }
+  func_arg (&__builtin_trap);        // { dg-error "builtin" }
+  func_arg (*__builtin_trap);        // { dg-error "builtin" }
+
+  // Passing through ellipsis.
+  func_arg (0, __builtin_trap);      // { dg-error "builtin" }
+  func_arg (0, &__builtin_trap);     // { dg-error "builtin" }
+  func_arg (0, *__builtin_trap);     // { dg-error "builtin" }
+
+  {
+    // Template specialization.
+    TestPointer<__builtin_trap> tp;         // { dg-error "builtin" }
+    TestReference<__builtin_trap> tr;       // { dg-error "builtin" }
+
+    TestPointers<__builtin_trap> tp1;       // { dg-error "builtin" }
+    TestReferences<__builtin_trap> tr1;     // { dg-error "builtin" }
+
+    TestPointers<f, __builtin_trap> tp2;    // { dg-error "builtin" }
+    TestReferences<f, __builtin_trap> tr2;  // { dg-error "builtin" }
+
+    TestPointers<__builtin_trap, f> tp3;    // { dg-error "builtin" }
+    TestReferences<__builtin_trap, f> tr3;  // { dg-error "builtin" }
+  }
+
+  try {
+      throw __builtin_trap;                 // { dg-error "builtin" }
+  }
+  catch (F) { }
+
+  return __builtin_trap;                    // { dg-error "builtin" }
+
+  (void)a;
+  (void)p;
+  (void)q;
+}
+
+// Make sure operators new and delete don't trigger false positives
+// (they return true from DECL_IS_BUILTIN(DECL) -- see tree.h).
+void test_taking_address_of_op_new_and_delete ()
+{
+  typedef __SIZE_TYPE__ size_t;
+
+  typedef void* (OpNew) (size_t);
+  typedef void (OpDelete) (void*);
+
+  OpNew &newr = operator new;
+  OpNew &newra = operator new[];
+  OpNew *newp = &operator new;
+  newp = &operator new[];
+
+  OpDelete &delr = operator delete;
+  OpDelete &delra = operator delete[];
+  OpDelete *delp = &operator delete;
+  delp = &operator delete[];
+
+  (void)newr;
+  (void)newp;
+  (void)delr;
+  (void)delp;
+}
+
+// Helper declaration to verify that it's possible to take the address
+// of a user-declared function that's also a GCC built-in.
+extern int abs (int);
+
+typedef __SIZE_TYPE__ size_t;
+extern size_t strlen (const char*);
+
+// Creating a reference to or taking the address of a built-in with
+// a library "fallback" must be allowed.
+void test_taking_address_of_library_builtin ()
+{
+  {
+    typedef int F (int);
+
+    F &r1 = __builtin_abs;
+    F &r2 = *__builtin_abs;
+    F *p = __builtin_abs;
+    p = &__builtin_abs;
+    p = *__builtin_abs;
+    (void)p;
+    (void)r1;
+    (void)r2;
+  }
+
+  {
+    typedef int F (int);
+
+    F &r1 = abs;
+    F &r2 = *abs;
+    F *p = abs;
+    p = &abs;
+    p = *abs;
+    (void)p;
+    (void)r1;
+    (void)r2;
+  }
+
+  {
+    typedef __SIZE_TYPE__ size_t;
+    typedef size_t F (const char*);
+    F &r1 = __builtin_strlen;
+    F &r2 = *__builtin_strlen;
+    F *p = __builtin_strlen;
+    p = &__builtin_strlen;
+    p = *__builtin_strlen;
+    (void)p;
+    (void)r1;
+    (void)r2;
+  }
+
+  {
+    typedef size_t F (const char*);
+    F &r1 = strlen;
+    F &r2 = *strlen;
+    F *p = strlen;
+    p = &strlen;
+    p = *strlen;
+    (void)p;
+    (void)r1;
+    (void)r2;
+  }
+}
diff --git a/gcc/testsuite/gcc.dg/addr_builtin-1.c b/gcc/testsuite/gcc.dg/addr_builtin-1.c
new file mode 100644
index 0000000..4998e6c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/addr_builtin-1.c
@@ -0,0 +1,198 @@
+/* PR66516 - missing diagnostic on taking the address of a builtin function
+   { dg-do compile }  */
+
+typedef void (F)(void);
+typedef __UINTPTR_TYPE__ uintptr_t;
+
+/* Utility function to test passing built-in functions as an ordinary
+   argument and via the ellipsis.  */
+static void func_arg (F *p, ...) { (void)p; }
+
+static F* test_taking_address_of_gcc_builtin (void)
+{
+  F *p;
+  void *q;
+  uintptr_t a;
+
+  /* Call, cast to void, and id are allowed.  */
+  __builtin_trap ();
+  (void)__builtin_trap;
+  __builtin_trap;
+
+  {
+    typedef __typeof__ (__builtin_trap) F;     /* Okay.  */
+  }
+
+  /* Address and indirection operators.  */
+  p = &__builtin_trap;               /* { dg-error "builtin" }  */
+  p = *__builtin_trap;               /* { dg-error "builtin" }  */
+
+  /* Unary NOT.  */
+  a = !__builtin_trap;               /* { dg-error "builtin" }  */
+
+  /* Sizeof and _Alignof are disallowed by C but allowed by GCC
+     and there's no reason to reject built-ins as operands since
+     doing so doesn't yield their address.  */
+#pragma GCC diagnostic push
+  /* Disable: invalid application of 'sizeof' to a function type.  */
+#pragma GCC diagnostic ignored "-Wpointer-arith"
+  a = sizeof __builtin_trap;
+#pragma GCC diagnostic pop
+
+#ifndef __STDC_VERSION__
+#  pragma GCC diagnostic push
+  /* Disable: ISO C90 does not support '_Alignof'.  */
+#  pragma GCC diagnostic ignored "-Wpedantic"
+#endif
+
+  a = _Alignof __builtin_trap;
+
+#ifndef __STDC_VERSION__
+#  pragma GCC diagnostic pop
+#endif
+
+  /* Casts.  */
+  p = (F*)__builtin_trap;            /* { dg-error "builtin" }  */
+  a = (uintptr_t)__builtin_trap;     /* { dg-error "builtin" }  */
+
+  /* Additive operator.  */
+  p = __builtin_trap + 0;            /* { dg-error "builtin" }  */
+  p = __builtin_trap - 0;            /* { dg-error "builtin" }  */
+  a = __builtin_trap - p;            /* { dg-error "builtin" }  */
+  a = p - __builtin_trap;            /* { dg-error "builtin" }  */
+
+  /* Relational operators.  */
+  a = __builtin_trap < p;            /* { dg-error "builtin" }  */
+  a = p < __builtin_trap;            /* { dg-error "builtin" }  */
+
+  a = __builtin_trap <= p;           /* { dg-error "builtin" }  */
+  a = p <= __builtin_trap;           /* { dg-error "builtin" }  */
+
+  a = __builtin_trap > p;            /* { dg-error "builtin" }  */
+  a = p > __builtin_trap;            /* { dg-error "builtin" }  */
+
+  a = __builtin_trap > p;            /* { dg-error "builtin" }  */
+  a = p > __builtin_trap;            /* { dg-error "builtin" }  */
+
+  a = __builtin_trap <= p;           /* { dg-error "builtin" }  */
+  a = p <= __builtin_trap;           /* { dg-error "builtin" }  */
+
+  a = __builtin_trap <= p;           /* { dg-error "builtin" }  */
+  a = p <= __builtin_trap;           /* { dg-error "builtin" }  */
+
+  /* Equality operators.  */
+  a = __builtin_trap == p;           /* { dg-error "builtin" }  */
+  a = p == __builtin_trap;           /* { dg-error "builtin" }  */
+  a = __builtin_trap != p;           /* { dg-error "builtin" }  */
+  a = p != __builtin_trap;           /* { dg-error "builtin" }  */
+
+  /* Logical AND and OR.  */
+  a = __builtin_trap && p;           /* { dg-error "builtin" }  */
+  a = p && __builtin_trap;           /* { dg-error "builtin" }  */
+
+  a = __builtin_trap || p;           /* { dg-error "builtin" }  */
+  a = p || __builtin_trap;           /* { dg-error "builtin" }  */
+
+  /* Conditional operator.  */
+  a = __builtin_trap ? 1 : 0;        /* { dg-error "builtin" }  */
+  p = a ? __builtin_trap : 0;        /* { dg-error "builtin" }  */
+  p = a ? 0 : __builtin_trap;        /* { dg-error "builtin" }  */
+
+  /* Assignment operator.  */
+  p = __builtin_trap;                /* { dg-error "builtin" }  */
+
+  q = __builtin_trap;                /* { dg-error "builtin" }  */
+  a = __builtin_trap;                /* { dg-error "builtin" }  */
+
+  /* Passing as an argument.  */
+  func_arg (__builtin_trap);         /* { dg-error "builtin" }  */
+
+  /* Passing through the ellipsis.  */
+  func_arg (0, __builtin_trap);      /* { dg-error "builtin" }  */
+
+  /* Return statement.  */
+  return __builtin_trap;             /* { dg-error "builtin" }  */
+
+  (void)a;
+  (void)p;
+  (void)q;
+}
+
+/* Helper declarations to verify that it's possible to take the address
+   of a user-declared function that's also a GCC built-in.  */
+extern int abs (int);
+
+extern __SIZE_TYPE__ strlen (const char*);
+
+/* Taking the address of a builtin with a library "fallback" must be
+   allowed, either using the __builtin_xxx form or the xxx form, when
+   the library fallback is declared either explicitly or implicitly
+   by virtue of first calling the function.  */
+void test_taking_address_of_library_builtin (int i)
+{
+  {
+    typedef int F (int);
+
+    /* Compute the address of libc's abs using the implicitly declared
+       __builtin_abs form (all expressions are valid).  */
+    F *p = __builtin_abs;
+    p = &__builtin_abs;
+    p = *__builtin_abs;
+
+    /* Compute the address of libc's abs declared above.  */
+    p = abs;
+    p = &abs;
+    p = *abs;
+    (void)p;
+  }
+
+  {
+    typedef __SIZE_TYPE__ size_t;
+    typedef size_t F (const char*);
+
+    /* Compute the address of libc's strlen using the implicitly
+       declared __builtin_strlen form.  */
+    F *p = __builtin_strlen;
+    p = &__builtin_strlen;
+    p = *__builtin_strlen;
+
+    /* Compute the address of libc's strlen declared above.  */
+    p = strlen;
+    p = &strlen;
+    p = *strlen;
+    (void)p;
+  }
+
+  {
+    typedef int F (int);
+
+    /* Compute the address of libc's isxxx functions using the implicitly
+       declared __builtin_xxx form.  */
+    F *p = __builtin_isalnum;
+    p = &__builtin_isalpha;
+    p = *__builtin_iscntrl;
+
+    /* According to C90 (see also the discussion in c/67386):
+       If the expression that precedes the parenthesized argument list
+       in a function call consists solely of an identifier, and if no
+       declaration is visible for this identifier, the identifier is
+       implicitly declared exactly as if, in the innermost block
+       containing the function call, the declaration
+       extern int identifier();
+       appeared.  */
+
+    /* Call the functions first to have their declarations "injected"
+       into the enclosing block.  Suppress warnings.  */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wimplicit-function-declaration"
+    i = isalnum (i) || isalpha (i) || iscntrl (i);
+#pragma GCC diagnostic pop
+
+    /* Take the address of the functions relying on their declarations
+       having been implicitly provided by the calls above.  */
+    p = isalnum;
+    p = &isalpha;
+    p = *iscntrl;
+    (void)p;
+  }
+}

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-09-01 15:01                               ` Martin Sebor
@ 2015-09-01 17:29                                 ` Joseph Myers
  2015-09-01 22:25                                   ` Martin Sebor
  0 siblings, 1 reply; 34+ messages in thread
From: Joseph Myers @ 2015-09-01 17:29 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Jason Merrill, Jakub Jelinek, Marek Polacek, Gcc Patch List

On Tue, 1 Sep 2015, Martin Sebor wrote:

> Attached is an updated patch that avoids diagnosing taking the address
> of implicitly declared library builtins like abs, bootstrapped and
> tested on ppc64le with no regressions.
> 
> The tweak below was added to reject_gcc_builtin make it possible.
> Since the expression is in c-family/c-common.c, the more descriptive
> C_DECL_IMPLICIT that exists for this purpose is not available (it's
> defined in c/c-tree.h).
> 
> +      && (c_dialect_cxx () || !DECL_LANG_FLAG_2 (expr))

I don't think hardcoding DECL_LANG_FLAG_2 here is a good idea; rather, 
some sort of c_decl_implicit hook should be defined (that would just 
return false for C++) - existing practice is various functions that have 
different definitions for C and C++.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-09-01 17:29                                 ` Joseph Myers
@ 2015-09-01 22:25                                   ` Martin Sebor
  2015-09-02  0:20                                     ` Joseph Myers
  2015-09-02 15:29                                     ` Jason Merrill
  0 siblings, 2 replies; 34+ messages in thread
From: Martin Sebor @ 2015-09-01 22:25 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Jason Merrill, Jakub Jelinek, Marek Polacek, Gcc Patch List

On 09/01/2015 11:29 AM, Joseph Myers wrote:
> On Tue, 1 Sep 2015, Martin Sebor wrote:
>
>> Attached is an updated patch that avoids diagnosing taking the address
>> of implicitly declared library builtins like abs, bootstrapped and
>> tested on ppc64le with no regressions.
>>
>> The tweak below was added to reject_gcc_builtin make it possible.
>> Since the expression is in c-family/c-common.c, the more descriptive
>> C_DECL_IMPLICIT that exists for this purpose is not available (it's
>> defined in c/c-tree.h).
>>
>> +      && (c_dialect_cxx () || !DECL_LANG_FLAG_2 (expr))
>
> I don't think hardcoding DECL_LANG_FLAG_2 here is a good idea; rather,
> some sort of c_decl_implicit hook should be defined (that would just
> return false for C++) - existing practice is various functions that have
> different definitions for C and C++.

I've made the suggested change and tested it by bootstrapping
and running the two tests.

Before I post another revision of the patch for review let me
make sure there are no other change requests, and that introducing
a new c_decl_implicit hook here is what everyone wants.

The original patch had a distinct function for each of the C and
C++ front ends to issue the diagnostics because each had slightly
different tests. I've since merged the two functions into one on
Jason's suggestion and removed most of the  differences. Adding
a hook seems to be step in the opposite direction.

Having now made this change, I don't think the added complexity
of three declarations and two trivial definitions of the new
c_decl_implicit function across five files is an improvement,
especially since we're still checking for c_dialect_cxx().
(The change simply replaces:

   && (c_dialect_cxx () || !DECL_LANG_FLAG_2 (expr))

with

   && (c_dialect_cxx () || c_decl_implicit (expr))

as suggested.)

Having also looked at the existing code in c-family/common.c
that does things differently between C and C++ I see examples
of both hooks for more complex computations, and conditionals
similar to the one I just replaced for simpler things.

I also noticed uses of DECL_LANG_FLAG_4 in the definitions of
what appear to be C-specific macros in c-family/c-common.h,
and then uses of the same macro in definitions of a C++-specific
macro in cp/cp-tree.h.

In this light it seems to me that leaving the test for the flag
as it was would be both in keeping with existing practice and
preferable to introducing the hook.

Let me know.

Thanks
Martin

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-09-01 22:25                                   ` Martin Sebor
@ 2015-09-02  0:20                                     ` Joseph Myers
  2015-09-02 15:29                                     ` Jason Merrill
  1 sibling, 0 replies; 34+ messages in thread
From: Joseph Myers @ 2015-09-02  0:20 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Jason Merrill, Jakub Jelinek, Marek Polacek, Gcc Patch List

On Tue, 1 Sep 2015, Martin Sebor wrote:

> I also noticed uses of DECL_LANG_FLAG_4 in the definitions of
> what appear to be C-specific macros in c-family/c-common.h,
> and then uses of the same macro in definitions of a C++-specific
> macro in cp/cp-tree.h.

That seems like a bug waiting to happen, given that there is nothing 
obviously C-specific about the users in common code of those macros.

> In this light it seems to me that leaving the test for the flag
> as it was would be both in keeping with existing practice and
> preferable to introducing the hook.

The existing practice you found was of use of DECL_LANG_FLAG_* in defining 
macros.  Not direct use in C files, which is clearly much worse.  I 
suppose there's the option of defining the macro in c-common.h, but 
defining it in a way that includes an assertion that it's never evaluated 
for C++.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-09-01 22:25                                   ` Martin Sebor
  2015-09-02  0:20                                     ` Joseph Myers
@ 2015-09-02 15:29                                     ` Jason Merrill
  2015-09-02 22:13                                       ` Martin Sebor
  1 sibling, 1 reply; 34+ messages in thread
From: Jason Merrill @ 2015-09-02 15:29 UTC (permalink / raw)
  To: Martin Sebor, Joseph Myers; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

On 09/01/2015 06:25 PM, Martin Sebor wrote:
> Having now made this change, I don't think the added complexity
> of three declarations and two trivial definitions of the new
> c_decl_implicit function across five files is an improvement,

Three declarations?  Isn't declaring it in c-common.h enough?

> especially since we're still checking for c_dialect_cxx().
> (The change simply replaces:
>
>    && (c_dialect_cxx () || !DECL_LANG_FLAG_2 (expr))
>
> with
>
>    && (c_dialect_cxx () || c_decl_implicit (expr))
>
> as suggested.)

Seems like you can do without the check for C++ if you're defining this 
function for C++ (to just return false).

I agree with Joseph that the function is better.

> +		 bool diag /* = true */)

Let's call these parameters "reject_builtin" rather than the generic "diag".

> +    function = decay_conversion (function, complain, false);

Please add a comment to "false", e.g. /*reject_builtin*/false

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-09-02 15:29                                     ` Jason Merrill
@ 2015-09-02 22:13                                       ` Martin Sebor
  2015-09-03 14:29                                         ` Jason Merrill
  2015-09-03 17:30                                         ` Jakub Jelinek
  0 siblings, 2 replies; 34+ messages in thread
From: Martin Sebor @ 2015-09-02 22:13 UTC (permalink / raw)
  To: Jason Merrill, Joseph Myers; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

On 09/02/2015 09:29 AM, Jason Merrill wrote:
> On 09/01/2015 06:25 PM, Martin Sebor wrote:
>> Having now made this change, I don't think the added complexity
>> of three declarations and two trivial definitions of the new
>> c_decl_implicit function across five files is an improvement,
>
> Three declarations?  Isn't declaring it in c-common.h enough?
...
>
> Seems like you can do without the check for C++ if you're defining this
> function for C++ (to just return false).

Yes on both counts. Thanks.

>
> I agree with Joseph that the function is better.
>
>> +         bool diag /* = true */)
>
> Let's call these parameters "reject_builtin" rather than the generic
> "diag".
>
>> +    function = decay_conversion (function, complain, false);
>
> Please add a comment to "false", e.g. /*reject_builtin*/false

Done.

The latest patch (attached) implements all of requested changes
plus a few tweaks to the tests to make them more strict.

I also noticed and fixed a gotcha in the dg-error directives
that might be interesting to mention: in prior patches tests
were all: /* dg-error "builtin" */
Unfortunately, since both the name of the test file and the
test function have the string "builtin" in them, the directives
would pass even if the error message wasn't the expected:

   builtin function ‘__builtin_trap’ must be directly called

This problem was hiding a few invalid C++ tests. I've fixed
the directives to avoid the problem.

I've tested it on x86_64 this time with no regressions.

Martin


[-- Attachment #2: gcc-66516-7.patch --]
[-- Type: text/x-patch, Size: 30630 bytes --]

gcc/ChangeLog
2015-09-02  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* doc/extend.texi (Other Builtins): Document when the address
	of a builtin function can be taken.

gcc/c-family/ChangeLog
2015-09-02  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* c-common.h (c_decl_implicit, reject_gcc_builtin): Declare new
	functions.
	* c-common.c (reject_gcc_builtin): Define.

gcc/c/ChangeLog
2015-09-02  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* c/c-typeck.c (convert_arguments, parser_build_unary_op)
	(build_conditional_expr, c_cast_expr, convert_for_assignment)
	(build_binary_op, _objc_common_truthvalue_conversion): Call
	reject_gcc_builtin.
	(c_decl_implicit): Define.

gcc/cp/ChangeLog
2015-09-02  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* cp/cp-tree.h (mark_rvalue_use, decay_conversion): Add new
	argument(s).
	* cp/expr.c (mark_rvalue_use): Use new argument.
	* cp/call.c (build_addr_func): Call decay_conversion with new
	argument.
	* cp/pt.c (convert_template_argument): Call reject_gcc_builtin.
	* cp/typeck.c (decay_conversion): Use new argument.
	(c_decl_implicit): Define.

gcc/testsuite/ChangeLog
2015-09-02  Martin Sebor  <msebor@redhat.com>

	PR c/66516
	* g++.dg/addr_builtin-1.C: New test.
	* gcc.dg/addr_builtin-1.c: New test.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 7691035..4cc6c3e 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -12882,4 +12882,41 @@ pointer_to_zero_sized_aggr_p (tree t)
   return (TYPE_SIZE (t) && integer_zerop (TYPE_SIZE (t)));
 }

+/* For an EXPR of a FUNCTION_TYPE that references a GCC built-in function
+   with no library fallback or for an ADDR_EXPR whose operand is such type
+   issues an error pointing to the location LOC.
+   Returns true when the expression has been diagnosed and false
+   otherwise.  */
+bool
+reject_gcc_builtin (const_tree expr, location_t loc /* = UNKNOWN_LOCATION */)
+{
+  if (TREE_CODE (expr) == ADDR_EXPR)
+    expr = TREE_OPERAND (expr, 0);
+
+  if (TREE_TYPE (expr)
+      && TREE_CODE (TREE_TYPE (expr)) == FUNCTION_TYPE
+      && DECL_P (expr)
+      /* The intersection of DECL_BUILT_IN and DECL_IS_BUILTIN avoids
+	 false positives for user-declared built-ins such as abs or
+	 strlen, and for C++ operators new and delete.
+	 The c_decl_implicit() test avoids false positives for implicitly
+	 declared builtins with library fallbacks (such as abs).  */
+      && DECL_BUILT_IN (expr)
+      && DECL_IS_BUILTIN (expr)
+      && !c_decl_implicit (expr)
+      && !DECL_ASSEMBLER_NAME_SET_P (expr))
+    {
+      if (loc == UNKNOWN_LOCATION)
+	loc = EXPR_LOC_OR_LOC (expr, input_location);
+
+      /* Reject arguments that are builtin functions with
+	 no library fallback.  */
+      error_at (loc, "builtin function %qE must be directly called", expr);
+
+      return true;
+    }
+
+  return false;
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index be63cd2..0d589b5 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -572,6 +572,7 @@ extern int field_decl_cmp (const void *, const void *);
 extern void resort_sorted_fields (void *, void *, gt_pointer_operator,
 				  void *);
 extern bool has_c_linkage (const_tree decl);
+extern bool c_decl_implicit (const_tree);
 \f
 /* Switches common to the C front ends.  */

@@ -1437,5 +1438,6 @@ extern bool contains_cilk_spawn_stmt (tree);
 extern tree cilk_for_number_of_iterations (tree);
 extern bool check_no_cilk (tree, const char *, const char *,
 		           location_t loc = UNKNOWN_LOCATION);
+extern bool reject_gcc_builtin (const_tree, location_t = UNKNOWN_LOCATION);

 #endif /* ! GCC_C_COMMON_H */
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index e8c8189..22b0d0d 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -3339,6 +3339,10 @@ convert_arguments (location_t loc, vec<location_t> arg_loc, tree typelist,
 	  error (invalid_func_diag);
 	  return -1;
 	}
+      else if (TREE_CODE (val) == ADDR_EXPR && reject_gcc_builtin (val))
+	{
+	  return -1;
+	}
       else
 	/* Convert `short' and `char' to full-size `int'.  */
 	parmval = default_conversion (val);
@@ -3376,12 +3380,20 @@ parser_build_unary_op (location_t loc, enum tree_code code, struct c_expr arg)
 {
   struct c_expr result;

-  result.value = build_unary_op (loc, code, arg.value, 0);
   result.original_code = code;
   result.original_type = NULL;

+  if (reject_gcc_builtin (arg.value))
+    {
+      result.value = error_mark_node;
+    }
+  else
+    {
+      result.value = build_unary_op (loc, code, arg.value, 0);
+
   if (TREE_OVERFLOW_P (result.value) && !TREE_OVERFLOW_P (arg.value))
     overflow_warning (loc, result.value);
+    }

   return result;
 }
@@ -4484,6 +4496,12 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
   type2 = TREE_TYPE (op2);
   code2 = TREE_CODE (type2);

+  if (code1 == POINTER_TYPE && reject_gcc_builtin (op1))
+    return error_mark_node;
+
+  if (code2 == POINTER_TYPE && reject_gcc_builtin (op2))
+    return error_mark_node;
+
   /* C90 does not permit non-lvalue arrays in conditional expressions.
      In C99 they will be pointers by now.  */
   if (code1 == ARRAY_TYPE || code2 == ARRAY_TYPE)
@@ -5222,6 +5240,10 @@ c_cast_expr (location_t loc, struct c_type_name *type_name, tree expr)
   type = groktypename (type_name, &type_expr, &type_expr_const);
   warn_strict_prototypes = saved_wsp;

+  if (TREE_CODE (expr) == ADDR_EXPR && !VOID_TYPE_P (type)
+      && reject_gcc_builtin (expr))
+    return error_mark_node;
+
   ret = build_c_cast (loc, type, expr);
   if (type_expr)
     {
@@ -5861,6 +5883,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
   rhs = require_complete_type (rhs);
   if (rhs == error_mark_node)
     return error_mark_node;
+
+  if (coder == POINTER_TYPE && reject_gcc_builtin (rhs))
+    return error_mark_node;
+
   /* A non-reference type can convert to a reference.  This handles
      va_start, va_copy and possibly port built-ins.  */
   if (codel == REFERENCE_TYPE && coder != REFERENCE_TYPE)
@@ -10350,6 +10376,14 @@ build_binary_op (location_t location, enum tree_code code,
   if (code0 == ERROR_MARK || code1 == ERROR_MARK)
     return error_mark_node;

+  if (code0 == POINTER_TYPE
+      && reject_gcc_builtin (op0, EXPR_LOCATION (orig_op0)))
+    return error_mark_node;
+
+  if (code1 == POINTER_TYPE
+      && reject_gcc_builtin (op1, EXPR_LOCATION (orig_op1)))
+    return error_mark_node;
+
   if ((invalid_op_diag
        = targetm.invalid_binary_op (code, type0, type1)))
     {
@@ -11330,6 +11364,11 @@ c_objc_common_truthvalue_conversion (location_t location, tree expr)
       error_at (location, "void value not ignored as it ought to be");
       return error_mark_node;

+    case POINTER_TYPE:
+      if (reject_gcc_builtin (expr))
+	return error_mark_node;
+      break;
+
     case FUNCTION_TYPE:
       gcc_unreachable ();

@@ -12882,3 +12921,13 @@ cilk_install_body_with_frame_cleanup (tree fndecl, tree body, void *w)
   append_to_statement_list (build_stmt (EXPR_LOCATION (body), TRY_FINALLY_EXPR,
 				       	body_list, dtor), &list);
 }
+
+/* Returns true when the function declaration FNDECL is implicit,
+   introduced as a result of a call to an otherwise undeclared
+   function, and false otherwise.  */
+
+bool
+c_decl_implicit (const_tree fndecl)
+{
+  return C_DECL_IMPLICIT (fndecl);
+}
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 8d4a9e2..c4e69a4 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -288,7 +288,7 @@ build_addr_func (tree function, tsubst_flags_t complain)
       function = build_address (function);
     }
   else
-    function = decay_conversion (function, complain);
+    function = decay_conversion (function, complain, /*reject_builtin=*/false);

   return function;
 }
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 4dee60c..784a616 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5787,7 +5787,9 @@ extern tree create_try_catch_expr               (tree, tree);

 /* in expr.c */
 extern tree cplus_expand_constant		(tree);
-extern tree mark_rvalue_use			(tree);
+extern tree mark_rvalue_use			(tree,
+                                                 location_t = UNKNOWN_LOCATION,
+                                                 bool = true);
 extern tree mark_lvalue_use			(tree);
 extern tree mark_type_use			(tree);
 extern void mark_exp_read			(tree);
@@ -6461,7 +6463,9 @@ extern tree cxx_alignas_expr                    (tree);
 extern tree cxx_sizeof_nowarn                   (tree);
 extern tree is_bitfield_expr_with_lowered_type  (const_tree);
 extern tree unlowered_expr_type                 (const_tree);
-extern tree decay_conversion			(tree, tsubst_flags_t);
+extern tree decay_conversion			(tree,
+                                                 tsubst_flags_t,
+                                                 bool = true);
 extern tree build_class_member_access_expr      (tree, tree, tree, bool,
 						 tsubst_flags_t);
 extern tree finish_class_member_access_expr     (tree, tree, bool,
diff --git a/gcc/cp/expr.c b/gcc/cp/expr.c
index 6d2d658..2addffe 100644
--- a/gcc/cp/expr.c
+++ b/gcc/cp/expr.c
@@ -91,18 +91,24 @@ cplus_expand_constant (tree cst)
   return cst;
 }

-/* Called whenever an expression is used
-   in a rvalue context.  */
-
+/* Called whenever the expression EXPR is used in an rvalue context.
+   When REJECT_BUILTIN is true the expression is checked to make sure
+   it doesn't make it possible to obtain the address of a GCC built-in
+   function with no library fallback (or any of its bits, such as in
+   a conversion to bool).  */
 tree
-mark_rvalue_use (tree expr)
+mark_rvalue_use (tree expr,
+		 location_t loc /* = UNKNOWN_LOCATION */,
+		 bool reject_builtin /* = true */)
 {
+  if (reject_builtin && reject_gcc_builtin (expr, loc))
+    return error_mark_node;
+
   mark_exp_read (expr);
   return expr;
 }

-/* Called whenever an expression is used
-   in a lvalue context.  */
+/* Called whenever an expression is used in an lvalue context.  */

 tree
 mark_lvalue_use (tree expr)
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index fb7b9d2..ec32c5a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -7199,6 +7199,18 @@ convert_template_argument (tree parm,
       else if (val == error_mark_node && (complain & tf_error))
 	error ("could not convert template argument %qE to %qT",  orig_arg, t);

+      if (INDIRECT_REF_P (val))
+        {
+          /* Reject template arguments that are references to built-in
+             functions with no library fallbacks.  */
+          const_tree inner = TREE_OPERAND (val, 0);
+          if (TREE_CODE (TREE_TYPE (inner)) == REFERENCE_TYPE
+              && TREE_CODE (TREE_TYPE (TREE_TYPE (inner))) == FUNCTION_TYPE
+              && TREE_CODE (TREE_TYPE (inner)) == REFERENCE_TYPE
+              && reject_gcc_builtin (TREE_OPERAND (inner, 0)))
+              return error_mark_node;
+        }
+
       if (TREE_CODE (val) == SCOPE_REF)
 	{
 	  /* Strip typedefs from the SCOPE_REF.  */
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 83fd34c..fac9c30 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -1911,7 +1911,9 @@ unlowered_expr_type (const_tree exp)
    that the return value is no longer an lvalue.  */

 tree
-decay_conversion (tree exp, tsubst_flags_t complain)
+decay_conversion (tree exp,
+		  tsubst_flags_t complain,
+		  bool reject_builtin /* = true */)
 {
   tree type;
   enum tree_code code;
@@ -1921,7 +1923,7 @@ decay_conversion (tree exp, tsubst_flags_t complain)
   if (type == error_mark_node)
     return error_mark_node;

-  exp = mark_rvalue_use (exp);
+  exp = mark_rvalue_use (exp, loc, reject_builtin);

   exp = resolve_nondeduced_context (exp);
   if (type_unknown_p (exp))
@@ -9397,3 +9399,12 @@ check_literal_operator_args (const_tree decl,
       return true;
     }
 }
+
+/* Always returns false since unlike C90, C++ has no concept of implicit
+   function declarations.  */
+
+bool
+c_decl_implicit (const_tree)
+{
+  return false;
+}
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index dba8b43..23e6a76 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -10316,14 +10316,22 @@ recommend general use of these functions.

 The remaining functions are provided for optimization purposes.

+With the exception of built-ins that have library equivalents such as
+the standard C library functions discussed below, or that expand to
+library calls, GCC built-in functions are always expanded inline and
+thus do not have corresponding entry points and their address cannot
+be obtained.  Attempting to use them in an expression other than
+a function call results in a compile-time error.
+
 @opindex fno-builtin
 GCC includes built-in versions of many of the functions in the standard
-C library.  The versions prefixed with @code{__builtin_} are always
-treated as having the same meaning as the C library function even if you
-specify the @option{-fno-builtin} option.  (@pxref{C Dialect Options})
-Many of these functions are only optimized in certain cases; if they are
-not optimized in a particular case, a call to the library function is
-emitted.
+C library.  These functions come in two forms: one whose names start with
+the @code{__builtin_} prefix, and the other without.  Both forms have the
+same type (including prototype), the same address (when their address is
+taken), and the same meaning as the C library functions even if you specify
+the @option{-fno-builtin} option @pxref{C Dialect Options}).  Many of these
+functions are only optimized in certain cases; if they are not optimized in
+a particular case, a call to the library function is emitted.

 @opindex ansi
 @opindex std
diff --git a/gcc/testsuite/g++.dg/addr_builtin-1.C b/gcc/testsuite/g++.dg/addr_builtin-1.C
new file mode 100644
index 0000000..09ffc4e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/addr_builtin-1.C
@@ -0,0 +1,276 @@
+// PR66516 - missing diagnostic on taking the address of a builtin function
+// { dg-do compile }
+
+namespace std {
+  // Define type_info type to be able to use typeid in tests without
+  // having to include <typeinfo>.
+  struct type_info {
+    const char *name_;
+
+    explicit type_info (const char *s): name_ (s) { }
+    const char* name() const { return name_; }
+  };
+}
+
+// Extern "C" since builtin functions used in tests have C linkage.
+extern "C" {
+
+typedef void (F)();
+typedef __UINTPTR_TYPE__ uintptr_t;
+
+// Utility function to test passing built-in functions as an ordinary
+// argument and via the ellipsis.
+static void func_arg (F*, ...);
+
+// Utility function with which, along with the built-in function,
+// to instantiate the C98 multi-parameter or C11 variadic tempates
+// below.
+void f () { }
+
+}   // extern "C"
+
+
+// Utility templates to test specializing templates on pointers and
+// references to built-in functions.
+template <F*> struct TestPointer { };
+template <F&> struct TestReference { };
+
+#if 201103 <= __cplusplus
+
+template <F*...> struct TestPointers { };
+template <F&...> struct TestReferences { };
+
+#else
+
+template <F* = &f, F* = &f> struct TestPointers { };
+template <F& = f, F& = f> struct TestReferences { };
+
+#endif
+
+static F* test_taking_address_of_gcc_builtin ()
+{
+  enum UINTPTR_E { e = ~(uintptr_t)0 };
+
+  F *p;
+  void *q;
+  uintptr_t a;
+
+  __builtin_trap ();                           // okay
+  (void)__builtin_trap;                        // okay
+  __builtin_trap;                              // okay (if pointless)
+
+  {
+    typedef __typeof__ (__builtin_trap) F;     // okay
+  }
+
+#if 201103 <= __cplusplus
+  {
+    typedef decltype (__builtin_trap) F;       // okay
+
+    a = noexcept (&__builtin_trap);
+  }
+#endif
+
+  // Address and indirection operators.
+  p = &__builtin_trap;                       // { dg-error "builtin function" }
+  p = *__builtin_trap;                       // { dg-error "builtin function" }
+
+  // Unary NOT.
+  // GCC issues two diagnostics here for some reason, so account for both.
+  a = !__builtin_trap;                   // { dg-error "builtin function|unary" }
+
+  // Casts.
+  p = (F*)__builtin_trap;                    // { dg-error "builtin function" }
+
+  p = &(F&)__builtin_trap;                   // { dg-error "builtin function" }
+
+  p = &reinterpret_cast<F&>(__builtin_trap); // { dg-error "builtin function" }
+  p = &static_cast<F&>(__builtin_trap);      // { dg-error "builtin function" }
+
+  p = reinterpret_cast<F*>(__builtin_trap);  // { dg-error "builtin function" }
+  p = static_cast<F*>(__builtin_trap);       // { dg-error "builtin function" }
+
+  // Expect a diagnostic for an invalid static_cast of a function to
+  // either uintptr_t or enum, rather than one for the argument being
+  // a built-in function, since the former is more relevant than the latter.
+  a = static_cast<uintptr_t>(__builtin_trap);       // { dg-error "invalid" }
+  a = static_cast<UINTPTR_E>(__builtin_trap);       // { dg-error "invalid" }
+
+  // Reinterpret cast can cast a function to uintptr_t or enum,
+  // so verify that a diagnostic is issued for the use of a builtin.
+  a = reinterpret_cast<uintptr_t>(__builtin_trap);  // { dg-error "builtin function" }
+  a = reinterpret_cast<UINTPTR_E>(__builtin_trap);  // { dg-error "builtin function" }
+
+  // Additive operator.  Ill-formed but allowed with -fpermissive.
+  p = __builtin_trap + 0;            // { dg-error "builtin function" }
+  p = __builtin_trap - 0;            // { dg-error "builtin function" }
+  a = __builtin_trap - p;            // { dg-error "builtin function" }
+  a = p - __builtin_trap;            // { dg-error "builtin function" }
+
+  // Relational operators.  Ill-formed but allowed with -fpermissive.
+  a = __builtin_trap < p;            // { dg-error "builtin function" }
+  a = p < __builtin_trap;            // { dg-error "builtin function" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin function" }
+  a = p <= __builtin_trap;           // { dg-error "builtin function" }
+
+  a = __builtin_trap > p;            // { dg-error "builtin function" }
+  a = p > __builtin_trap;            // { dg-error "builtin function" }
+
+  a = __builtin_trap > p;            // { dg-error "builtin function" }
+  a = p > __builtin_trap;            // { dg-error "builtin function" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin function" }
+  a = p <= __builtin_trap;           // { dg-error "builtin function" }
+
+  a = __builtin_trap <= p;           // { dg-error "builtin function" }
+  a = p <= __builtin_trap;           // { dg-error "builtin function" }
+
+  // Equality operators.
+  a = __builtin_trap == p;           // { dg-error "builtin function" }
+  a = p == __builtin_trap;           // { dg-error "builtin function" }
+  a = __builtin_trap != p;           // { dg-error "builtin function" }
+  a = p != __builtin_trap;           // { dg-error "builtin function" }
+
+  // Logical AND and OR.
+  a = __builtin_trap && p;           // { dg-error "builtin function" }
+  a = p && __builtin_trap;           // { dg-error "builtin function" }
+
+  a = __builtin_trap || p;           // { dg-error "builtin function" }
+  a = p || __builtin_trap;           // { dg-error "builtin function" }
+
+  // Conditional operator.
+  a = __builtin_trap ? 1 : 0;        // { dg-error "builtin function" }
+  p = a ? __builtin_trap : 0;        // { dg-error "builtin function" }
+  p = a ? 0 : __builtin_trap;        // { dg-error "builtin function" }
+
+  // Assignment operator.
+  p = __builtin_trap;                // { dg-error "builtin function" }
+
+  // Passing as an argument.
+  func_arg (__builtin_trap);         // { dg-error "builtin function" }
+  func_arg (&__builtin_trap);        // { dg-error "builtin function" }
+  func_arg (*__builtin_trap);        // { dg-error "builtin function" }
+
+  // Passing through ellipsis.
+  func_arg (0, __builtin_trap);      // { dg-error "builtin function" }
+  func_arg (0, &__builtin_trap);     // { dg-error "builtin function" }
+  func_arg (0, *__builtin_trap);     // { dg-error "builtin function" }
+
+  {
+    // Template specialization.
+    // GCC issues two diagnostics and we must account for both.
+    TestPointer<__builtin_trap> tp;         // { dg-error "builtin function|could not convert" }
+    TestReference<__builtin_trap> tr;       // { dg-error "builtin function|could not convert" }
+
+    TestPointers<__builtin_trap> tp1;       // { dg-error "builtin function|could not convert" }
+    TestReferences<__builtin_trap> tr1;     // { dg-error "builtin function|could not convert" }
+
+    TestPointers<f, __builtin_trap> tp2;    // { dg-error "builtin function|could not convert" }
+    TestReferences<f, __builtin_trap> tr2;  // { dg-error "builtin function|could not convert" }
+
+    TestPointers<__builtin_trap, f> tp3;    // { dg-error "builtin function|could not convert" }
+    TestReferences<__builtin_trap, f> tr3;  // { dg-error "builtin function|could not convert" }
+  }
+
+  try {
+    throw __builtin_trap;                 // { dg-error "builtin function" }
+  }
+  catch (F) { }
+
+  return __builtin_trap;                    // { dg-error "builtin function" }
+
+  (void)a;
+  (void)p;
+  (void)q;
+}
+
+// Make sure operators new and delete don't trigger false positives
+// (they return true from DECL_IS_BUILTIN(DECL) -- see tree.h).
+void test_taking_address_of_op_new_and_delete ()
+{
+  typedef __SIZE_TYPE__ size_t;
+
+  typedef void* (OpNew) (size_t);
+  typedef void (OpDelete) (void*);
+
+  OpNew &newr = operator new;
+  OpNew &newra = operator new[];
+  OpNew *newp = &operator new;
+  newp = &operator new[];
+
+  OpDelete &delr = operator delete;
+  OpDelete &delra = operator delete[];
+  OpDelete *delp = &operator delete;
+  delp = &operator delete[];
+
+  (void)newr;
+  (void)newra;
+  (void)newp;
+  (void)delr;
+  (void)delra;
+  (void)delp;
+}
+
+// Helper declaration to verify that it's possible to take the address
+// of a user-declared function that's also a GCC built-in.
+extern int abs (int);
+
+typedef __SIZE_TYPE__ size_t;
+extern size_t strlen (const char*);
+
+// Creating a reference to or taking the address of a built-in with
+// a library "fallback" must be allowed.
+void test_taking_address_of_library_builtin ()
+{
+  {
+    typedef int F (int);
+
+    F &r1 = __builtin_abs;
+    F &r2 = *__builtin_abs;
+    F *p = __builtin_abs;
+    p = &__builtin_abs;
+    p = *__builtin_abs;
+    (void)p;
+    (void)r1;
+    (void)r2;
+  }
+
+  {
+    typedef int F (int);
+
+    F &r1 = abs;
+    F &r2 = *abs;
+    F *p = abs;
+    p = &abs;
+    p = *abs;
+    (void)p;
+    (void)r1;
+    (void)r2;
+  }
+
+  {
+    typedef __SIZE_TYPE__ size_t;
+    typedef size_t F (const char*);
+    F &r1 = __builtin_strlen;
+    F &r2 = *__builtin_strlen;
+    F *p = __builtin_strlen;
+    p = &__builtin_strlen;
+    p = *__builtin_strlen;
+    (void)p;
+    (void)r1;
+    (void)r2;
+  }
+
+  {
+    typedef size_t F (const char*);
+    F &r1 = strlen;
+    F &r2 = *strlen;
+    F *p = strlen;
+    p = &strlen;
+    p = *strlen;
+    (void)p;
+    (void)r1;
+    (void)r2;
+  }
+}
diff --git a/gcc/testsuite/gcc.dg/addr_builtin-1.c b/gcc/testsuite/gcc.dg/addr_builtin-1.c
new file mode 100644
index 0000000..f3ce5f1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/addr_builtin-1.c
@@ -0,0 +1,198 @@
+/* PR66516 - missing diagnostic on taking the address of a builtin function
+   { dg-do compile }  */
+
+typedef void (F)(void);
+typedef __UINTPTR_TYPE__ uintptr_t;
+
+/* Utility function to test passing built-in functions as an ordinary
+   argument and via the ellipsis.  */
+static void func_arg (F *p, ...) { (void)p; }
+
+static F* test_taking_address_of_gcc_builtin (void)
+{
+  F *p;
+  void *q;
+  uintptr_t a;
+
+  /* Call, cast to void, and id are allowed.  */
+  __builtin_trap ();
+  (void)__builtin_trap;
+  __builtin_trap;
+
+  {
+    typedef __typeof__ (__builtin_trap) F;     /* Okay.  */
+  }
+
+  /* Address and indirection operators.  */
+  p = &__builtin_trap;               /* { dg-error "builtin function" }  */
+  p = *__builtin_trap;               /* { dg-error "builtin function" }  */
+
+  /* Unary NOT.  */
+  a = !__builtin_trap;               /* { dg-error "builtin function" }  */
+
+  /* Sizeof and _Alignof are disallowed by C but allowed by GCC
+     and there's no reason to reject built-ins as operands since
+     doing so doesn't yield their address.  */
+#pragma GCC diagnostic push
+  /* Disable: invalid application of 'sizeof' to a function type.  */
+#pragma GCC diagnostic ignored "-Wpointer-arith"
+  a = sizeof __builtin_trap;
+#pragma GCC diagnostic pop
+
+#ifndef __STDC_VERSION__
+#  pragma GCC diagnostic push
+  /* Disable: ISO C90 does not support '_Alignof'.  */
+#  pragma GCC diagnostic ignored "-Wpedantic"
+#endif
+
+  a = _Alignof __builtin_trap;
+
+#ifndef __STDC_VERSION__
+#  pragma GCC diagnostic pop
+#endif
+
+  /* Casts.  */
+  p = (F*)__builtin_trap;            /* { dg-error "builtin function" }  */
+  a = (uintptr_t)__builtin_trap;     /* { dg-error "builtin function" }  */
+
+  /* Additive operator.  */
+  p = __builtin_trap + 0;            /* { dg-error "builtin function" }  */
+  p = __builtin_trap - 0;            /* { dg-error "builtin function" }  */
+  a = __builtin_trap - p;            /* { dg-error "builtin function" }  */
+  a = p - __builtin_trap;            /* { dg-error "builtin function" }  */
+
+  /* Relational operators.  */
+  a = __builtin_trap < p;            /* { dg-error "builtin function" }  */
+  a = p < __builtin_trap;            /* { dg-error "builtin function" }  */
+
+  a = __builtin_trap <= p;           /* { dg-error "builtin function" }  */
+  a = p <= __builtin_trap;           /* { dg-error "builtin function" }  */
+
+  a = __builtin_trap > p;            /* { dg-error "builtin function" }  */
+  a = p > __builtin_trap;            /* { dg-error "builtin function" }  */
+
+  a = __builtin_trap > p;            /* { dg-error "builtin function" }  */
+  a = p > __builtin_trap;            /* { dg-error "builtin function" }  */
+
+  a = __builtin_trap <= p;           /* { dg-error "builtin function" }  */
+  a = p <= __builtin_trap;           /* { dg-error "builtin function" }  */
+
+  a = __builtin_trap <= p;           /* { dg-error "builtin function" }  */
+  a = p <= __builtin_trap;           /* { dg-error "builtin function" }  */
+
+  /* Equality operators.  */
+  a = __builtin_trap == p;           /* { dg-error "builtin function" }  */
+  a = p == __builtin_trap;           /* { dg-error "builtin function" }  */
+  a = __builtin_trap != p;           /* { dg-error "builtin function" }  */
+  a = p != __builtin_trap;           /* { dg-error "builtin function" }  */
+
+  /* Logical AND and OR.  */
+  a = __builtin_trap && p;           /* { dg-error "builtin function" }  */
+  a = p && __builtin_trap;           /* { dg-error "builtin function" }  */
+
+  a = __builtin_trap || p;           /* { dg-error "builtin function" }  */
+  a = p || __builtin_trap;           /* { dg-error "builtin function" }  */
+
+  /* Conditional operator.  */
+  a = __builtin_trap ? 1 : 0;        /* { dg-error "builtin function" }  */
+  p = a ? __builtin_trap : 0;        /* { dg-error "builtin function" }  */
+  p = a ? 0 : __builtin_trap;        /* { dg-error "builtin function" }  */
+
+  /* Assignment operator.  */
+  p = __builtin_trap;                /* { dg-error "builtin function" }  */
+
+  q = __builtin_trap;                /* { dg-error "builtin function" }  */
+  a = __builtin_trap;                /* { dg-error "builtin function" }  */
+
+  /* Passing as an argument.  */
+  func_arg (__builtin_trap);         /* { dg-error "builtin function" }  */
+
+  /* Passing through the ellipsis.  */
+  func_arg (0, __builtin_trap);      /* { dg-error "builtin function" }  */
+
+  /* Return statement.  */
+  return __builtin_trap;             /* { dg-error "builtin function" }  */
+
+  (void)a;
+  (void)p;
+  (void)q;
+}
+
+/* Helper declarations to verify that it's possible to take the address
+   of a user-declared function that's also a GCC built-in.  */
+extern int abs (int);
+
+extern __SIZE_TYPE__ strlen (const char*);
+
+/* Taking the address of a builtin with a library "fallback" must be
+   allowed, either using the __builtin_xxx form or the xxx form, when
+   the library fallback is declared either explicitly or implicitly
+   by virtue of first calling the function.  */
+void test_taking_address_of_library_builtin (int i)
+{
+  {
+    typedef int F (int);
+
+    /* Compute the address of libc's abs using the implicitly declared
+       __builtin_abs form (all expressions are valid).  */
+    F *p = __builtin_abs;
+    p = &__builtin_abs;
+    p = *__builtin_abs;
+
+    /* Compute the address of libc's abs declared above.  */
+    p = abs;
+    p = &abs;
+    p = *abs;
+    (void)p;
+  }
+
+  {
+    typedef __SIZE_TYPE__ size_t;
+    typedef size_t F (const char*);
+
+    /* Compute the address of libc's strlen using the implicitly
+       declared __builtin_strlen form.  */
+    F *p = __builtin_strlen;
+    p = &__builtin_strlen;
+    p = *__builtin_strlen;
+
+    /* Compute the address of libc's strlen declared above.  */
+    p = strlen;
+    p = &strlen;
+    p = *strlen;
+    (void)p;
+  }
+
+  {
+    typedef int F (int);
+
+    /* Compute the address of libc's isxxx functions using the implicitly
+       declared __builtin_xxx form.  */
+    F *p = __builtin_isalnum;
+    p = &__builtin_isalpha;
+    p = *__builtin_iscntrl;
+
+    /* According to C90 (see also the discussion in c/67386):
+       If the expression that precedes the parenthesized argument list
+       in a function call consists solely of an identifier, and if no
+       declaration is visible for this identifier, the identifier is
+       implicitly declared exactly as if, in the innermost block
+       containing the function call, the declaration
+       extern int identifier();
+       appeared.  */
+
+    /* Call the functions first to have their declarations "injected"
+       into the enclosing block.  Suppress warnings.  */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wimplicit-function-declaration"
+    i = isalnum (i) || isalpha (i) || iscntrl (i);
+#pragma GCC diagnostic pop
+
+    /* Take the address of the functions relying on their declarations
+       having been implicitly provided by the calls above.  */
+    p = isalnum;
+    p = &isalpha;
+    p = *iscntrl;
+    (void)p;
+  }
+}

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-09-02 22:13                                       ` Martin Sebor
@ 2015-09-03 14:29                                         ` Jason Merrill
  2015-09-03 14:53                                           ` Joseph Myers
  2015-09-03 17:30                                         ` Jakub Jelinek
  1 sibling, 1 reply; 34+ messages in thread
From: Jason Merrill @ 2015-09-03 14:29 UTC (permalink / raw)
  To: Martin Sebor, Joseph Myers; +Cc: Jakub Jelinek, Marek Polacek, Gcc Patch List

The C++ parts are OK.

Jason

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-09-03 14:29                                         ` Jason Merrill
@ 2015-09-03 14:53                                           ` Joseph Myers
  0 siblings, 0 replies; 34+ messages in thread
From: Joseph Myers @ 2015-09-03 14:53 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Martin Sebor, Jakub Jelinek, Marek Polacek, Gcc Patch List

On Thu, 3 Sep 2015, Jason Merrill wrote:

> The C++ parts are OK.

The diagnostic should say "built-in" not "builtin" (see 
codingconventions.html).  The C parts are OK with that change (which will 
require testcases to be updated).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-09-02 22:13                                       ` Martin Sebor
  2015-09-03 14:29                                         ` Jason Merrill
@ 2015-09-03 17:30                                         ` Jakub Jelinek
  2015-09-03 18:28                                           ` Martin Sebor
  1 sibling, 1 reply; 34+ messages in thread
From: Jakub Jelinek @ 2015-09-03 17:30 UTC (permalink / raw)
  To: Martin Sebor; +Cc: Jason Merrill, Joseph Myers, Marek Polacek, Gcc Patch List

On Wed, Sep 02, 2015 at 03:53:07PM -0600, Martin Sebor wrote:
> gcc/ChangeLog
> 2015-09-02  Martin Sebor  <msebor@redhat.com>
> 
> 	PR c/66516
> 	* doc/extend.texi (Other Builtins): Document when the address
> 	of a builtin function can be taken.
> 
> gcc/c-family/ChangeLog
> 2015-09-02  Martin Sebor  <msebor@redhat.com>
> 
> 	PR c/66516
> 	* c-common.h (c_decl_implicit, reject_gcc_builtin): Declare new
> 	functions.
> 	* c-common.c (reject_gcc_builtin): Define.

You've committed empty gcc/builtins.c.orig file, I've removed it, but
please be more careful next time.

> gcc/c/ChangeLog
> 2015-09-02  Martin Sebor  <msebor@redhat.com>
> 
> 	PR c/66516
> 	* c/c-typeck.c (convert_arguments, parser_build_unary_op)

And c/ or cp/ prefixes don't belong to c/ChangeLog or cp/ChangeLog
(also fixed).

	Jakub

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function
  2015-09-03 17:30                                         ` Jakub Jelinek
@ 2015-09-03 18:28                                           ` Martin Sebor
  0 siblings, 0 replies; 34+ messages in thread
From: Martin Sebor @ 2015-09-03 18:28 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jason Merrill, Joseph Myers, Marek Polacek, Gcc Patch List

> You've committed empty gcc/builtins.c.orig file, I've removed it, but
> please be more careful next time.
>
>
> And c/ or cp/ prefixes don't belong to c/ChangeLog or cp/ChangeLog
> (also fixed).
>
> 	Jakub
>

Thank you for fixing that up.

Martin

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2015-09-03 17:41 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-22  0:36 [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function Martin Sebor
2015-06-22 14:11 ` Joseph Myers
2015-06-22 14:41 ` Marek Polacek
2015-06-23  4:38   ` Martin Sebor
2015-06-23 10:29     ` Marek Polacek
2015-06-23 10:39       ` Jakub Jelinek
2015-06-23 15:12         ` Martin Sebor
2015-06-29  7:37           ` Martin Sebor
2015-07-02 14:20             ` Joseph Myers
2015-07-04 22:32               ` Martin Sebor
2015-07-14  3:37                 ` [PING] " Martin Sebor
2015-07-14 15:01                   ` Jason Merrill
2015-07-14 15:07                     ` Jason Merrill
2015-07-14 18:04                     ` Martin Sebor
2015-07-29  7:27                       ` Jason Merrill
2015-07-29 19:07                         ` Martin Sebor
2015-08-03 23:02                           ` Martin Sebor
2015-08-04 15:04                             ` Jason Merrill
2015-08-04 15:58                               ` Jeff Law
2015-08-04 15:19                             ` Joseph Myers
2015-08-28 20:10                         ` Martin Sebor
2015-08-28 20:49                           ` Joseph Myers
2015-08-29  0:57                             ` Martin Sebor
2015-09-01 15:01                               ` Martin Sebor
2015-09-01 17:29                                 ` Joseph Myers
2015-09-01 22:25                                   ` Martin Sebor
2015-09-02  0:20                                     ` Joseph Myers
2015-09-02 15:29                                     ` Jason Merrill
2015-09-02 22:13                                       ` Martin Sebor
2015-09-03 14:29                                         ` Jason Merrill
2015-09-03 14:53                                           ` Joseph Myers
2015-09-03 17:30                                         ` Jakub Jelinek
2015-09-03 18:28                                           ` Martin Sebor
2015-06-22 19:45 ` Marek Polacek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).