coroutines: Fix ICE on invalid (PR93458).
authorIain Sandoe <iain@sandoe.co.uk>
Mon, 3 Feb 2020 19:15:31 +0000 (19:15 +0000)
committerIain Sandoe <iain@sandoe.co.uk>
Mon, 3 Feb 2020 19:48:23 +0000 (19:48 +0000)
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  <iain@sandoe.co.uk>

* 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  <iain@sandoe.co.uk>

* 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
gcc/cp/coroutines.cc
gcc/testsuite/ChangeLog
gcc/testsuite/g++.dg/coroutines/pr93458-1-missing-traits.C [new file with mode: 0644]
gcc/testsuite/g++.dg/coroutines/pr93458-2-bad-traits.C [new file with mode: 0644]
gcc/testsuite/g++.dg/coroutines/pr93458-3-missing-handle.C [new file with mode: 0644]
gcc/testsuite/g++.dg/coroutines/pr93458-4-bad-coro-handle.C [new file with mode: 0644]
gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C [new file with mode: 0644]

index 3144688f090c406a38fc16aeb95e045f27bd08b5..2429b1f164282171153bcb4cf4b0927b1f41a768 100644 (file)
@@ -1,3 +1,25 @@
+2020-02-03  Iain Sandoe  <iain@sandoe.co.uk>
+
+       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  <jason@redhat.com>
 
        PR c++/88256
index 62d92d9fc98099cdc8cb55e8ceff6e0c1d090773..8a0ce384425d08c51eba1da583ea248f0d36d3dd 100644 (file)
@@ -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<coroutine_info>
@@ -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 %<coroutine traits%> 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 <coroutine>%> 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 %<coroutine traits%>");
       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 %<coroutine handle%> 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<coroutine_info_hasher>::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<coroutine_info_hasher>::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 <R, ...>  */
+      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)
     {
index 9e2bbda7fdf8d36ff30760693d1e97935a4322d2..28ecbe1472ebbd90a4d36b2c3e4c8e031a2ccc10 100644 (file)
@@ -1,3 +1,12 @@
+2020-02-03  Iain Sandoe  <iain@sandoe.co.uk>
+
+       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  <dmalcolm@redhat.com>
 
        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 (file)
index 0000000..638a106
--- /dev/null
@@ -0,0 +1,10 @@
+//  { dg-additional-options "-fsyntax-only -fexceptions -w" }
+
+// Diagose missing traits (e.g. fail to include <coroutine>).
+
+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 (file)
index 0000000..0466af8
--- /dev/null
@@ -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 (file)
index 0000000..766f740
--- /dev/null
@@ -0,0 +1,17 @@
+//  { dg-additional-options "-fsyntax-only -fexceptions -w" }
+
+// Diagose missing coroutine handle class template.
+
+namespace std {
+  //  coroutine traits
+  template<typename _R, typename...> 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 (file)
index 0000000..5d212ac
--- /dev/null
@@ -0,0 +1,21 @@
+//  { dg-additional-options "-fsyntax-only -fexceptions -w" }
+
+// Diagose missing coroutine handle class template.
+
+namespace std {
+  //  coroutine traits
+  template<typename _R, typename...> 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 (file)
index 0000000..8bb58cc
--- /dev/null
@@ -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;
+}