From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4697 invoked by alias); 17 Jun 2019 18:11:06 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 4687 invoked by uid 89); 17 Jun 2019 18:11:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 17 Jun 2019 18:11:03 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BD5CF31628ED; Mon, 17 Jun 2019 18:10:56 +0000 (UTC) Received: from redhat.com (unknown [10.20.4.51]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C27107DF6A; Mon, 17 Jun 2019 18:10:54 +0000 (UTC) Date: Mon, 17 Jun 2019 18:11:00 -0000 From: Marek Polacek To: Martin Sebor Cc: Jakub Jelinek , GCC Patches , Jason Merrill Subject: Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) Message-ID: <20190617181053.GR5989@redhat.com> References: <20190615142917.GI5989@redhat.com> <20190615143326.GK19695@tucnak> <20190615143913.GJ5989@redhat.com> <20190616161037.GK5989@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) X-SW-Source: 2019-06/txt/msg00981.txt.bz2 On Mon, Jun 17, 2019 at 09:02:17AM -0600, Martin Sebor wrote: > > diff --git gcc/cp/tree.c gcc/cp/tree.c > > index cd021b7f594..bb695e14e73 100644 > > --- gcc/cp/tree.c > > +++ gcc/cp/tree.c > > @@ -4453,6 +4453,8 @@ const struct attribute_spec std_attribute_table[] = > > handle_likeliness_attribute, attr_cold_hot_exclusions }, > > { "unlikely", 0, 0, false, false, false, false, > > handle_likeliness_attribute, attr_cold_hot_exclusions }, > > + { "noreturn", 0, 0, true, false, false, false, > > + handle_noreturn_attribute, NULL }, > ^^^^ > > The GNU attribute is made mutually exclusive with a bunch of other > attributes (e.g., malloc or warn_unused_result) by setting the last > member to the array of exclusive attribute. Does the change preserve > this relationship some other way? Oop, no, that is a bug. I meant to go back to that, but I'd forgotten to add an XXX comment as I'm wont to, and the testsuite doesn't test that, so it slipped. Fixed & new test added. Thanks for catching it. Also added a test for the scenario Jakub pointed out in the other mail. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-06-17 Marek Polacek PR c++/60364 - noreturn after first decl not diagnosed. * attribs.c (get_attribute_namespace): No longer static. (decl_attributes): Avoid shadowing. Preserve the C++11 form for C++11 attributes. (attr_noreturn_exclusions): Make it extern. * attribs.h (get_attribute_namespace): Declare. * tree-inline.c (function_attribute_inlinable_p): Use get_attribute_name. * c-attribs.c (handle_noreturn_attribute): No longer static. * c-common.h (handle_noreturn_attribute, attr_noreturn_exclusions): Declare. * c-format.c (check_function_format): Use get_attribute_name. * decl.c (duplicate_decls): Give an error when a function is declared [[noreturn]] after its first declaration. * parser.c (cp_parser_std_attribute): Don't treat C++11 noreturn attribute as equivalent to GNU's. * tree.c (std_attribute_table): Add noreturn. * g++.dg/warn/noreturn-8.C: New test. * g++.dg/warn/noreturn-9.C: New test. * g++.dg/warn/noreturn-10.C: New test. * g++.dg/warn/noreturn-11.C: New test. diff --git gcc/attribs.c gcc/attribs.c index 4441922543f..8e540165597 100644 --- gcc/attribs.c +++ gcc/attribs.c @@ -340,7 +340,7 @@ lookup_attribute_spec (const_tree name) Please read the comments of cxx11_attribute_p to understand the format of attributes. */ -static tree +tree get_attribute_namespace (const_tree attr) { if (cxx11_attribute_p (attr)) @@ -469,7 +469,6 @@ tree decl_attributes (tree *node, tree attributes, int flags, tree last_decl /* = NULL_TREE */) { - tree a; tree returned_attrs = NULL_TREE; if (TREE_TYPE (*node) == error_mark_node || attributes == error_mark_node) @@ -548,22 +547,23 @@ decl_attributes (tree *node, tree attributes, int flags, /* Note that attributes on the same declaration are not necessarily in the same order as in the source. */ - for (a = attributes; a; a = TREE_CHAIN (a)) + for (tree attr = attributes; attr; attr = TREE_CHAIN (attr)) { - tree ns = get_attribute_namespace (a); - tree name = get_attribute_name (a); - tree args = TREE_VALUE (a); + tree ns = get_attribute_namespace (attr); + tree name = get_attribute_name (attr); + tree args = TREE_VALUE (attr); tree *anode = node; const struct attribute_spec *spec = lookup_scoped_attribute_spec (ns, name); int fn_ptr_quals = 0; tree fn_ptr_tmp = NULL_TREE; + const bool cxx11_attr_p = cxx11_attribute_p (attr); if (spec == NULL) { if (!(flags & (int) ATTR_FLAG_BUILT_IN)) { - if (ns == NULL_TREE || !cxx11_attribute_p (a)) + if (ns == NULL_TREE || !cxx11_attr_p) warning (OPT_Wattributes, "%qE attribute directive ignored", name); else @@ -584,7 +584,7 @@ decl_attributes (tree *node, tree attributes, int flags, gcc_assert (is_attribute_p (spec->name, name)); if (TYPE_P (*node) - && cxx11_attribute_p (a) + && cxx11_attr_p && !(flags & ATTR_FLAG_TYPE_IN_PLACE)) { /* This is a c++11 attribute that appertains to a @@ -707,8 +707,7 @@ decl_attributes (tree *node, tree attributes, int flags, if (spec->handler != NULL) { - int cxx11_flag = - cxx11_attribute_p (a) ? ATTR_FLAG_CXX11 : 0; + int cxx11_flag = (cxx11_attr_p ? ATTR_FLAG_CXX11 : 0); /* Pass in an array of the current declaration followed by the last pushed/merged declaration if one exists. @@ -756,17 +755,23 @@ decl_attributes (tree *node, tree attributes, int flags, if (a == NULL_TREE) { /* This attribute isn't already in the list. */ + tree r; + /* Preserve the C++11 form. */ + if (cxx11_attr_p) + r = tree_cons (build_tree_list (ns, name), args, old_attrs); + else + r = tree_cons (name, args, old_attrs); + if (DECL_P (*anode)) - DECL_ATTRIBUTES (*anode) = tree_cons (name, args, old_attrs); + DECL_ATTRIBUTES (*anode) = r; else if (flags & (int) ATTR_FLAG_TYPE_IN_PLACE) { - TYPE_ATTRIBUTES (*anode) = tree_cons (name, args, old_attrs); + TYPE_ATTRIBUTES (*anode) = r; /* If this is the main variant, also push the attributes out to the other variants. */ if (*anode == TYPE_MAIN_VARIANT (*anode)) { - tree variant; - for (variant = *anode; variant; + for (tree variant = *anode; variant; variant = TYPE_NEXT_VARIANT (variant)) { if (TYPE_ATTRIBUTES (variant) == old_attrs) @@ -780,9 +785,7 @@ decl_attributes (tree *node, tree attributes, int flags, } } else - *anode = build_type_attribute_variant (*anode, - tree_cons (name, args, - old_attrs)); + *anode = build_type_attribute_variant (*anode, r); } } diff --git gcc/attribs.h gcc/attribs.h index 83ecbbbeec9..23a7321e04a 100644 --- gcc/attribs.h +++ gcc/attribs.h @@ -35,6 +35,7 @@ extern tree decl_attributes (tree *, tree, int, tree = NULL_TREE); extern bool cxx11_attribute_p (const_tree); extern tree get_attribute_name (const_tree); +extern tree get_attribute_namespace (const_tree); extern void apply_tm_attr (tree, tree); extern tree make_attribute (const char *, const char *, tree); diff --git gcc/c-family/c-attribs.c gcc/c-family/c-attribs.c index 7a8d3935d79..48819e74e5b 100644 --- gcc/c-family/c-attribs.c +++ gcc/c-family/c-attribs.c @@ -49,7 +49,6 @@ along with GCC; see the file COPYING3. If not see static tree handle_packed_attribute (tree *, tree, tree, int, bool *); static tree handle_nocommon_attribute (tree *, tree, tree, int, bool *); static tree handle_common_attribute (tree *, tree, tree, int, bool *); -static tree handle_noreturn_attribute (tree *, tree, tree, int, bool *); static tree handle_hot_attribute (tree *, tree, tree, int, bool *); static tree handle_cold_attribute (tree *, tree, tree, int, bool *); static tree handle_no_sanitize_attribute (tree *, tree, tree, int, bool *); @@ -190,7 +189,7 @@ static const struct attribute_spec::exclusions attr_noinline_exclusions[] = ATTR_EXCL (NULL, false, false, false), }; -static const struct attribute_spec::exclusions attr_noreturn_exclusions[] = +extern const struct attribute_spec::exclusions attr_noreturn_exclusions[] = { ATTR_EXCL ("alloc_align", true, true, true), ATTR_EXCL ("alloc_size", true, true, true), @@ -779,7 +778,7 @@ handle_common_attribute (tree *node, tree name, tree ARG_UNUSED (args), /* Handle a "noreturn" attribute; arguments as in struct attribute_spec.handler. */ -static tree +tree handle_noreturn_attribute (tree *node, tree name, tree ARG_UNUSED (args), int ARG_UNUSED (flags), bool *no_add_attrs) { diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h index 1cf2cae6395..5ac6e5eacf9 100644 --- gcc/c-family/c-common.h +++ gcc/c-family/c-common.h @@ -1344,6 +1344,8 @@ extern int tm_attr_to_mask (tree); extern tree tm_mask_to_attr (int); extern tree find_tm_attribute (tree); extern const struct attribute_spec::exclusions attr_cold_hot_exclusions[]; +extern const struct attribute_spec::exclusions attr_noreturn_exclusions[]; +extern tree handle_noreturn_attribute (tree *, tree, tree, int, bool *); /* In c-format.c. */ extern bool valid_format_string_type_p (tree); diff --git gcc/c-family/c-format.c gcc/c-family/c-format.c index b10599f9982..bd9d5aeda4e 100644 --- gcc/c-family/c-format.c +++ gcc/c-family/c-format.c @@ -1110,7 +1110,7 @@ check_function_format (const_tree fntype, tree attrs, int nargs, /* See if this function has any format attributes. */ for (a = attrs; a; a = TREE_CHAIN (a)) { - if (is_attribute_p ("format", TREE_PURPOSE (a))) + if (is_attribute_p ("format", get_attribute_name (a))) { /* Yup; check it. */ function_format_info info; diff --git gcc/cp/decl.c gcc/cp/decl.c index 0a3ef452536..85f96f73739 100644 --- gcc/cp/decl.c +++ gcc/cp/decl.c @@ -1922,10 +1922,29 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend) if (TREE_CODE (newdecl) == FUNCTION_DECL) { - if (merge_attr && diagnose_mismatched_attributes (olddecl, newdecl)) - inform (olddecl_loc, DECL_INITIAL (olddecl) - ? G_("previous definition of %qD here") - : G_("previous declaration of %qD here"), olddecl); + if (merge_attr) + { + if (diagnose_mismatched_attributes (olddecl, newdecl)) + inform (olddecl_loc, DECL_INITIAL (olddecl) + ? G_("previous definition of %qD here") + : G_("previous declaration of %qD here"), olddecl); + + /* [dcl.attr.noreturn]: The first declaration of a function shall + specify the noreturn attribute if any declaration of that function + specifies the noreturn attribute. */ + tree a; + if (TREE_THIS_VOLATILE (newdecl) + && !TREE_THIS_VOLATILE (olddecl) + /* This applies to [[noreturn]] only, not its GNU variants. */ + && (a = lookup_attribute ("noreturn", DECL_ATTRIBUTES (newdecl))) + && cxx11_attribute_p (a) + && get_attribute_namespace (a) == NULL_TREE) + { + error_at (newdecl_loc, "function %qD declared %<[[noreturn]]%> " + "but its first declaration was not", newdecl); + inform (olddecl_loc, "previous declaration of %qD", olddecl); + } + } /* Now that functions must hold information normally held by field decls, there is extra work to do so that diff --git gcc/cp/parser.c gcc/cp/parser.c index 8f5ae84670a..996e54235fd 100644 --- gcc/cp/parser.c +++ gcc/cp/parser.c @@ -26121,11 +26121,11 @@ cp_parser_std_attribute (cp_parser *parser, tree attr_ns) attr_id = canonicalize_attr_name (attr_id); attribute = build_tree_list (build_tree_list (NULL_TREE, attr_id), NULL_TREE); - /* C++11 noreturn attribute is equivalent to GNU's. */ - if (is_attribute_p ("noreturn", attr_id)) - TREE_PURPOSE (TREE_PURPOSE (attribute)) = gnu_identifier; + /* We used to treat C++11 noreturn attribute as equivalent to GNU's, + but no longer: we have to be able to tell [[noreturn]] and + __attribute__((noreturn)) apart. */ /* C++14 deprecated attribute is equivalent to GNU's. */ - else if (is_attribute_p ("deprecated", attr_id)) + if (is_attribute_p ("deprecated", attr_id)) TREE_PURPOSE (TREE_PURPOSE (attribute)) = gnu_identifier; /* C++17 fallthrough attribute is equivalent to GNU's. */ else if (is_attribute_p ("fallthrough", attr_id)) diff --git gcc/cp/tree.c gcc/cp/tree.c index cd021b7f594..978aea56193 100644 --- gcc/cp/tree.c +++ gcc/cp/tree.c @@ -4453,6 +4453,8 @@ const struct attribute_spec std_attribute_table[] = handle_likeliness_attribute, attr_cold_hot_exclusions }, { "unlikely", 0, 0, false, false, false, false, handle_likeliness_attribute, attr_cold_hot_exclusions }, + { "noreturn", 0, 0, true, false, false, false, + handle_noreturn_attribute, attr_noreturn_exclusions }, { NULL, 0, 0, false, false, false, false, NULL, NULL } }; diff --git gcc/testsuite/g++.dg/warn/noreturn-10.C gcc/testsuite/g++.dg/warn/noreturn-10.C new file mode 100644 index 00000000000..6f7df48bf18 --- /dev/null +++ gcc/testsuite/g++.dg/warn/noreturn-10.C @@ -0,0 +1,10 @@ +// PR c++/60364 +// { dg-do compile { target c++11 } } + +void* fn1 [[gnu::returns_twice, noreturn]] (); // { dg-warning "ignoring attribute 'noreturn' because it conflicts with attribute 'returns_twice'" } +void* fn2 [[gnu::alloc_align(1), noreturn]] (int); // { dg-warning "ignoring attribute 'noreturn' because it conflicts with attribute 'alloc_align'" } +void* fn3 [[gnu::alloc_size(1), noreturn]] (int); // { dg-warning "ignoring attribute 'noreturn' because it conflicts with attribute 'alloc_size'" } +void* fn4 [[gnu::const, noreturn]] (); // { dg-warning "ignoring attribute 'noreturn' because it conflicts with attribute 'const'" } +void* fn5 [[gnu::malloc, noreturn]] (int); // { dg-warning "ignoring attribute 'noreturn' because it conflicts with attribute 'malloc'" } +void* fn6 [[gnu::pure, noreturn]] (); // { dg-warning "ignoring attribute 'noreturn' because it conflicts with attribute 'pure'" } +void* fn7 [[gnu::warn_unused_result, noreturn]] (); // { dg-warning "ignoring attribute 'noreturn' because it conflicts with attribute 'warn_unused_result'" } diff --git gcc/testsuite/g++.dg/warn/noreturn-11.C gcc/testsuite/g++.dg/warn/noreturn-11.C new file mode 100644 index 00000000000..e0265eba701 --- /dev/null +++ gcc/testsuite/g++.dg/warn/noreturn-11.C @@ -0,0 +1,25 @@ +// PR c++/60364 +// { dg-do compile { target c++11 } } + +void f1 (); +void f1 [[gnu::noreturn]] (); +void f1 [[noreturn]] (); + +void f2 (); +__attribute__((noreturn)) void f2 (); +void f2 [[noreturn]] (); + +void f3 (); +void f3 [[gnu::noreturn]] (); +void f3 (); +void f3 [[noreturn]] (); + +void f4 (); +void f4 (); +void f4 (); +void f4 [[noreturn]] (); // { dg-error "declared '\\\[\\\[noreturn\\\]\\\]' but its first declaration was not" } + +void f5 [[noreturn]] (); +void f5 (); +void f5 (); +void f5 [[noreturn]] (); diff --git gcc/testsuite/g++.dg/warn/noreturn-8.C gcc/testsuite/g++.dg/warn/noreturn-8.C new file mode 100644 index 00000000000..d465468decb --- /dev/null +++ gcc/testsuite/g++.dg/warn/noreturn-8.C @@ -0,0 +1,21 @@ +// PR c++/60364 +// { dg-do compile { target c++11 } } +// { dg-options "-Wpedantic" } + +void f (); // { dg-message "previous declaration" } +void f [[noreturn]] (); // { dg-error "declared '\\\[\\\[noreturn\\\]\\\]' but its first declaration was not" } + +void f2 (); +void f2 [[gnu::noreturn]] (); + +void f3 (); +__attribute__((noreturn)) void f3 (); + +void f4 () { __builtin_abort (); } // { dg-message "previous declaration" } +void f4 [[noreturn]] (); // { dg-error "declared '\\\[\\\[noreturn\\\]\\\]' but its first declaration was not" } + +void f5 () { __builtin_abort (); } +void f5 [[gnu::noreturn]] (); + +void f6 () { __builtin_abort (); } +__attribute__((noreturn)) void f6 (); diff --git gcc/testsuite/g++.dg/warn/noreturn-9.C gcc/testsuite/g++.dg/warn/noreturn-9.C new file mode 100644 index 00000000000..f7ede57aab0 --- /dev/null +++ gcc/testsuite/g++.dg/warn/noreturn-9.C @@ -0,0 +1,21 @@ +// PR c++/60364 +// { dg-do compile { target c++11 } } +// { dg-options "-Wpedantic" } + +void f1 [[gnu::noreturn]] (); +void f1 [[noreturn]] (); + +void f2 [[noreturn]] (); +void f2 [[gnu::noreturn]] (); + +__attribute__((noreturn)) void f3 (); +void f3 [[noreturn]] (); + +void f4 [[noreturn]] (); +__attribute__((noreturn)) void f4 (); + +__attribute__((noreturn)) void f5 (); +void f5 [[gnu::noreturn]] (); + +void f6 [[gnu::noreturn]] (); +__attribute__((noreturn)) void f6 (); diff --git gcc/tree-inline.c gcc/tree-inline.c index 52f45a73b1d..2de5e22f10f 100644 --- gcc/tree-inline.c +++ gcc/tree-inline.c @@ -3899,7 +3899,7 @@ function_attribute_inlinable_p (const_tree fndecl) for (a = DECL_ATTRIBUTES (fndecl); a; a = TREE_CHAIN (a)) { - const_tree name = TREE_PURPOSE (a); + const_tree name = get_attribute_name (a); int i; for (i = 0; targetm.attribute_table[i].name != NULL; i++)