diff mbox

[PING] Re: [PATCH] c/66516 - missing diagnostic on taking the address of a builtin function

Message ID 55E5BDA3.5060005@gmail.com
State New
Headers show

Commit Message

Martin Sebor Sept. 1, 2015, 3 p.m. UTC
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

Comments

Joseph Myers Sept. 1, 2015, 5:29 p.m. UTC | #1
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++.
Martin Sebor Sept. 1, 2015, 10:25 p.m. UTC | #2
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
Joseph Myers Sept. 2, 2015, 12:20 a.m. UTC | #3
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++.
Jason Merrill Sept. 2, 2015, 3:29 p.m. UTC | #4
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
diff mbox

Patch

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;
+  }
+}