[PATCH] attribute copy, leaf, weakref and -Wmisisng-attributes (PR 88546)

[PATCH] attribute copy, leaf, weakref and -Wmisisng-attributes (PR 88546)

[Martin Sebor]

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

The enhancement to detect mismatched attributes between function
aliases and their targets triggers (expected) warnings in GCC
builds due to aliases being declared with fewer attributes than
their targets.

Using attribute copy as recommended to copy the attributes from
the target to the alias triggers another warning, this time due
to applying attribute leaf to static functions (the attribute
only applies to extern functions).  This is due to an oversight
in both the handler for attribute copy and in
the -Wmissing-attributes warning.

In addition, the copy attribute handler doesn't account for C11
_Noreturn and C++ throw() specifications, both of which set
the corresponding tree bits but don't attach the synonymous
attribute to it.  This also leads to warnings in GCC builds
(in libgfortran).

The attached patch corrects all of these problems: the attribute
copy handler to avoid copying attribute leaf to declarations of
static functions, and to set the noreturn and nonthrow bits, and
the missing attribute warning to avoid triggering for static
weakref aliases whose targets are decorated wiwth attribute leaf.

With this patch, GCC should build with no -Wmissing-attributes
warnings.

Tested on x86_64-linux.

Martin

[-- Attachment #2: gcc-88546.diff --]
[-- Type: text/x-patch, Size: 5443 bytes --]

PR c/88546 - Copy attribute unusable for weakrefs

gcc/c-family/ChangeLog:

	PR c/88546
	* c-attribs.c (handle_copy_attribute): Avoid copying attribute leaf.

gcc/ChangeLog:

	PR c/88546
	* attribs.c (decls_mismatched_attributes): Avoid warning for attribute
	leaf.

libgcc/ChangeLog:

	PR c/88546
	* gthr-posix.h (__gthrw2): Use attribute copy.

libgfortran/ChangeLog:

	PR c/88546
	* libgfortran.h (iexport2): Use attribute copy.

gcc/testsuite/ChangeLog:

	PR c/88546
	* gcc.dg/attr-copy-6.c: New test.

Index: gcc/attribs.c
===================================================================
--- gcc/attribs.c	(revision 267282)
+++ gcc/attribs.c	(working copy)
@@ -1912,6 +1912,12 @@ decls_mismatched_attributes (tree tmpl, tree decl,
 
   for (unsigned i = 0; blacklist[i]; ++i)
     {
+      /* Attribute leaf only applies to extern functions.  Avoid mentioning
+	 it when it's missing from a static declaration.  */
+      if (!TREE_PUBLIC (decl)
+	  && !strcmp ("leaf", blacklist[i]))
+	continue;
+
       for (unsigned j = 0; j != 2; ++j)
 	{
 	  if (!has_attribute (tmpls[j], tmpl_attrs[j], blacklist[i]))
Index: gcc/c-family/c-attribs.c
===================================================================
--- gcc/c-family/c-attribs.c	(revision 267282)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -2455,6 +2455,12 @@ handle_copy_attribute (tree *node, tree name, tree
 	      || is_attribute_p ("weakref", atname))
 	    continue;
 
+	  /* Aattribute leaf only applies to extern functions.
+	     Avoid copying it to static ones.  */
+	  if (!TREE_PUBLIC (decl)
+	      && is_attribute_p ("leaf", atname))
+	    continue;
+
 	  tree atargs = TREE_VALUE (at);
 	  /* Create a copy of just the one attribute ar AT, including
 	     its argumentsm and add it to DECL.  */
Index: gcc/testsuite/gcc.dg/attr-copy-6.c
===================================================================
--- gcc/testsuite/gcc.dg/attr-copy-6.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/attr-copy-6.c	(working copy)
@@ -0,0 +1,68 @@
+/* PR middle-end/88546 - Copy attribute unusable for weakrefs
+   { dg-do compile }
+   { dg-options "-O2 -Wall" }
+   { dg-require-weak "" } */
+
+#define ATTR(...)   __attribute__ ((__VA_ARGS__))
+#define ASRT(expr)   _Static_assert (expr, #expr)
+
+/* Variable that is local to this translation unit but that can
+   be modified from other units by calling reset_unit_local().  */
+static int unit_local;
+
+void reset_unit_local (void)
+{
+  unit_local = 0;
+}
+
+/* Attribute leaf implies that fleaf() doesn't modify unit_local().  */
+ATTR (leaf, returns_nonnull)
+void* fleaf_retnz (void);
+
+/* Verify both attributes have been applied.  */
+ASRT (__builtin_has_attribute (fleaf_retnz, leaf));
+ASRT (__builtin_has_attribute (fleaf_retnz, returns_nonnull));
+
+/* Verify that attribute leaf has the expected effect.  */
+void call_fleaf_retnz (void)
+{
+  int i = unit_local;
+  void *p = fleaf_retnz ();
+
+  /* Expect both tests to be folded to false and the calls eliminated.  */
+  extern void call_fleaf_retnz_test_leaf_eliminated (void);
+  if (i != unit_local)
+    call_fleaf_retnz_test_leaf_eliminated ();
+
+  extern void call_fleaf_retnz_test_nonnull_eliminated (void);
+  if (p == 0)
+    call_fleaf_retnz_test_nonnull_eliminated ();
+}
+
+
+/* Verify that attribute copy copies the returns_nonnull attribute
+   but doesn't try to copy attribute leaf which only applies to extern
+   function.  */
+static ATTR (copy (fleaf_retnz), weakref ("fleaf_retnz"))
+void* fweakref_fleaf_retnz_copy (void);
+
+ASRT (!__builtin_has_attribute (fweakref_fleaf_retnz_copy, leaf));
+ASRT (__builtin_has_attribute (fweakref_fleaf_retnz_copy, returns_nonnull));
+
+void call_fweakref_fleaf_retnz_copy (void)
+{
+  int i = unit_local;
+  void *p = fweakref_fleaf_retnz_copy ();
+
+  /* Since leaf is not copied, expect the following test not to be
+     folded and the call to be emitted.  */
+  extern void call_fweakref_test_leaf_emitted (void);
+  if (i != unit_local)
+    call_fweakref_test_leaf_emitted ();
+
+  /* Expect the following test to be folded to false and the call
+     eliminated.  */
+  extern void call_fweakref_fleaf_nonnull_eliminated (void);
+  if (p == 0)
+    call_fweakref_fleaf_nonnull_eliminated ();
+}
Index: libgcc/gthr-posix.h
===================================================================
--- libgcc/gthr-posix.h	(revision 267282)
+++ libgcc/gthr-posix.h	(working copy)
@@ -87,7 +87,8 @@ typedef struct timespec __gthread_time_t;
 #  define __gthrw_pragma(pragma)
 # endif
 # define __gthrw2(name,name2,type) \
-  static __typeof(type) name __attribute__ ((__weakref__(#name2))); \
+  static __typeof(type) name \
+    __attribute__ ((__weakref__(#name2), copy (type))); \
   __gthrw_pragma(weak type)
 # define __gthrw_(name) __gthrw_ ## name
 #else
Index: libgfortran/libgfortran.h
===================================================================
--- libgfortran/libgfortran.h	(revision 267282)
+++ libgfortran/libgfortran.h	(working copy)
@@ -202,7 +202,7 @@ extern int __mingw_snprintf (char *, size_t, const
 # define iexport(x)		iexport1(x, IPREFIX(x))
 # define iexport1(x,y)		iexport2(x,y)
 # define iexport2(x,y) \
-	extern __typeof(x) PREFIX(x) __attribute__((__alias__(#y)))
+  extern __typeof(x) PREFIX(x) __attribute__((__alias__(#y), __copy__ (x)))
 #else
 # define export_proto(x)	sym_rename(x, PREFIX(x))
 # define export_proto_np(x)	extern char swallow_semicolon

Re: [PATCH] attribute copy, leaf, weakref and -Wmisisng-attributes (PR 88546)

[Jakub Jelinek]

Hi!

I think the main question is whether we should accept leaf attribute
on weakrefs, despite them being marked as !TREE_PUBLIC.

I know we haven't allowed that until now, but weakrefs are weirdo things
which have both static and external effects, static for that they are a
local alias and external for being actually aliases to (usually) external
functions.  If we add a weakref for some function declared as leaf,
it is unnecessarily pessimizing when we don't allow the leaf attribute on
the weakref.

Your patch looks reasonable to me to revert to previous state, but if we
decide to change the above, it would need to change.

On Thu, Dec 20, 2018 at 08:45:03PM -0700, Martin Sebor wrote:
> --- gcc/c-family/c-attribs.c	(revision 267282)
> +++ gcc/c-family/c-attribs.c	(working copy)
> @@ -2455,6 +2455,12 @@ handle_copy_attribute (tree *node, tree name, tree
>  	      || is_attribute_p ("weakref", atname))
>  	    continue;
>  
> +	  /* Aattribute leaf only applies to extern functions.
> +	     Avoid copying it to static ones.  */

s/Aatribute/Attribute/

	Jakub

Re: [PATCH] attribute copy, leaf, weakref and -Wmisisng-attributes (PR 88546)

[Martin Sebor]

On 12/21/18 3:05 AM, Jakub Jelinek wrote:
> Hi!
> 
> I think the main question is whether we should accept leaf attribute
> on weakrefs, despite them being marked as !TREE_PUBLIC.
> 
> I know we haven't allowed that until now, but weakrefs are weirdo things
> which have both static and external effects, static for that they are a
> local alias and external for being actually aliases to (usually) external
> functions.  If we add a weakref for some function declared as leaf,
> it is unnecessarily pessimizing when we don't allow the leaf attribute on
> the weakref.
> 
> Your patch looks reasonable to me to revert to previous state, but if we
> decide to change the above, it would need to change.

Yes.  I confess I don't fully understand the rationale for disallowing
attribute leaf on extern functions, or the implications of allowing it
again.  It's accepted by GCC 4.1 but in r108074 it was made an error.
(GCC 4.1 also requires the declaration to be extern and gives an error
for static.)  The description in the patch submission says it's because:

   - requires that 'weakref' is attached only to a static object,
     because that's all that the object file formats support; and
   - makes it work on Darwin, or at least makes the testcases pass.

   https://gcc.gnu.org/ml/gcc-patches/2005-12/msg00375.html

The text added to the manual makes it sounds like it was thought to be
a limitation that could be removed in the future:

   At present, a declaration to which @code{weakref} is attached can
   only be @code{static}.

I leave it up to you and others to decide if it's possible to accept
it on externs again.

That said, I'm also not sure the warning is necessarily the best way
to deal with the attribute mismatches in these cases (declarations
of aliases in .c files).  Wouldn't it make more sense to copy
the attributes from targets to their aliases unconditionally?

Joseph, any thoughts based on your experience with the warning (and
attribute copy) in Glibc?

> On Thu, Dec 20, 2018 at 08:45:03PM -0700, Martin Sebor wrote:
>> --- gcc/c-family/c-attribs.c	(revision 267282)
>> +++ gcc/c-family/c-attribs.c	(working copy)
>> @@ -2455,6 +2455,12 @@ handle_copy_attribute (tree *node, tree name, tree
>>   	      || is_attribute_p ("weakref", atname))
>>   	    continue;
>>   
>> +	  /* Aattribute leaf only applies to extern functions.
>> +	     Avoid copying it to static ones.  */
> 
> s/Aatribute/Attribute/

Fixed, thanks.

Martin

Re: [PATCH] attribute copy, leaf, weakref and -Wmisisng-attributes (PR 88546)

[Joseph Myers]

On Fri, 21 Dec 2018, Martin Sebor wrote:

> That said, I'm also not sure the warning is necessarily the best way
> to deal with the attribute mismatches in these cases (declarations
> of aliases in .c files).  Wouldn't it make more sense to copy
> the attributes from targets to their aliases unconditionally?
> 
> Joseph, any thoughts based on your experience with the warning (and
> attribute copy) in Glibc?

My expectation is that the normal case is that the same attributes should 
apply to all names for a function (except for the ones already excluded 
from copying because they're properties of a symbol rather than of the 
function that symbol points to), but there may be niche cases where you 
deliberately want calls to different names for a function to be handled 
differently based on different attributes (and so have deliberately 
different declarations for both names in a header which is included in the 
translation unit defining the function and alias, say).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] attribute copy, leaf, weakref and -Wmisisng-attributes (PR 88546)

[Martin Sebor]

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

The first revision of the patch was missing a test and didn't
completely or completely correctly handle attribute noreturn.
Attached is an update with the test included and the omission
and bug fixed.

I think it makes sense to consider the patch independently of
the question whether weakrefs should be extern.  That change can
be made separately, with only minor tweaks to the attribute copy
handling and the warning.  None of the other fixes in this patch
(precipitated by more thorough testing) should be affected by it.

Martin

On 12/20/18 8:45 PM, Martin Sebor wrote:
> The enhancement to detect mismatched attributes between function
> aliases and their targets triggers (expected) warnings in GCC
> builds due to aliases being declared with fewer attributes than
> their targets.
> 
> Using attribute copy as recommended to copy the attributes from
> the target to the alias triggers another warning, this time due
> to applying attribute leaf to static functions (the attribute
> only applies to extern functions).  This is due to an oversight
> in both the handler for attribute copy and in
> the -Wmissing-attributes warning.
> 
> In addition, the copy attribute handler doesn't account for C11
> _Noreturn and C++ throw() specifications, both of which set
> the corresponding tree bits but don't attach the synonymous
> attribute to it.  This also leads to warnings in GCC builds
> (in libgfortran).
> 
> The attached patch corrects all of these problems: the attribute
> copy handler to avoid copying attribute leaf to declarations of
> static functions, and to set the noreturn and nonthrow bits, and
> the missing attribute warning to avoid triggering for static
> weakref aliases whose targets are decorated wiwth attribute leaf.
> 
> With this patch, GCC should build with no -Wmissing-attributes
> warnings.
> 
> Tested on x86_64-linux.
> 
> Martin


[-- Attachment #2: gcc-88546.diff --]
[-- Type: text/x-patch, Size: 13002 bytes --]

PR c/88546 - Copy attribute unusable for weakrefs

gcc/c-family/ChangeLog:

	PR c/88546
	* c-attribs.c (handle_copy_attribute): Avoid copying attribute leaf.
	Handle C++ empty throw specification and C11 _Noreturn.
	(has_attribute): Also handle C11 _Noreturn.

gcc/ChangeLog:

	PR c/88546
	* attribs.c (decls_mismatched_attributes): Avoid warning for attribute
	leaf.

libgcc/ChangeLog:

	PR c/88546
	* gthr-posix.h (__gthrw2): Use attribute copy.

libgfortran/ChangeLog:

	PR c/88546
	* libgfortran.h (iexport2): Use attribute copy.

gcc/testsuite/ChangeLog:

	PR c/88546
	* g++.dg/ext/attr-copy.C: New test.
	* gcc.dg/attr-copy-4.c: Disable macro expansion tracking.
	* gcc.dg/attr-copy-6.c: New test.
	* gcc.dg/attr-copy-7.c: New test.

Index: gcc/attribs.c
===================================================================
--- gcc/attribs.c	(revision 267301)
+++ gcc/attribs.c	(working copy)
@@ -1912,6 +1912,12 @@ decls_mismatched_attributes (tree tmpl, tree decl,
 
   for (unsigned i = 0; blacklist[i]; ++i)
     {
+      /* Attribute leaf only applies to extern functions.  Avoid mentioning
+	 it when it's missing from a static declaration.  */
+      if (!TREE_PUBLIC (decl)
+	  && !strcmp ("leaf", blacklist[i]))
+	continue;
+
       for (unsigned j = 0; j != 2; ++j)
 	{
 	  if (!has_attribute (tmpls[j], tmpl_attrs[j], blacklist[i]))
Index: gcc/c-family/c-attribs.c
===================================================================
--- gcc/c-family/c-attribs.c	(revision 267301)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -2455,6 +2455,12 @@ handle_copy_attribute (tree *node, tree name, tree
 	      || is_attribute_p ("weakref", atname))
 	    continue;
 
+	  /* Aattribute leaf only applies to extern functions.
+	     Avoid copying it to static ones.  */
+	  if (!TREE_PUBLIC (decl)
+	      && is_attribute_p ("leaf", atname))
+	    continue;
+
 	  tree atargs = TREE_VALUE (at);
 	  /* Create a copy of just the one attribute ar AT, including
 	     its argumentsm and add it to DECL.  */
@@ -2472,7 +2478,19 @@ handle_copy_attribute (tree *node, tree name, tree
       return NULL_TREE;
     }
 
+  /* A function declared with attribute nothrow has the attribute
+     attached to it, but a C++ throw() function does not.  */
+  if (TREE_NOTHROW (ref))
+    TREE_NOTHROW (decl) = true;
+
+  /* Similarly, a function declared with attribute noreturn has it
+     attached on to it, but a C11 _Noreturn function does not.  */
   tree reftype = ref;
+  if (DECL_P (ref)
+      && TREE_THIS_VOLATILE (ref)
+      && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (reftype)))
+    TREE_THIS_VOLATILE (decl) = true;
+
   if (DECL_P (ref) || EXPR_P (ref))
     reftype = TREE_TYPE (ref);
 
@@ -2479,6 +2497,9 @@ handle_copy_attribute (tree *node, tree name, tree
   if (POINTER_TYPE_P (reftype))
     reftype = TREE_TYPE (reftype);
 
+  if (!TYPE_P (reftype))
+    return NULL_TREE;
+
   tree attrs = TYPE_ATTRIBUTES (reftype);
 
   /* Copy type attributes from REF to DECL.  */
@@ -4188,6 +4209,15 @@ has_attribute (location_t atloc, tree t, tree attr
 	      if (expr && DECL_P (expr))
 		found_match = TREE_READONLY (expr);
 	    }
+	  else if (!strcmp ("noreturn", namestr))
+	    {
+	      /* C11 _Noreturn sets the volatile bit without attaching
+		 an attribute to the decl.  */
+	      if (expr
+		  && DECL_P (expr)
+		  && FUNC_OR_METHOD_TYPE_P (TREE_TYPE (expr)))
+		found_match = TREE_THIS_VOLATILE (expr);
+	    }
 	  else if (!strcmp ("pure", namestr))
 	    {
 	      if (expr && DECL_P (expr))
Index: gcc/testsuite/g++.dg/ext/attr-copy.C
===================================================================
--- gcc/testsuite/g++.dg/ext/attr-copy.C	(nonexistent)
+++ gcc/testsuite/g++.dg/ext/attr-copy.C	(working copy)
@@ -0,0 +1,82 @@
+/* PR middle-end/88546 - Copy attribute unusable for weakrefs
+   { dg-do compile }
+   { dg-options "-O1 -Wall -fdump-tree-optimized" }
+   { dg-require-weak "" } */
+
+#define ATTR(...)   __attribute__ ((__VA_ARGS__))
+#define ASRT(expr)   _Static_assert (expr, #expr)
+
+extern "C" {
+
+  ATTR (leaf, nothrow) int
+  fnothrow ()
+  {
+    return 0;
+  }
+
+  static __typeof__ (fnothrow)
+    ATTR (weakref ("fnothrow"), copy (fnothrow))
+    alias_fnothrow;
+
+
+  ATTR (leaf) int
+  fthrow_none () throw ()
+  {
+    return 0;
+  }
+
+  // Verify that no warning is issued for the alias having less
+  // restrictive attributes than the target: nothrow.
+  static __typeof (fthrow_none)
+    ATTR (weakref ("fthrow_none"), copy (fthrow_none))
+    alias_fthrow_none;
+
+  // Same as above but with no definition of the target.
+  ATTR (leaf) int
+  fthrow_none_nodef () throw ();
+
+  static __typeof (fthrow_none_nodef)
+    ATTR (weakref ("fthrow_none_nodef"), copy (fthrow_none_nodef))
+    alias_fthrow_none_nodef;
+
+  // And again but without using typeof to make sure the nothrow
+  // bit is copied by attribute copy alone.
+  static int
+  ATTR (weakref ("fthrow_none_nodef"), copy (fthrow_none_nodef))
+    alias_fthrow_none_nodef_func ();
+}
+
+
+struct UsrClass
+{
+  ~UsrClass ();
+};
+
+// Verify that the nothrow attribute/bit was copied to the alias and
+// that no exception handling code is emitted in any of these calls.
+
+int call_alias_fnothrow ()
+{
+  UsrClass usr;
+  return alias_fnothrow ();
+}
+
+int call_alias_fthrow_none ()
+{
+  UsrClass usr;
+  return alias_fthrow_none ();
+}
+
+int call_alias_fthrow_none_nodef ()
+{
+  UsrClass usr;
+  return alias_fthrow_none_nodef ();
+}
+
+int call_alias_fthrow_none_nodef_func ()
+{
+  UsrClass usr;
+  return alias_fthrow_none_nodef_func ();
+}
+
+// { dg-final { scan-tree-dump-not "__builtin_unwind" "optimized" } }
Index: gcc/testsuite/gcc.dg/attr-copy-4.c
===================================================================
--- gcc/testsuite/gcc.dg/attr-copy-4.c	(revision 267301)
+++ gcc/testsuite/gcc.dg/attr-copy-4.c	(working copy)
@@ -1,7 +1,7 @@
 /* PR middle-end/81824 - Warn for missing attributes with function aliases
    Exercise attribute copy for types.
    { dg-do compile }
-   { dg-options "-O2 -Wall" } */
+   { dg-options "-O2 -Wall -ftrack-macro-expansion=0" } */
 
 #define Assert(expr)   typedef char AssertExpr[2 * !!(expr) - 1]
 
Index: gcc/testsuite/gcc.dg/attr-copy-6.c
===================================================================
--- gcc/testsuite/gcc.dg/attr-copy-6.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/attr-copy-6.c	(working copy)
@@ -0,0 +1,93 @@
+/* PR middle-end/88546 - Copy attribute unusable for weakrefs
+   { dg-do compile }
+   { dg-options "-O2 -Wall" }
+   { dg-require-weak "" } */
+
+#define ATTR(...)   __attribute__ ((__VA_ARGS__))
+#define ASRT(expr)   _Static_assert (expr, #expr)
+
+/* Variable that is local to this translation unit but that can
+   be modified from other units by calling reset_unit_local().  */
+static int unit_local;
+
+void reset_unit_local (void)
+{
+  unit_local = 0;
+}
+
+/* Attribute leaf implies that fleaf() doesn't modify unit_local().  */
+ATTR (leaf, returns_nonnull)
+void* fleaf_retnz (void);
+
+/* Verify both attributes have been applied.  */
+ASRT (__builtin_has_attribute (fleaf_retnz, leaf));
+ASRT (__builtin_has_attribute (fleaf_retnz, returns_nonnull));
+
+/* Verify that attribute leaf has the expected effect.  */
+void call_fleaf_retnz (void)
+{
+  int i = unit_local;
+  void *p = fleaf_retnz ();
+
+  /* Expect both tests to be folded to false and the calls eliminated.  */
+  extern void call_fleaf_retnz_test_leaf_eliminated (void);
+  if (i != unit_local)
+    call_fleaf_retnz_test_leaf_eliminated ();
+
+  extern void call_fleaf_retnz_test_nonnull_eliminated (void);
+  if (p == 0)
+    call_fleaf_retnz_test_nonnull_eliminated ();
+}
+
+
+/* Verify that attribute copy copies the returns_nonnull attribute
+   but doesn't try to copy attribute leaf which only applies to extern
+   function.  */
+static ATTR (copy (fleaf_retnz), weakref ("fleaf_retnz"))
+void* fweakref_fleaf_retnz_copy (void);
+
+ASRT (!__builtin_has_attribute (fweakref_fleaf_retnz_copy, leaf));
+ASRT (__builtin_has_attribute (fweakref_fleaf_retnz_copy, returns_nonnull));
+
+void call_fweakref_fleaf_retnz_copy (void)
+{
+  int i = unit_local;
+  void *p = fweakref_fleaf_retnz_copy ();
+
+  /* Since leaf is not copied, expect the following test not to be
+     folded and the call to be emitted.  */
+  extern void call_fweakref_test_leaf_emitted (void);
+  if (i != unit_local)
+    call_fweakref_test_leaf_emitted ();
+
+  /* Expect the following test to be folded to false and the call
+     eliminated.  */
+  extern void call_fweakref_fleaf_nonnull_eliminated (void);
+  if (p == 0)
+    call_fweakref_fleaf_nonnull_eliminated ();
+}
+
+/* This is reduced from libgfortran/runtime/error.c.  Verify it
+   doesn't trigger warnings and that the noreturn bit is copied
+   to the alias by verifying that calling the alias in a non-void
+   function with no return statement isn't diagnosed.  */
+
+extern _Noreturn void fnoreturn (void);
+
+extern __typeof (fnoreturn)
+  ATTR (visibility ("hidden"))
+  fnoreturn __asm__ ("fnoreturn_name");
+
+void fnoreturn (void)
+{
+  __builtin_abort ();
+}
+
+extern __typeof (fnoreturn)
+  ATTR (alias ("fnoreturn_name"), copy (fnoreturn))
+  fnoreturn_alias;
+
+int call_fnoreturn_alias (void)
+{
+  fnoreturn_alias ();
+}
Index: gcc/testsuite/gcc.dg/attr-copy-7.c
===================================================================
--- gcc/testsuite/gcc.dg/attr-copy-7.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/attr-copy-7.c	(working copy)
@@ -0,0 +1,76 @@
+/* PR middle-end/88546 - Copy attribute unusable for weakrefs
+   Verify that attribute noreturn (represented as volatile on function
+   decls) is interpreted correctly and doesn't affect variables.
+   { dg-do compile }
+   { dg-options "-O1 -Wall -fdump-tree-optimized" }*/
+
+#define ATTR(...)   __attribute__ ((__VA_ARGS__))
+#define ASRT(expr)   _Static_assert (expr, #expr)
+
+ATTR (noreturn) void fnoreturn (void);
+ATTR (copy (fnoreturn)) void fnoreturn_copy (void);
+ASRT (__builtin_has_attribute (fnoreturn_copy, noreturn));
+
+int call_fnoreturn_copy (void)
+{
+  fnoreturn_copy ();
+  fnoreturn_copy ();   // should be eliminated
+}
+
+// { dg-final { scan-tree-dump-times "fnoreturn_copy \\(\\);" 1 "optimized" } }
+
+
+_Noreturn void f_Noreturn (void);
+ATTR (copy (f_Noreturn)) void f_Noreturn_copy (void);
+ASRT (__builtin_has_attribute (f_Noreturn_copy, noreturn));
+
+int call_f_Noreturn_copy (void)
+{
+  f_Noreturn_copy ();
+  f_Noreturn_copy ();   // should be eliminated
+}
+
+// { dg-final { scan-tree-dump-times "f_Noreturn_copy \\(\\);" 1 "optimized" } }
+
+
+// Verify the combination of both is accepted and works too,
+// just for fun.
+ATTR (noreturn) _Noreturn void fnoreturn_Noreturn (void);
+ATTR (copy (fnoreturn_Noreturn)) void fnoreturn_Noreturn_copy (void);
+ASRT (__builtin_has_attribute (fnoreturn_Noreturn_copy, noreturn));
+
+int call_fnoreturn_Noreturn_copy (void)
+{
+  fnoreturn_Noreturn_copy ();
+  fnoreturn_Noreturn_copy ();   // should be eliminated
+}
+
+// { dg-final { scan-tree-dump-times "fnoreturn_Noreturn_copy \\(\\);" 1 "optimized" } }
+
+
+typedef void func_t (void);
+
+ATTR (noreturn) func_t func_noreturn;
+ATTR (copy (func_noreturn)) func_t func_noreturn_copy;
+ASRT (__builtin_has_attribute (func_noreturn_copy, noreturn));
+
+int call_func_noreturn_copy (void)
+{
+  func_noreturn_copy ();
+  func_noreturn_copy ();   // should be eliminated
+}
+
+// { dg-final { scan-tree-dump-times "func_noreturn_copy \\(\\);" 1 "optimized" } }
+
+
+// Finally, verify that the volatile bit isn't copied for variables.
+extern volatile int vi;
+
+int read_nonvolatile (void)
+{
+  ATTR (copy (vi)) int i = 0;
+
+  return i + i;   // should be folded to return 0;
+}
+
+// { dg-final { scan-tree-dump-times "return 0;" 1 "optimized" } }
Index: libgcc/gthr-posix.h
===================================================================
--- libgcc/gthr-posix.h	(revision 267301)
+++ libgcc/gthr-posix.h	(working copy)
@@ -87,7 +87,8 @@ typedef struct timespec __gthread_time_t;
 #  define __gthrw_pragma(pragma)
 # endif
 # define __gthrw2(name,name2,type) \
-  static __typeof(type) name __attribute__ ((__weakref__(#name2))); \
+  static __typeof(type) name \
+    __attribute__ ((__weakref__(#name2), copy (type))); \
   __gthrw_pragma(weak type)
 # define __gthrw_(name) __gthrw_ ## name
 #else
Index: libgfortran/libgfortran.h
===================================================================
--- libgfortran/libgfortran.h	(revision 267301)
+++ libgfortran/libgfortran.h	(working copy)
@@ -202,7 +202,7 @@ extern int __mingw_snprintf (char *, size_t, const
 # define iexport(x)		iexport1(x, IPREFIX(x))
 # define iexport1(x,y)		iexport2(x,y)
 # define iexport2(x,y) \
-	extern __typeof(x) PREFIX(x) __attribute__((__alias__(#y)))
+  extern __typeof(x) PREFIX(x) __attribute__((__alias__(#y), __copy__ (x)))
 #else
 # define export_proto(x)	sym_rename(x, PREFIX(x))
 # define export_proto_np(x)	extern char swallow_semicolon

Re: [PATCH] attribute copy, leaf, weakref and -Wmisisng-attributes (PR 88546)

[Jakub Jelinek]

On Fri, Dec 21, 2018 at 04:50:47PM -0700, Martin Sebor wrote:
> The first revision of the patch was missing a test and didn't
> completely or completely correctly handle attribute noreturn.
> Attached is an update with the test included and the omission
> and bug fixed.
> 
> I think it makes sense to consider the patch independently of
> the question whether weakrefs should be extern.  That change can

Weakrefs shouldn't be extern, that is what we were using initially and
changed to static.  At this point we can't change that again IMNSHO.

	Jakub

Re: [PATCH] attribute copy, leaf, weakref and -Wmisisng-attributes (PR 88546)

[Martin Sebor]

On 12/21/18 5:16 PM, Jakub Jelinek wrote:
> On Fri, Dec 21, 2018 at 04:50:47PM -0700, Martin Sebor wrote:
>> The first revision of the patch was missing a test and didn't
>> completely or completely correctly handle attribute noreturn.
>> Attached is an update with the test included and the omission
>> and bug fixed.
>>
>> I think it makes sense to consider the patch independently of
>> the question whether weakrefs should be extern.  That change can
> 
> Weakrefs shouldn't be extern, that is what we were using initially and
> changed to static.  At this point we can't change that again IMNSHO.

Sorry, I mixed things up.  What I meant is "independently of
the question whether leaf should be accepted on extern declarations."

Martin

Re: [PATCH] attribute copy, leaf, weakref and -Wmisisng-attributes (PR 88546)

[Martin Sebor]

Ping: https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01616.html

(The first sentence in the second paragraph below should have
read "...independently of the question whether leaf should be
accepted on extern declarations.")

On 12/21/18 4:50 PM, Martin Sebor wrote:
> The first revision of the patch was missing a test and didn't
> completely or completely correctly handle attribute noreturn.
> Attached is an update with the test included and the omission
> and bug fixed.
> 
> I think it makes sense to consider the patch independently of
> the question whether weakrefs should be extern.  That change can
> be made separately, with only minor tweaks to the attribute copy
> handling and the warning.  None of the other fixes in this patch
> (precipitated by more thorough testing) should be affected by it.
> 
> Martin
> 
> On 12/20/18 8:45 PM, Martin Sebor wrote:
>> The enhancement to detect mismatched attributes between function
>> aliases and their targets triggers (expected) warnings in GCC
>> builds due to aliases being declared with fewer attributes than
>> their targets.
>>
>> Using attribute copy as recommended to copy the attributes from
>> the target to the alias triggers another warning, this time due
>> to applying attribute leaf to static functions (the attribute
>> only applies to extern functions).  This is due to an oversight
>> in both the handler for attribute copy and in
>> the -Wmissing-attributes warning.
>>
>> In addition, the copy attribute handler doesn't account for C11
>> _Noreturn and C++ throw() specifications, both of which set
>> the corresponding tree bits but don't attach the synonymous
>> attribute to it.  This also leads to warnings in GCC builds
>> (in libgfortran).
>>
>> The attached patch corrects all of these problems: the attribute
>> copy handler to avoid copying attribute leaf to declarations of
>> static functions, and to set the noreturn and nonthrow bits, and
>> the missing attribute warning to avoid triggering for static
>> weakref aliases whose targets are decorated wiwth attribute leaf.
>>
>> With this patch, GCC should build with no -Wmissing-attributes
>> warnings.
>>
>> Tested on x86_64-linux.
>>
>> Martin
> 

Re: [PATCH] attribute copy, leaf, weakref and -Wmisisng-attributes (PR 88546)

[Martin Sebor]

Ping: https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01616.html

(The first sentence in the second paragraph below should have
read "...independently of the question whether leaf should be
accepted on extern declarations.")

On 12/21/18 4:50 PM, Martin Sebor wrote:
> The first revision of the patch was missing a test and didn't
> completely or completely correctly handle attribute noreturn.
> Attached is an update with the test included and the omission
> and bug fixed.
> 
> I think it makes sense to consider the patch independently of
> the question whether weakrefs should be extern.  That change can
> be made separately, with only minor tweaks to the attribute copy
> handling and the warning.  None of the other fixes in this patch
> (precipitated by more thorough testing) should be affected by it.
> 
> Martin
> 
> On 12/20/18 8:45 PM, Martin Sebor wrote:
>> The enhancement to detect mismatched attributes between function
>> aliases and their targets triggers (expected) warnings in GCC
>> builds due to aliases being declared with fewer attributes than
>> their targets.
>>
>> Using attribute copy as recommended to copy the attributes from
>> the target to the alias triggers another warning, this time due
>> to applying attribute leaf to static functions (the attribute
>> only applies to extern functions).  This is due to an oversight
>> in both the handler for attribute copy and in
>> the -Wmissing-attributes warning.
>>
>> In addition, the copy attribute handler doesn't account for C11
>> _Noreturn and C++ throw() specifications, both of which set
>> the corresponding tree bits but don't attach the synonymous
>> attribute to it.  This also leads to warnings in GCC builds
>> (in libgfortran).
>>
>> The attached patch corrects all of these problems: the attribute
>> copy handler to avoid copying attribute leaf to declarations of
>> static functions, and to set the noreturn and nonthrow bits, and
>> the missing attribute warning to avoid triggering for static
>> weakref aliases whose targets are decorated wiwth attribute leaf.
>>
>> With this patch, GCC should build with no -Wmissing-attributes
>> warnings.
>>
>> Tested on x86_64-linux.
>>
>> Martin
> 

Re: [PATCH] attribute copy, leaf, weakref and -Wmisisng-attributes (PR 88546)

[Joseph Myers]

On Thu, 3 Jan 2019, Martin Sebor wrote:

> Ping: https://gcc.gnu.org/ml/gcc-patches/2018-12/msg01616.html

This is OK (with the typo fix Jakub noted).

-- 
Joseph S. Myers
joseph@codesourcery.com