From d60c25fa02044181196768fa8b63980e0f535c38 Mon Sep 17 00:00:00 2001 From: Iain Sandoe Date: Mon, 3 Feb 2020 19:15:31 +0000 Subject: [PATCH] coroutines: Fix ICE on invalid (PR93458). Since coroutine-ness is discovered lazily, we encounter the diagnostics during each keyword parse. We were not handling the case where a user code failed to include fundamental information (e.g. the traits) in a graceful manner. Once we've emitted an error for this level of fail, then we suppress additional copies (otherwise the same thing will be reported for every coroutine keyword seen). gcc/cp/ChangeLog: 2020-02-03 Iain Sandoe * coroutines.cc (struct coroutine_info): Add a bool flag to note that we emitted an error for a bad function return type. (get_coroutine_info): Tolerate an unset info table in case of missing traits. (find_coro_traits_template_decl): In case of error or if we didn't find a type template, note we emitted the error and suppress duplicates. (find_coro_handle_template_decl): Likewise. (instantiate_coro_traits): Only check for error_mark_node in the return from lookup_qualified_name. (coro_promise_type_found_p): Reorder initialization so that we check for the traits and their usability before allocation of the info table. Check for a suitable return type and emit a diagnostic for here instead of relying on the lookup machinery. This allows the error to have a better location, and means we can suppress multiple copies. (coro_function_valid_p): Re-check for a valid promise (and thus the traits) before proceeding. Tolerate missing info as a fatal error. gcc/testsuite/ChangeLog: 2020-02-03 Iain Sandoe * g++.dg/coroutines/pr93458-1-missing-traits.C: New test. * g++.dg/coroutines/pr93458-2-bad-traits.C: New test. * g++.dg/coroutines/pr93458-3-missing-handle.C: New test. * g++.dg/coroutines/pr93458-4-bad-coro-handle.C: New test. * g++.dg/coroutines/pr93458-5-bad-coro-type.C: New test. --- gcc/cp/ChangeLog | 22 ++++ gcc/cp/coroutines.cc | 103 +++++++++++++----- gcc/testsuite/ChangeLog | 9 ++ .../coroutines/pr93458-1-missing-traits.C | 10 ++ .../g++.dg/coroutines/pr93458-2-bad-traits.C | 16 +++ .../coroutines/pr93458-3-missing-handle.C | 17 +++ .../coroutines/pr93458-4-bad-coro-handle.C | 21 ++++ .../coroutines/pr93458-5-bad-coro-type.C | 12 ++ 8 files changed, 184 insertions(+), 26 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/pr93458-1-missing-traits.C create mode 100644 gcc/testsuite/g++.dg/coroutines/pr93458-2-bad-traits.C create mode 100644 gcc/testsuite/g++.dg/coroutines/pr93458-3-missing-handle.C create mode 100644 gcc/testsuite/g++.dg/coroutines/pr93458-4-bad-coro-handle.C create mode 100644 gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 3144688f090..2429b1f1642 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,25 @@ +2020-02-03 Iain Sandoe + + PR c++/93458 + * coroutines.cc (struct coroutine_info): Add a bool flag to note + that we emitted an error for a bad function return type. + (get_coroutine_info): Tolerate an unset info table in case of + missing traits. + (find_coro_traits_template_decl): In case of error or if we didn't + find a type template, note we emitted the error and suppress + duplicates. + (find_coro_handle_template_decl): Likewise. + (instantiate_coro_traits): Only check for error_mark_node in the + return from lookup_qualified_name. + (coro_promise_type_found_p): Reorder initialization so that we check + for the traits and their usability before allocation of the info + table. Check for a suitable return type and emit a diagnostic for + here instead of relying on the lookup machinery. This allows the + error to have a better location, and means we can suppress multiple + copies. + (coro_function_valid_p): Re-check for a valid promise (and thus the + traits) before proceeding. Tolerate missing info as a fatal error. + 2020-02-03 Jason Merrill PR c++/88256 diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 62d92d9fc98..8a0ce384425 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -91,6 +91,8 @@ struct GTY((for_user)) coroutine_info tree promise_proxy; /* Likewise, a proxy promise instance. */ location_t first_coro_keyword; /* The location of the keyword that made this function into a coroutine. */ + /* Flags to avoid repeated errors for per-function issues. */ + bool coro_ret_type_error_emitted; }; struct coroutine_info_hasher : ggc_ptr_hash @@ -169,7 +171,8 @@ get_or_insert_coroutine_info (tree fn_decl) coroutine_info * get_coroutine_info (tree fn_decl) { - gcc_checking_assert (coroutine_info_table != NULL); + if (coroutine_info_table == NULL) + return NULL; coroutine_info **slot = coroutine_info_table->find_slot_with_hash (fn_decl, coroutine_info_hasher::hash (fn_decl), NO_INSERT); @@ -255,11 +258,25 @@ static GTY(()) tree void_coro_handle_type; static tree find_coro_traits_template_decl (location_t kw) { + /* If we are missing fundmental information, such as the traits, (or the + declaration found is not a type template), then don't emit an error for + every keyword in a TU, just do it once. */ + static bool traits_error_emitted = false; + tree traits_decl = lookup_qualified_name (std_node, coro_traits_identifier, - 0, true); - if (traits_decl == NULL_TREE || traits_decl == error_mark_node) + 0, + /*complain=*/!traits_error_emitted); + if (traits_decl == error_mark_node + || !DECL_TYPE_TEMPLATE_P (traits_decl)) { - error_at (kw, "cannot find % template"); + if (!traits_error_emitted) + { + gcc_rich_location richloc (kw); + error_at (&richloc, "coroutines require a traits template; cannot" + " find %<%E::%E%>", std_node, coro_traits_identifier); + inform (&richloc, "perhaps %<#include %> is missing"); + traits_error_emitted = true; + } return NULL_TREE; } else @@ -299,7 +316,7 @@ instantiate_coro_traits (tree fndecl, location_t kw) /*in_decl=*/NULL_TREE, /*context=*/NULL_TREE, /*entering scope=*/false, tf_warning_or_error); - if (traits_class == error_mark_node || traits_class == NULL_TREE) + if (traits_class == error_mark_node) { error_at (kw, "cannot instantiate %"); return NULL_TREE; @@ -313,11 +330,18 @@ instantiate_coro_traits (tree fndecl, location_t kw) static tree find_coro_handle_template_decl (location_t kw) { + /* As for the coroutine traits, this error is per TU, so only emit + it once. */ + static bool coro_handle_error_emitted = false; tree handle_decl = lookup_qualified_name (std_node, coro_handle_identifier, - 0, true); - if (handle_decl == NULL_TREE || handle_decl == error_mark_node) - { - error_at (kw, "cannot find % template"); + 0, !coro_handle_error_emitted); + if (handle_decl == error_mark_node + || !DECL_CLASS_TEMPLATE_P (handle_decl)) + { + if (!coro_handle_error_emitted) + error_at (kw, "coroutines require a handle class template;" + " cannot find %<%E::%E%>", std_node, coro_handle_identifier); + coro_handle_error_emitted = true; return NULL_TREE; } else @@ -370,34 +394,42 @@ coro_promise_type_found_p (tree fndecl, location_t loc) { gcc_assert (fndecl != NULL_TREE); - /* Save the coroutine data on the side to avoid the overhead on every - function decl. */ - - /* We only need one entry per coroutine in a TU, the assumption here is that - there are typically not 1000s. */ if (!coro_initialized) { - gcc_checking_assert (coroutine_info_table == NULL); - /* A table to hold the state, per coroutine decl. */ - coroutine_info_table = - hash_table::create_ggc (11); - /* Set up the identifiers we will use. */ - gcc_checking_assert (coro_traits_identifier == NULL); + /* Trees we only need to create once. + Set up the identifiers we will use. */ coro_init_identifiers (); - /* Trees we only need to create once. */ + /* Coroutine traits template. */ coro_traits_templ = find_coro_traits_template_decl (loc); - gcc_checking_assert (coro_traits_templ != NULL); + if (coro_traits_templ == NULL_TREE) + return false; + /* coroutine_handle<> template. */ coro_handle_templ = find_coro_handle_template_decl (loc); - gcc_checking_assert (coro_handle_templ != NULL); + if (coro_handle_templ == NULL_TREE) + return false; + /* We can also instantiate the void coroutine_handle<> */ void_coro_handle_type = instantiate_coro_handle_for_promise_type (loc, NULL_TREE); - gcc_checking_assert (void_coro_handle_type != NULL); + if (void_coro_handle_type == NULL_TREE) + return false; + + /* A table to hold the state, per coroutine decl. */ + gcc_checking_assert (coroutine_info_table == NULL); + coroutine_info_table = + hash_table::create_ggc (11); + + if (coroutine_info_table == NULL) + return false; + coro_initialized = true; } + /* Save the coroutine data on the side to avoid the overhead on every + function decl tree. */ + coroutine_info *coro_info = get_or_insert_coroutine_info (fndecl); /* Without this, we cannot really proceed. */ gcc_checking_assert (coro_info); @@ -407,6 +439,19 @@ coro_promise_type_found_p (tree fndecl, location_t loc) { /* Get the coroutine traits template class instance for the function signature we have - coroutine_traits */ + tree return_type = TREE_TYPE (TREE_TYPE (fndecl)); + if (!CLASS_TYPE_P (return_type)) + { + /* It makes more sense to show the function header for this, even + though we will have encountered it when processing a keyword. + Only emit the error once, not for every keyword we encounter. */ + if (!coro_info->coro_ret_type_error_emitted) + error_at (DECL_SOURCE_LOCATION (fndecl), "coroutine return type" + " %qT is not a class", return_type); + coro_info->coro_ret_type_error_emitted = true; + return false; + } + tree templ_class = instantiate_coro_traits (fndecl, loc); /* Find the promise type for that. */ @@ -597,11 +642,17 @@ coro_function_valid_p (tree fndecl) { location_t f_loc = DECL_SOURCE_LOCATION (fndecl); + /* For cases where fundamental information cannot be found, e.g. the + coroutine traits are missing, we need to punt early. */ + if (!coro_promise_type_found_p (fndecl, f_loc)) + return false; + /* Since we think the function is a coroutine, that implies we parsed a keyword that triggered this. Keywords check promise validity for their context and thus the promise type should be known at this point. */ - gcc_checking_assert (get_coroutine_handle_type (fndecl) != NULL_TREE - && get_coroutine_promise_type (fndecl) != NULL_TREE); + if (get_coroutine_handle_type (fndecl) == NULL_TREE + || get_coroutine_promise_type (fndecl) == NULL_TREE) + return false; if (current_function_returns_value || current_function_returns_null) { diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 9e2bbda7fdf..28ecbe1472e 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2020-02-03 Iain Sandoe + + PR c++/93458 + * g++.dg/coroutines/pr93458-1-missing-traits.C: New test. + * g++.dg/coroutines/pr93458-2-bad-traits.C: New test. + * g++.dg/coroutines/pr93458-3-missing-handle.C: New test. + * g++.dg/coroutines/pr93458-4-bad-coro-handle.C: New test. + * g++.dg/coroutines/pr93458-5-bad-coro-type.C: New test. + 2020-02-03 David Malcolm PR analyzer/93544 diff --git a/gcc/testsuite/g++.dg/coroutines/pr93458-1-missing-traits.C b/gcc/testsuite/g++.dg/coroutines/pr93458-1-missing-traits.C new file mode 100644 index 00000000000..638a10606f3 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr93458-1-missing-traits.C @@ -0,0 +1,10 @@ +// { dg-additional-options "-fsyntax-only -fexceptions -w" } + +// Diagose missing traits (e.g. fail to include ). + +int +bad_coroutine (void) +{ + co_yield 5; // { dg-error {coroutines require a traits template; cannot find 'std::coroutine_traits'} } + co_return; +} diff --git a/gcc/testsuite/g++.dg/coroutines/pr93458-2-bad-traits.C b/gcc/testsuite/g++.dg/coroutines/pr93458-2-bad-traits.C new file mode 100644 index 00000000000..0466af8e15a --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr93458-2-bad-traits.C @@ -0,0 +1,16 @@ +// { dg-additional-options "-fsyntax-only -fexceptions -w" } + +// Diagose bad traits traits : fake something faulty. + +namespace std { + // name is present, but not a template. + struct coroutine_traits { + }; +} + +int +bad_coroutine (void) +{ + co_yield 5; // { dg-error {coroutines require a traits template; cannot find 'std::coroutine_traits'} } + co_return; +} diff --git a/gcc/testsuite/g++.dg/coroutines/pr93458-3-missing-handle.C b/gcc/testsuite/g++.dg/coroutines/pr93458-3-missing-handle.C new file mode 100644 index 00000000000..766f740fcbd --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr93458-3-missing-handle.C @@ -0,0 +1,17 @@ +// { dg-additional-options "-fsyntax-only -fexceptions -w" } + +// Diagose missing coroutine handle class template. + +namespace std { + // coroutine traits + template struct coroutine_traits { + using promise_type = typename _R::promise_type; + }; +} + +int +bad_coroutine (void) +{ + co_yield 5; // { dg-error {coroutines require a handle class template; cannot find 'std::coroutine_handle'} } + co_return; +} diff --git a/gcc/testsuite/g++.dg/coroutines/pr93458-4-bad-coro-handle.C b/gcc/testsuite/g++.dg/coroutines/pr93458-4-bad-coro-handle.C new file mode 100644 index 00000000000..5d212ace8d4 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr93458-4-bad-coro-handle.C @@ -0,0 +1,21 @@ +// { dg-additional-options "-fsyntax-only -fexceptions -w" } + +// Diagose missing coroutine handle class template. + +namespace std { + // coroutine traits + template struct coroutine_traits { + using promise_type = typename _R::promise_type; + }; + + // name is present, but not a template. + struct coroutine_handle { + }; +} + +int +bad_coroutine (void) +{ + co_yield 5; // { dg-error {coroutines require a handle class template; cannot find 'std::coroutine_handle'} } + co_return; +} diff --git a/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C b/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C new file mode 100644 index 00000000000..8bb58cc0a78 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C @@ -0,0 +1,12 @@ +// { dg-additional-options "-fsyntax-only -fexceptions -w" } + +// Diagose bad coroutine function type. + +#include "coro.h" + +int +bad_coroutine (void) // { dg-error {coroutine return type 'int' is not a class} } +{ + co_yield 5; + co_return; +} -- 2.30.2