diff mbox

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

Message ID 55E0BD94.2060105@gmail.com
State New
Headers show

Commit Message

Martin Sebor Aug. 28, 2015, 7:59 p.m. UTC
>> 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

Comments

Joseph Myers Aug. 28, 2015, 8:09 p.m. UTC | #1
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.
Martin Sebor Aug. 28, 2015, 9:33 p.m. UTC | #2
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;
      ^
diff mbox

Patch

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);