* C++ PATCH for c++/60364 - noreturn after first decl not diagnosed @ 2019-06-15 14:29 Marek Polacek 2019-06-15 14:33 ` Jakub Jelinek 0 siblings, 1 reply; 17+ messages in thread From: Marek Polacek @ 2019-06-15 14:29 UTC (permalink / raw) To: GCC Patches, Jason Merrill [dcl.attr.noreturn] says "The first declaration of a function shall specify the noreturn attribute if any declaration of that function specifies the noreturn attribute" meaning that we should diagnose void func (); void func [[noreturn]] (); but we do not. I'd been meaning to issue a hard error for [[noreturn]] and only a warning for __attribute__((noreturn)) but then I found out that we treat [[noreturn]] exactly as the GNU attribute, and so cxx11_attribute_p returns false for it, so I decided to make it a pedwarn for all the cases. A pedwarn counts as a diagnostic, so we'd be conforming still. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-06-15 Marek Polacek <polacek@redhat.com> PR c++/60364 - noreturn after first decl not diagnosed. * decl.c (duplicate_decls): Warn when the first declaration doesn't have noreturn but a subsequent has. * g++.dg/warn/noreturn-8.C: New test. diff --git gcc/cp/decl.c gcc/cp/decl.c index 0a3ef452536..e50873d111c 100644 --- gcc/cp/decl.c +++ gcc/cp/decl.c @@ -1922,10 +1922,23 @@ 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. */ + if (TREE_THIS_VOLATILE (newdecl) + && !TREE_THIS_VOLATILE (olddecl) + && pedwarn (newdecl_loc, OPT_Wpedantic, "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/testsuite/g++.dg/warn/noreturn-8.C gcc/testsuite/g++.dg/warn/noreturn-8.C new file mode 100644 index 00000000000..d876cf9d156 --- /dev/null +++ gcc/testsuite/g++.dg/warn/noreturn-8.C @@ -0,0 +1,21 @@ +// PR c++/60364 - noreturn after first decl not diagnosed. +// { dg-do compile { target c++11 } } +// { dg-options "-Wpedantic" } + +void f (); // { dg-message "previous declaration" } +void f [[noreturn]] (); // { dg-warning "declared 'noreturn' but its first declaration was not" } + +void f2 (); // { dg-message "previous declaration" } +void f2 [[gnu::noreturn]] (); // { dg-warning "declared 'noreturn' but its first declaration was not" } + +void f3 (); // { dg-message "previous declaration" } +__attribute__((noreturn)) void f3 (); // { dg-warning "declared 'noreturn' but its first declaration was not" } + +void f4 () { __builtin_abort (); } // { dg-message "previous declaration" } +void f4 [[noreturn]] (); // { dg-warning "declared 'noreturn' but its first declaration was not" } + +void f5 () { __builtin_abort (); } // { dg-message "previous declaration" } +void f5 [[gnu::noreturn]] (); // { dg-warning "declared 'noreturn' but its first declaration was not" } + +void f6 () { __builtin_abort (); } // { dg-message "previous declaration" } +__attribute__((noreturn)) void f6 (); // { dg-warning "declared 'noreturn' but its first declaration was not" } ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed 2019-06-15 14:29 C++ PATCH for c++/60364 - noreturn after first decl not diagnosed Marek Polacek @ 2019-06-15 14:33 ` Jakub Jelinek 2019-06-15 14:39 ` Marek Polacek 0 siblings, 1 reply; 17+ messages in thread From: Jakub Jelinek @ 2019-06-15 14:33 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches, Jason Merrill On Sat, Jun 15, 2019 at 10:29:17AM -0400, Marek Polacek wrote: > [dcl.attr.noreturn] says "The first declaration of a function shall specify the > noreturn attribute if any declaration of that function specifies the noreturn > attribute" meaning that we should diagnose > > void func (); > void func [[noreturn]] (); > > but we do not. I'd been meaning to issue a hard error for [[noreturn]] and > only a warning for __attribute__((noreturn)) but then I found out that we > treat [[noreturn]] exactly as the GNU attribute, and so cxx11_attribute_p > returns false for it, so I decided to make it a pedwarn for all the cases. > A pedwarn counts as a diagnostic, so we'd be conforming still. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? IMHO we should treat __attribute__((noreturn)) as before without any warnings, just [[noreturn]] that way. There is nothing wrong on declaring it just on second or following declaration, it is an optimization attribute. Jakub ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed 2019-06-15 14:33 ` Jakub Jelinek @ 2019-06-15 14:39 ` Marek Polacek 2019-06-16 16:10 ` C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) Marek Polacek 0 siblings, 1 reply; 17+ messages in thread From: Marek Polacek @ 2019-06-15 14:39 UTC (permalink / raw) To: Jakub Jelinek; +Cc: GCC Patches, Jason Merrill On Sat, Jun 15, 2019 at 04:33:26PM +0200, Jakub Jelinek wrote: > On Sat, Jun 15, 2019 at 10:29:17AM -0400, Marek Polacek wrote: > > [dcl.attr.noreturn] says "The first declaration of a function shall specify the > > noreturn attribute if any declaration of that function specifies the noreturn > > attribute" meaning that we should diagnose > > > > void func (); > > void func [[noreturn]] (); > > > > but we do not. I'd been meaning to issue a hard error for [[noreturn]] and > > only a warning for __attribute__((noreturn)) but then I found out that we > > treat [[noreturn]] exactly as the GNU attribute, and so cxx11_attribute_p > > returns false for it, so I decided to make it a pedwarn for all the cases. > > A pedwarn counts as a diagnostic, so we'd be conforming still. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > IMHO we should treat __attribute__((noreturn)) as before without any > warnings, just [[noreturn]] that way. There is nothing wrong on declaring > it just on second or following declaration, it is an optimization attribute. That's a complication then; currently [[noreturn]], [[gnu::noreturn]], and __attribute__((noreturn)) are not distinguishable. :( Guess I will really have to make the changes to treat [[noreturn]] similarly to e.g. [[nodiscard]], so that cxx11_attribute_p works. Thanks for pointing it out, though. Marek ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) 2019-06-15 14:39 ` Marek Polacek @ 2019-06-16 16:10 ` Marek Polacek 2019-06-16 16:19 ` Jakub Jelinek 2019-06-17 15:02 ` C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) Martin Sebor 0 siblings, 2 replies; 17+ messages in thread From: Marek Polacek @ 2019-06-16 16:10 UTC (permalink / raw) To: Jakub Jelinek; +Cc: GCC Patches, Jason Merrill On Sat, Jun 15, 2019 at 10:39:13AM -0400, Marek Polacek wrote: > On Sat, Jun 15, 2019 at 04:33:26PM +0200, Jakub Jelinek wrote: > > On Sat, Jun 15, 2019 at 10:29:17AM -0400, Marek Polacek wrote: > > > [dcl.attr.noreturn] says "The first declaration of a function shall specify the > > > noreturn attribute if any declaration of that function specifies the noreturn > > > attribute" meaning that we should diagnose > > > > > > void func (); > > > void func [[noreturn]] (); > > > > > > but we do not. I'd been meaning to issue a hard error for [[noreturn]] and > > > only a warning for __attribute__((noreturn)) but then I found out that we > > > treat [[noreturn]] exactly as the GNU attribute, and so cxx11_attribute_p > > > returns false for it, so I decided to make it a pedwarn for all the cases. > > > A pedwarn counts as a diagnostic, so we'd be conforming still. > > > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > IMHO we should treat __attribute__((noreturn)) as before without any > > warnings, just [[noreturn]] that way. There is nothing wrong on declaring > > it just on second or following declaration, it is an optimization attribute. > > That's a complication then; currently [[noreturn]], [[gnu::noreturn]], and > __attribute__((noreturn)) are not distinguishable. :( > > Guess I will really have to make the changes to treat [[noreturn]] similarly > to e.g. [[nodiscard]], so that cxx11_attribute_p works. Thus. Changes I've made: * don't treat [[noreturn]] as an equivalent to __attribute__((noreturn)); * for that I had to adjust decl_attributes, it wasn't preserving the C++11 form (a list in another list); fix shadowing while at it; * the above turned up two spots that were wrongly accessing TREE_PURPOSE directly instead of using get_attribute_name; * give error only for [[noreturn]] but not for __attribute__((noreturn)) or [[gnu::noreturn]]. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-06-16 Marek Polacek <polacek@redhat.com> 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. * 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): 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. 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..1cd95dc6665 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 *); @@ -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..079225ca127 100644 --- gcc/c-family/c-common.h +++ gcc/c-family/c-common.h @@ -1344,6 +1344,7 @@ 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 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..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 }, { NULL, 0, 0, false, false, false, false, NULL, NULL } }; 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/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++) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) 2019-06-16 16:10 ` C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) Marek Polacek @ 2019-06-16 16:19 ` Jakub Jelinek 2019-06-16 16:37 ` C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v3) Marek Polacek 2019-06-17 15:02 ` C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) Martin Sebor 1 sibling, 1 reply; 17+ messages in thread From: Jakub Jelinek @ 2019-06-16 16:19 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches, Jason Merrill On Sun, Jun 16, 2019 at 12:10:37PM -0400, Marek Polacek wrote: > > Guess I will really have to make the changes to treat [[noreturn]] similarly > > to e.g. [[nodiscard]], so that cxx11_attribute_p works. > > Thus. Changes I've made: > * don't treat [[noreturn]] as an equivalent to __attribute__((noreturn)); > * for that I had to adjust decl_attributes, it wasn't preserving the > C++11 form (a list in another list); fix shadowing while at it; > * the above turned up two spots that were wrongly accessing TREE_PURPOSE > directly instead of using get_attribute_name; > * give error only for [[noreturn]] but not for __attribute__((noreturn)) > or [[gnu::noreturn]]. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? I'd prefer to defer review of this to Jason, just want to note that I don't see any testsuite coverage on mixing declarations with different forms of attributes ([[noreturn]] on one decl and __attribute__((noreturn)) or [[gnu::noreturn]] on another one or vice versa. Jakub ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v3) 2019-06-16 16:19 ` Jakub Jelinek @ 2019-06-16 16:37 ` Marek Polacek 2019-06-17 15:48 ` Jakub Jelinek 0 siblings, 1 reply; 17+ messages in thread From: Marek Polacek @ 2019-06-16 16:37 UTC (permalink / raw) To: Jakub Jelinek; +Cc: GCC Patches, Jason Merrill On Sun, Jun 16, 2019 at 06:18:56PM +0200, Jakub Jelinek wrote: > On Sun, Jun 16, 2019 at 12:10:37PM -0400, Marek Polacek wrote: > > > Guess I will really have to make the changes to treat [[noreturn]] similarly > > > to e.g. [[nodiscard]], so that cxx11_attribute_p works. > > > > Thus. Changes I've made: > > * don't treat [[noreturn]] as an equivalent to __attribute__((noreturn)); > > * for that I had to adjust decl_attributes, it wasn't preserving the > > C++11 form (a list in another list); fix shadowing while at it; > > * the above turned up two spots that were wrongly accessing TREE_PURPOSE > > directly instead of using get_attribute_name; > > * give error only for [[noreturn]] but not for __attribute__((noreturn)) > > or [[gnu::noreturn]]. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > I'd prefer to defer review of this to Jason, just want to note that I don't > see any testsuite coverage on mixing declarations with different forms of > attributes ([[noreturn]] on one decl and __attribute__((noreturn)) or > [[gnu::noreturn]] on another one or vice versa. Added now. I suppose it should compile fine, which it does. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-06-16 Marek Polacek <polacek@redhat.com> 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. * 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): 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. 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..1cd95dc6665 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 *); @@ -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..079225ca127 100644 --- gcc/c-family/c-common.h +++ gcc/c-family/c-common.h @@ -1344,6 +1344,7 @@ 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 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..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 }, { NULL, 0, 0, false, false, false, false, NULL, NULL } }; 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++) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v3) 2019-06-16 16:37 ` C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v3) Marek Polacek @ 2019-06-17 15:48 ` Jakub Jelinek 2019-06-17 18:15 ` Marek Polacek 0 siblings, 1 reply; 17+ messages in thread From: Jakub Jelinek @ 2019-06-17 15:48 UTC (permalink / raw) To: Marek Polacek; +Cc: GCC Patches, Jason Merrill On Sun, Jun 16, 2019 at 12:36:58PM -0400, Marek Polacek wrote: > On Sun, Jun 16, 2019 at 06:18:56PM +0200, Jakub Jelinek wrote: > > On Sun, Jun 16, 2019 at 12:10:37PM -0400, Marek Polacek wrote: > > > > Guess I will really have to make the changes to treat [[noreturn]] similarly > > > > to e.g. [[nodiscard]], so that cxx11_attribute_p works. > > > > > > Thus. Changes I've made: > > > * don't treat [[noreturn]] as an equivalent to __attribute__((noreturn)); > > > * for that I had to adjust decl_attributes, it wasn't preserving the > > > C++11 form (a list in another list); fix shadowing while at it; > > > * the above turned up two spots that were wrongly accessing TREE_PURPOSE > > > directly instead of using get_attribute_name; > > > * give error only for [[noreturn]] but not for __attribute__((noreturn)) > > > or [[gnu::noreturn]]. > > > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > I'd prefer to defer review of this to Jason, just want to note that I don't > > see any testsuite coverage on mixing declarations with different forms of > > attributes ([[noreturn]] on one decl and __attribute__((noreturn)) or > > [[gnu::noreturn]] on another one or vice versa. > > Added now. I suppose it should compile fine, which it does. I meant also the tests of the new diagnostics, say if you have a decl without any of those attributes, then gnu:: one (or __attribute__ one; that merge decls should be ok) and on third decl [[noreturn]] (shall that diagnose anything or not? As there is no way to differentiate it from the gnu:: attribute on the very first one, I'd say it shouldn't, with the use of the gnu:: or __attribute__ we are already outside of the standard. Jakub ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v3) 2019-06-17 15:48 ` Jakub Jelinek @ 2019-06-17 18:15 ` Marek Polacek 0 siblings, 0 replies; 17+ messages in thread From: Marek Polacek @ 2019-06-17 18:15 UTC (permalink / raw) To: Jakub Jelinek; +Cc: GCC Patches, Jason Merrill On Mon, Jun 17, 2019 at 05:48:23PM +0200, Jakub Jelinek wrote: > On Sun, Jun 16, 2019 at 12:36:58PM -0400, Marek Polacek wrote: > > On Sun, Jun 16, 2019 at 06:18:56PM +0200, Jakub Jelinek wrote: > > > On Sun, Jun 16, 2019 at 12:10:37PM -0400, Marek Polacek wrote: > > > > > Guess I will really have to make the changes to treat [[noreturn]] similarly > > > > > to e.g. [[nodiscard]], so that cxx11_attribute_p works. > > > > > > > > Thus. Changes I've made: > > > > * don't treat [[noreturn]] as an equivalent to __attribute__((noreturn)); > > > > * for that I had to adjust decl_attributes, it wasn't preserving the > > > > C++11 form (a list in another list); fix shadowing while at it; > > > > * the above turned up two spots that were wrongly accessing TREE_PURPOSE > > > > directly instead of using get_attribute_name; > > > > * give error only for [[noreturn]] but not for __attribute__((noreturn)) > > > > or [[gnu::noreturn]]. > > > > > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > > > I'd prefer to defer review of this to Jason, just want to note that I don't > > > see any testsuite coverage on mixing declarations with different forms of > > > attributes ([[noreturn]] on one decl and __attribute__((noreturn)) or > > > [[gnu::noreturn]] on another one or vice versa. > > > > Added now. I suppose it should compile fine, which it does. > > I meant also the tests of the new diagnostics, say if you have > a decl without any of those attributes, then gnu:: one (or __attribute__ > one; that merge decls should be ok) and on third decl [[noreturn]] (shall > that diagnose anything or not? As there is no way to differentiate it from > the gnu:: attribute on the very first one, I'd say it shouldn't, with the > use of the gnu:: or __attribute__ we are already outside of the standard. I've added noreturn-11.C for that. With my patch we don't diagnose void f1 (); void f1 [[gnu::noreturn]] (); void f1 [[noreturn]] (); but that seems fine to me, too. Of course there's the problem that we only check the previous decl, not the first one, but I guess we'll have to live with it; it will detect the bogus cases anyway. -- Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) 2019-06-16 16:10 ` C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) Marek Polacek 2019-06-16 16:19 ` Jakub Jelinek @ 2019-06-17 15:02 ` Martin Sebor 2019-06-17 18:11 ` Marek Polacek 1 sibling, 1 reply; 17+ messages in thread From: Martin Sebor @ 2019-06-17 15:02 UTC (permalink / raw) To: Marek Polacek, Jakub Jelinek; +Cc: GCC Patches, Jason Merrill On 6/16/19 10:10 AM, Marek Polacek wrote: > On Sat, Jun 15, 2019 at 10:39:13AM -0400, Marek Polacek wrote: >> On Sat, Jun 15, 2019 at 04:33:26PM +0200, Jakub Jelinek wrote: >>> On Sat, Jun 15, 2019 at 10:29:17AM -0400, Marek Polacek wrote: >>>> [dcl.attr.noreturn] says "The first declaration of a function shall specify the >>>> noreturn attribute if any declaration of that function specifies the noreturn >>>> attribute" meaning that we should diagnose >>>> >>>> void func (); >>>> void func [[noreturn]] (); >>>> >>>> but we do not. I'd been meaning to issue a hard error for [[noreturn]] and >>>> only a warning for __attribute__((noreturn)) but then I found out that we >>>> treat [[noreturn]] exactly as the GNU attribute, and so cxx11_attribute_p >>>> returns false for it, so I decided to make it a pedwarn for all the cases. >>>> A pedwarn counts as a diagnostic, so we'd be conforming still. >>>> >>>> Bootstrapped/regtested on x86_64-linux, ok for trunk? >>> >>> IMHO we should treat __attribute__((noreturn)) as before without any >>> warnings, just [[noreturn]] that way. There is nothing wrong on declaring >>> it just on second or following declaration, it is an optimization attribute. >> >> That's a complication then; currently [[noreturn]], [[gnu::noreturn]], and >> __attribute__((noreturn)) are not distinguishable. :( >> >> Guess I will really have to make the changes to treat [[noreturn]] similarly >> to e.g. [[nodiscard]], so that cxx11_attribute_p works. > > Thus. Changes I've made: > * don't treat [[noreturn]] as an equivalent to __attribute__((noreturn)); > * for that I had to adjust decl_attributes, it wasn't preserving the > C++11 form (a list in another list); fix shadowing while at it; > * the above turned up two spots that were wrongly accessing TREE_PURPOSE > directly instead of using get_attribute_name; > * give error only for [[noreturn]] but not for __attribute__((noreturn)) > or [[gnu::noreturn]]. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2019-06-16 Marek Polacek <polacek@redhat.com> > > 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. > * 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): 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. ... > 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? Martin ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) 2019-06-17 15:02 ` C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) Martin Sebor @ 2019-06-17 18:11 ` Marek Polacek 2019-06-19 19:31 ` Jeff Law 2019-06-20 16:44 ` Joseph Myers 0 siblings, 2 replies; 17+ messages in thread From: Marek Polacek @ 2019-06-17 18:11 UTC (permalink / raw) To: Martin Sebor; +Cc: Jakub Jelinek, GCC Patches, Jason Merrill 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 <polacek@redhat.com> 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++) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) 2019-06-17 18:11 ` Marek Polacek @ 2019-06-19 19:31 ` Jeff Law 2019-06-20 16:44 ` Joseph Myers 1 sibling, 0 replies; 17+ messages in thread From: Jeff Law @ 2019-06-19 19:31 UTC (permalink / raw) To: Marek Polacek, Martin Sebor; +Cc: Jakub Jelinek, GCC Patches, Jason Merrill On 6/17/19 12:10 PM, Marek Polacek wrote: > 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 <polacek@redhat.com> > > 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. > Turns out there was more generic stuff than C++ specific stuff here. So I went ahead and walked through it. OK for the trunk. jeff ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) 2019-06-17 18:11 ` Marek Polacek 2019-06-19 19:31 ` Jeff Law @ 2019-06-20 16:44 ` Joseph Myers 2019-06-20 16:49 ` Marek Polacek 1 sibling, 1 reply; 17+ messages in thread From: Joseph Myers @ 2019-06-20 16:44 UTC (permalink / raw) To: Marek Polacek; +Cc: Martin Sebor, Jakub Jelinek, GCC Patches, Jason Merrill This (commit r272486) introduces an ICE building libstdc++-v3 for sh4-linux-gnu. libtool: compile: /scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/./gcc/xgcc -shared-libgcc -B/scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/./gcc -nostdinc++ -L/scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/sh4-glibc-linux-gnu/libstdc++-v3/src -L/scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/sh4-glibc-linux-gnu/libstdc++-v3/src/.libs -L/scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/sh4-glibc-linux-gnu/libstdc++-v3/libsupc++/.libs -B/scratch/jmyers/glibc-bot/install/compilers/sh4-linux-gnu/sh4-glibc-linux-gnu/bin/ -B/scratch/jmyers/glibc-bot/install/compilers/sh4-linux-gnu/sh4-glibc-linux-gnu/lib/ -isystem /scratch/jmyers/glibc-bot/install/compilers/sh4-linux-gnu/sh4-glibc-linux-gnu/include -isystem /scratch/jmyers/glibc-bot/install/compilers/sh4-linux-gnu/sh4-glibc-linux-gnu/sys-include -I/scratch/jmyers/glibc-bot/src/gcc/libstdc++-v3/../libgcc -I/scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/sh4-glibc-linux-gnu/libstdc++-v3/include/sh4-glibc-linux-gnu -I/scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/sh4-glibc-linux-gnu/libstdc++-v3/include -I/scratch/jmyers/glibc-bot/src/gcc/libstdc++-v3/libsupc++ -D_GLIBCXX_SHARED -fno-implicit-templates -Wall -Wextra -Wwrite-strings -Wcast-qual -Wabi=2 -fdiagnostics-show-location=once -ffunction-sections -fdata-sections -frandom-seed=new_opa.lo -g -O2 -D_GNU_SOURCE -std=gnu++1z -c /scratch/jmyers/glibc-bot/src/gcc/libstdc++-v3/libsupc++/new_opa.cc -fPIC -DPIC -D_GLIBCXX_SHARED -o new_opa.o during RTL pass: final /scratch/jmyers/glibc-bot/src/gcc/libstdc++-v3/libsupc++/new_opa.cc: In function 'void* operator new(std::size_t, std::align_val_t)': /scratch/jmyers/glibc-bot/src/gcc/libstdc++-v3/libsupc++/new_opa.cc:132:1: internal compiler error: tree check: expected identifier_node, have tree_list in is_attribute_p, at attribs.h:155 132 | } | ^ 0x5b320d tree_check_failed(tree_node const*, char const*, int, char const*, ...) /scratch/jmyers/glibc-bot/src/gcc/gcc/tree.c:9899 0x5b3f0d tree_check(tree_node const*, char const*, int, char const*, tree_code) /scratch/jmyers/glibc-bot/src/gcc/gcc/tree.h:3453 0x5b3f0d is_attribute_p /scratch/jmyers/glibc-bot/src/gcc/gcc/attribs.h:155 0x11884c3 is_attribute_p /scratch/jmyers/glibc-bot/src/gcc/gcc/tree.h:3197 0x11884c3 sh2a_function_vector_p /scratch/jmyers/glibc-bot/src/gcc/gcc/config/sh/sh.c:8649 0x1188527 sh_encode_section_info /scratch/jmyers/glibc-bot/src/gcc/gcc/config/sh/sh.c:1570 0x1153ba8 make_decl_rtl(tree_node*) /scratch/jmyers/glibc-bot/src/gcc/gcc/varasm.c:1524 0x115460c get_fnname_from_decl(tree_node*) /scratch/jmyers/glibc-bot/src/gcc/gcc/varasm.c:1720 0xab2aa9 rest_of_handle_final /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4648 0xab2aa9 execute /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4737 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions. Makefile:960: recipe for target 'new_opa.lo' failed make[5]: *** [new_opa.lo] Error 1 -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) 2019-06-20 16:44 ` Joseph Myers @ 2019-06-20 16:49 ` Marek Polacek 2019-06-20 17:08 ` Joseph Myers 2019-06-20 17:57 ` Jakub Jelinek 0 siblings, 2 replies; 17+ messages in thread From: Marek Polacek @ 2019-06-20 16:49 UTC (permalink / raw) To: Joseph Myers; +Cc: Martin Sebor, Jakub Jelinek, GCC Patches, Jason Merrill On Thu, Jun 20, 2019 at 04:43:58PM +0000, Joseph Myers wrote: > This (commit r272486) introduces an ICE building libstdc++-v3 for > sh4-linux-gnu. > > libtool: compile: > /scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/./gcc/xgcc > -shared-libgcc > -B/scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/./gcc > -nostdinc++ > -L/scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/sh4-glibc-linux-gnu/libstdc++-v3/src > -L/scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/sh4-glibc-linux-gnu/libstdc++-v3/src/.libs > -L/scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/sh4-glibc-linux-gnu/libstdc++-v3/libsupc++/.libs > -B/scratch/jmyers/glibc-bot/install/compilers/sh4-linux-gnu/sh4-glibc-linux-gnu/bin/ > -B/scratch/jmyers/glibc-bot/install/compilers/sh4-linux-gnu/sh4-glibc-linux-gnu/lib/ > -isystem > /scratch/jmyers/glibc-bot/install/compilers/sh4-linux-gnu/sh4-glibc-linux-gnu/include > -isystem > /scratch/jmyers/glibc-bot/install/compilers/sh4-linux-gnu/sh4-glibc-linux-gnu/sys-include > -I/scratch/jmyers/glibc-bot/src/gcc/libstdc++-v3/../libgcc > -I/scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/sh4-glibc-linux-gnu/libstdc++-v3/include/sh4-glibc-linux-gnu > -I/scratch/jmyers/glibc-bot/build/compilers/sh4-linux-gnu/gcc/sh4-glibc-linux-gnu/libstdc++-v3/include > -I/scratch/jmyers/glibc-bot/src/gcc/libstdc++-v3/libsupc++ > -D_GLIBCXX_SHARED -fno-implicit-templates -Wall -Wextra -Wwrite-strings > -Wcast-qual -Wabi=2 -fdiagnostics-show-location=once -ffunction-sections > -fdata-sections -frandom-seed=new_opa.lo -g -O2 -D_GNU_SOURCE -std=gnu++1z > -c /scratch/jmyers/glibc-bot/src/gcc/libstdc++-v3/libsupc++/new_opa.cc > -fPIC -DPIC -D_GLIBCXX_SHARED -o new_opa.o > during RTL pass: final > /scratch/jmyers/glibc-bot/src/gcc/libstdc++-v3/libsupc++/new_opa.cc: In function 'void* operator new(std::size_t, std::align_val_t)': > /scratch/jmyers/glibc-bot/src/gcc/libstdc++-v3/libsupc++/new_opa.cc:132:1: internal compiler error: tree check: expected identifier_node, have tree_list in is_attribute_p, at attribs.h:155 > 132 | } > | ^ > 0x5b320d tree_check_failed(tree_node const*, char const*, int, char const*, ...) > /scratch/jmyers/glibc-bot/src/gcc/gcc/tree.c:9899 > 0x5b3f0d tree_check(tree_node const*, char const*, int, char const*, > tree_code) > /scratch/jmyers/glibc-bot/src/gcc/gcc/tree.h:3453 > 0x5b3f0d is_attribute_p > /scratch/jmyers/glibc-bot/src/gcc/gcc/attribs.h:155 > 0x11884c3 is_attribute_p > /scratch/jmyers/glibc-bot/src/gcc/gcc/tree.h:3197 > 0x11884c3 sh2a_function_vector_p > /scratch/jmyers/glibc-bot/src/gcc/gcc/config/sh/sh.c:8649 > 0x1188527 sh_encode_section_info > /scratch/jmyers/glibc-bot/src/gcc/gcc/config/sh/sh.c:1570 > 0x1153ba8 make_decl_rtl(tree_node*) > /scratch/jmyers/glibc-bot/src/gcc/gcc/varasm.c:1524 > 0x115460c get_fnname_from_decl(tree_node*) > /scratch/jmyers/glibc-bot/src/gcc/gcc/varasm.c:1720 > 0xab2aa9 rest_of_handle_final > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4648 > 0xab2aa9 execute > /scratch/jmyers/glibc-bot/src/gcc/gcc/final.c:4737 > Please submit a full bug report, > with preprocessed source if appropriate. > Please include the complete backtrace with any bug report. > See <https://gcc.gnu.org/bugs/> for instructions. > Makefile:960: recipe for target 'new_opa.lo' failed > make[5]: *** [new_opa.lo] Error 1 Sorry about that. Does this patch work? 2019-06-20 Marek Polacek <polacek@redhat.com> * config/sh/sh.c (sh2a_function_vector_p): Use get_attribute_name. diff --git gcc/config/sh/sh.c gcc/config/sh/sh.c index 07d5b3c1df5..dfaeab55142 100644 --- gcc/config/sh/sh.c +++ gcc/config/sh/sh.c @@ -8646,7 +8646,7 @@ sh2a_function_vector_p (tree func) return false; for (tree list = SH_ATTRIBUTES (func); list; list = TREE_CHAIN (list)) - if (is_attribute_p ("function_vector", TREE_PURPOSE (list))) + if (is_attribute_p ("function_vector", get_attribute_name (list))) return true; return false; ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) 2019-06-20 16:49 ` Marek Polacek @ 2019-06-20 17:08 ` Joseph Myers 2019-06-20 17:14 ` Marek Polacek 2019-06-20 17:57 ` Jakub Jelinek 1 sibling, 1 reply; 17+ messages in thread From: Joseph Myers @ 2019-06-20 17:08 UTC (permalink / raw) To: Marek Polacek; +Cc: Martin Sebor, Jakub Jelinek, GCC Patches, Jason Merrill On Thu, 20 Jun 2019, Marek Polacek wrote: > Sorry about that. Does this patch work? Yes, that fixes it, thanks. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) 2019-06-20 17:08 ` Joseph Myers @ 2019-06-20 17:14 ` Marek Polacek 0 siblings, 0 replies; 17+ messages in thread From: Marek Polacek @ 2019-06-20 17:14 UTC (permalink / raw) To: Joseph Myers; +Cc: Martin Sebor, Jakub Jelinek, GCC Patches, Jason Merrill On Thu, Jun 20, 2019 at 05:08:37PM +0000, Joseph Myers wrote: > On Thu, 20 Jun 2019, Marek Polacek wrote: > > > Sorry about that. Does this patch work? > > Yes, that fixes it, thanks. Great, I've applied the patch. Marek ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) 2019-06-20 16:49 ` Marek Polacek 2019-06-20 17:08 ` Joseph Myers @ 2019-06-20 17:57 ` Jakub Jelinek 2019-06-20 19:12 ` Marek Polacek 1 sibling, 1 reply; 17+ messages in thread From: Jakub Jelinek @ 2019-06-20 17:57 UTC (permalink / raw) To: Marek Polacek; +Cc: Joseph Myers, Martin Sebor, GCC Patches, Jason Merrill On Thu, Jun 20, 2019 at 12:49:13PM -0400, Marek Polacek wrote: > Sorry about that. Does this patch work? > > 2019-06-20 Marek Polacek <polacek@redhat.com> > > * config/sh/sh.c (sh2a_function_vector_p): Use get_attribute_name. Just that? grep is_attribute_p.*TREE_PURPOSE config/*/* config/m32c/m32c.c: if (is_attribute_p ("interrupt", TREE_PURPOSE (list))) config/m32c/m32c.c: if (is_attribute_p ("bank_switch", TREE_PURPOSE (list))) config/m32c/m32c.c: if (is_attribute_p ("fast_interrupt", TREE_PURPOSE (list))) config/m32c/m32c.c: if (is_attribute_p ("function_vector", TREE_PURPOSE (list))) config/m32c/m32c.c: if (is_attribute_p ("function_vector", TREE_PURPOSE (list))) config/rl78/rl78.c: if (is_attribute_p ("saddr", TREE_PURPOSE (list))) config/sh/sh.c: if (is_attribute_p ("sp_switch", TREE_PURPOSE (attrs)) config/sh/sh.c: || is_attribute_p ("trap_exit", TREE_PURPOSE (attrs)) config/sh/sh.c: || is_attribute_p ("nosave_low_regs", TREE_PURPOSE (attrs)) config/sh/sh.c: || is_attribute_p ("resbank", TREE_PURPOSE (attrs))) config/sh/sh.c: if (is_attribute_p ("function_vector", TREE_PURPOSE (list))) config/sh/sh.c: if (is_attribute_p ("function_vector", TREE_PURPOSE (list))) > diff --git gcc/config/sh/sh.c gcc/config/sh/sh.c > index 07d5b3c1df5..dfaeab55142 100644 > --- gcc/config/sh/sh.c > +++ gcc/config/sh/sh.c > @@ -8646,7 +8646,7 @@ sh2a_function_vector_p (tree func) > return false; > > for (tree list = SH_ATTRIBUTES (func); list; list = TREE_CHAIN (list)) > - if (is_attribute_p ("function_vector", TREE_PURPOSE (list))) > + if (is_attribute_p ("function_vector", get_attribute_name (list))) > return true; > > return false; Jakub ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) 2019-06-20 17:57 ` Jakub Jelinek @ 2019-06-20 19:12 ` Marek Polacek 0 siblings, 0 replies; 17+ messages in thread From: Marek Polacek @ 2019-06-20 19:12 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Joseph Myers, Martin Sebor, GCC Patches, Jason Merrill On Thu, Jun 20, 2019 at 07:57:08PM +0200, Jakub Jelinek wrote: > On Thu, Jun 20, 2019 at 12:49:13PM -0400, Marek Polacek wrote: > > Sorry about that. Does this patch work? > > > > 2019-06-20 Marek Polacek <polacek@redhat.com> > > > > * config/sh/sh.c (sh2a_function_vector_p): Use get_attribute_name. > > Just that? > grep is_attribute_p.*TREE_PURPOSE config/*/* > config/m32c/m32c.c: if (is_attribute_p ("interrupt", TREE_PURPOSE (list))) > config/m32c/m32c.c: if (is_attribute_p ("bank_switch", TREE_PURPOSE (list))) > config/m32c/m32c.c: if (is_attribute_p ("fast_interrupt", TREE_PURPOSE (list))) > config/m32c/m32c.c: if (is_attribute_p ("function_vector", TREE_PURPOSE (list))) > config/m32c/m32c.c: if (is_attribute_p ("function_vector", TREE_PURPOSE (list))) > config/rl78/rl78.c: if (is_attribute_p ("saddr", TREE_PURPOSE (list))) > config/sh/sh.c: if (is_attribute_p ("sp_switch", TREE_PURPOSE (attrs)) > config/sh/sh.c: || is_attribute_p ("trap_exit", TREE_PURPOSE (attrs)) > config/sh/sh.c: || is_attribute_p ("nosave_low_regs", TREE_PURPOSE (attrs)) > config/sh/sh.c: || is_attribute_p ("resbank", TREE_PURPOSE (attrs))) > config/sh/sh.c: if (is_attribute_p ("function_vector", TREE_PURPOSE (list))) > config/sh/sh.c: if (is_attribute_p ("function_vector", TREE_PURPOSE (list))) Here's a bunch of more; I've audited the uses of TREE_PURPOSE in config/*/*. I haven't tested it or anything, but... ok for trunk? 2019-06-20 Marek Polacek <polacek@redhat.com> * config/epiphany/epiphany.c (epiphany_compute_function_type): Use get_attribute_name. * config/m32c/m32c.c (interrupt_p): Likewise. (bank_switch_p): Likewise. (fast_interrupt_p): Likewise. (m32c_special_page_vector_p): Likewise. (current_function_special_page_vector): Likewise. * config/nds32/nds32.c (nds32_asm_function_prologue): Likewise. * config/rl78/rl78.c (rl78_attrlist_to_encoding): Likewise. * config/sh/sh.c (sh_insert_attributes): Likewise. (sh2a_get_function_vector_number): Likewise. diff --git gcc/config/epiphany/epiphany.c gcc/config/epiphany/epiphany.c index 657a8886ac7..2cc6c59eae3 100644 --- gcc/config/epiphany/epiphany.c +++ gcc/config/epiphany/epiphany.c @@ -1044,7 +1044,7 @@ epiphany_compute_function_type (tree decl) a; a = TREE_CHAIN (a)) { - tree name = TREE_PURPOSE (a); + tree name = get_attribute_name (a); if (name == get_identifier ("interrupt")) fn_type = EPIPHANY_FUNCTION_INTERRUPT; diff --git gcc/config/m32c/m32c.c gcc/config/m32c/m32c.c index 1a0d0c681b4..bda56e3beee 100644 --- gcc/config/m32c/m32c.c +++ gcc/config/m32c/m32c.c @@ -2858,7 +2858,7 @@ interrupt_p (tree node ATTRIBUTE_UNUSED) tree list = M32C_ATTRIBUTES (node); while (list) { - if (is_attribute_p ("interrupt", TREE_PURPOSE (list))) + if (is_attribute_p ("interrupt", get_attribute_name (list))) return 1; list = TREE_CHAIN (list); } @@ -2872,7 +2872,7 @@ bank_switch_p (tree node ATTRIBUTE_UNUSED) tree list = M32C_ATTRIBUTES (node); while (list) { - if (is_attribute_p ("bank_switch", TREE_PURPOSE (list))) + if (is_attribute_p ("bank_switch", get_attribute_name (list))) return 1; list = TREE_CHAIN (list); } @@ -2886,7 +2886,7 @@ fast_interrupt_p (tree node ATTRIBUTE_UNUSED) tree list = M32C_ATTRIBUTES (node); while (list) { - if (is_attribute_p ("fast_interrupt", TREE_PURPOSE (list))) + if (is_attribute_p ("fast_interrupt", get_attribute_name (list))) return 1; list = TREE_CHAIN (list); } @@ -2915,7 +2915,7 @@ m32c_special_page_vector_p (tree func) list = M32C_ATTRIBUTES (func); while (list) { - if (is_attribute_p ("function_vector", TREE_PURPOSE (list))) + if (is_attribute_p ("function_vector", get_attribute_name (list))) return 1; list = TREE_CHAIN (list); } @@ -2984,7 +2984,7 @@ current_function_special_page_vector (rtx x) list = M32C_ATTRIBUTES (t); while (list) { - if (is_attribute_p ("function_vector", TREE_PURPOSE (list))) + if (is_attribute_p ("function_vector", get_attribute_name (list))) { num = TREE_INT_CST_LOW (TREE_VALUE (TREE_VALUE (list))); return num; diff --git gcc/config/nds32/nds32.c gcc/config/nds32/nds32.c index eba98126705..ea532ce1eb3 100644 --- gcc/config/nds32/nds32.c +++ gcc/config/nds32/nds32.c @@ -2190,7 +2190,7 @@ nds32_asm_function_prologue (FILE *file) /* Display all attributes of this function. */ while (attrs) { - name = TREE_PURPOSE (attrs); + name = get_attribute_name (attrs); fprintf (file, "%s ", IDENTIFIER_POINTER (name)); /* Pick up the next attribute. */ diff --git gcc/config/rl78/rl78.c gcc/config/rl78/rl78.c index d2caa118281..067b966b48d 100644 --- gcc/config/rl78/rl78.c +++ gcc/config/rl78/rl78.c @@ -4532,7 +4532,7 @@ rl78_attrlist_to_encoding (tree list, tree decl ATTRIBUTE_UNUSED) { while (list) { - if (is_attribute_p ("saddr", TREE_PURPOSE (list))) + if (is_attribute_p ("saddr", get_attribute_name (list))) return 's'; list = TREE_CHAIN (list); } diff --git gcc/config/sh/sh.c gcc/config/sh/sh.c index dfaeab55142..5354d7960b0 100644 --- gcc/config/sh/sh.c +++ gcc/config/sh/sh.c @@ -8360,16 +8360,17 @@ sh_insert_attributes (tree node, tree *attributes) for (tail = attributes; attrs; attrs = TREE_CHAIN (attrs)) { - if (is_attribute_p ("sp_switch", TREE_PURPOSE (attrs)) - || is_attribute_p ("trap_exit", TREE_PURPOSE (attrs)) - || is_attribute_p ("nosave_low_regs", TREE_PURPOSE (attrs)) - || is_attribute_p ("resbank", TREE_PURPOSE (attrs))) + if (is_attribute_p ("sp_switch", get_attribute_name (attrs)) + || is_attribute_p ("trap_exit", get_attribute_name (attrs)) + || is_attribute_p ("nosave_low_regs", + get_attribute_name (attrs)) + || is_attribute_p ("resbank", get_attribute_name (attrs))) warning (OPT_Wattributes, "%qE attribute only applies to interrupt functions", - TREE_PURPOSE (attrs)); + get_attribute_name (attrs)); else { - *tail = tree_cons (TREE_PURPOSE (attrs), NULL_TREE, + *tail = tree_cons (get_attribute_name (attrs), NULL_TREE, NULL_TREE); tail = &TREE_CHAIN (*tail); } @@ -8537,7 +8538,7 @@ sh2a_get_function_vector_number (rtx x) return 0; for (tree list = SH_ATTRIBUTES (t); list; list = TREE_CHAIN (list)) - if (is_attribute_p ("function_vector", TREE_PURPOSE (list))) + if (is_attribute_p ("function_vector", get_attribute_name (list))) return TREE_INT_CST_LOW (TREE_VALUE (TREE_VALUE (list))); return 0; ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2019-06-20 19:12 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-15 14:29 C++ PATCH for c++/60364 - noreturn after first decl not diagnosed Marek Polacek 2019-06-15 14:33 ` Jakub Jelinek 2019-06-15 14:39 ` Marek Polacek 2019-06-16 16:10 ` C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) Marek Polacek 2019-06-16 16:19 ` Jakub Jelinek 2019-06-16 16:37 ` C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v3) Marek Polacek 2019-06-17 15:48 ` Jakub Jelinek 2019-06-17 18:15 ` Marek Polacek 2019-06-17 15:02 ` C++ PATCH for c++/60364 - noreturn after first decl not diagnosed (v2) Martin Sebor 2019-06-17 18:11 ` Marek Polacek 2019-06-19 19:31 ` Jeff Law 2019-06-20 16:44 ` Joseph Myers 2019-06-20 16:49 ` Marek Polacek 2019-06-20 17:08 ` Joseph Myers 2019-06-20 17:14 ` Marek Polacek 2019-06-20 17:57 ` Jakub Jelinek 2019-06-20 19:12 ` Marek Polacek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).