From patchwork Wed Jan 29 15:39:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Iain Sandoe X-Patchwork-Id: 1231000 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-518505-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=sandoe.co.uk Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha1 header.s=default header.b=creHt0Lc; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48774w1xQSz9sRk for ; Thu, 30 Jan 2020 02:40:26 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; q=dns; s= default; b=Y5DVLBImjpzGukJRWsjc3LSIN7ghjNgNPt/nAydN7yGE2EVbkmekW D96HN2gM40Fdw//O/e4YvnvgP1t20s6TEFtZFJUVuxFHAD8Oe/6n8ygFYLh982Zb uA4QRoL3NPHwKPy8KVTDIv6hSJLIBgvsuUaA0CQrCN71JxD81yfwnY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; s=default; bh=d5zzkjKnxPvoi2vHOQ0XfZU3QHU=; b=creHt0Lc0UgbMj6CsLB0voaPE15s PJwPT35Ltks04jLEgPm1IQJo0ie2icRB3WpU1EVLrZADcFdYkZVUx6pKRUeRCE4y 0BjyHeZgtaOH2e9XSSZIcHIMMbwO/dY2PWU6adcq8MjhC6TE0DMyzNTrQ1X/lX40 /dHsiGyzvqFQpHo= Received: (qmail 102577 invoked by alias); 29 Jan 2020 15:40:18 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 102045 invoked by uid 89); 29 Jan 2020 15:40:17 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_COUK, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=H*f:sk:d9aaf06, 2997, H*MI:sk:d9aaf06, H*i:sk:d9aaf06 X-HELO: smtp2.wavenetuk.net Received: from smtp.wavenetuk.net (HELO smtp2.wavenetuk.net) (195.26.37.10) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 29 Jan 2020 15:40:15 +0000 Received: from [192.168.1.212] (host81-138-1-83.in-addr.btopenworld.com [81.138.1.83]) by smtp2.wavenetuk.net (Postfix) with ESMTPA id 4636360011A; Wed, 29 Jan 2020 15:40:11 +0000 (GMT) Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [PATCH, v2] coroutines: Fix ICE on invalid (PR93458). From: Iain Sandoe In-Reply-To: Date: Wed, 29 Jan 2020 15:39:52 +0000 Cc: GCC-patches Message-Id: <342C42BF-D3B0-4465-A809-6C3B6DC3A45A@sandoe.co.uk> References: To: Nathan Sidwell Hi Nathan, Nathan Sidwell wrote: > On 1/28/20 11:14 AM, Iain Sandoe wrote: >> + if (!error_emitted && >> + (traits_decl == NULL_TREE || traits_decl == error_mark_node)) >> { >> error_at (kw, "cannot find % template"); >> + error_emitted = true; >> return NULL_TREE; >> } > > don't you just want to protect the error_at call with error_emitted? yes, thanks. > You might want to pick a canonical 'error' value -- either NULL_TREE or error_mark_node, but not both? done. Made the function type error recorded per function too. OK now? thanks Iain 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-01-29 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): Note we emitted the error and suppress duplicates. (instantiate_coro_traits): Only check for error_mark_node in the return from lookup_qualified_name. (find_coro_handle_template_decl): Likewise. (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-01-29 Iain Sandoe * g++.dg/coroutines/pr93458-1-missing-traits.C: New test. * g++.dg/coroutines/pr93458-2-bad-coro-type.C: New test. --- gcc/cp/coroutines.cc | 86 +++++++++++++------ .../coroutines/pr93458-1-missing-traits.C | 10 +++ .../coroutines/pr93458-2-bad-coro-type.C | 12 +++ 3 files changed, 84 insertions(+), 24 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-coro-type.C diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index e8a6a4033f6..3ad80699ca0 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); @@ -257,9 +260,15 @@ find_coro_traits_template_decl (location_t kw) { tree traits_decl = lookup_qualified_name (std_node, coro_traits_identifier, 0, true); - if (traits_decl == NULL_TREE || traits_decl == error_mark_node) - { - error_at (kw, "cannot find % template"); + /* If we are missing fundmental information, such as the traits, then don't + emit this for every keyword in a TU. This particular error is per TU + so we don't need to keep the indicator per function. */ + static bool traits_error_emitted = false; + if (traits_decl == error_mark_node) + { + if (!traits_error_emitted) + error_at (kw, "cannot find % template"); + traits_error_emitted = true; return NULL_TREE; } else @@ -299,7 +308,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; @@ -315,7 +324,7 @@ find_coro_handle_template_decl (location_t kw) { tree handle_decl = lookup_qualified_name (std_node, coro_handle_identifier, 0, true); - if (handle_decl == NULL_TREE || handle_decl == error_mark_node) + if (handle_decl == error_mark_node) { error_at (kw, "cannot find % template"); return NULL_TREE; @@ -370,34 +379,45 @@ 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 + || coro_traits_templ == error_mark_node) + 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 + || coro_handle_templ == error_mark_node) + 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 + || void_coro_handle_type == error_mark_node) + 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 +427,18 @@ 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 */ + if (!CLASS_TYPE_P (TREE_TYPE (TREE_TYPE (fndecl)))) + { + /* 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), "a coroutine must have a" + " class or struct 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. */ @@ -422,7 +454,7 @@ coro_promise_type_found_p (tree fndecl, location_t loc) /* Try to find the handle type for the promise. */ tree handle_type = instantiate_coro_handle_for_promise_type (loc, coro_info->promise_type); - if (handle_type == NULL_TREE) + if (handle_type == NULL_TREE || handle_type == error_mark_node) return false; /* Complete this, we're going to use it. */ @@ -597,11 +629,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/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..46adccded98 --- /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 (fail to include ). + +int +bad_coroutine (void) +{ + co_yield 5; // { dg-error {cannot find 'coroutine traits' template} } + co_return; +} diff --git a/gcc/testsuite/g++.dg/coroutines/pr93458-2-bad-coro-type.C b/gcc/testsuite/g++.dg/coroutines/pr93458-2-bad-coro-type.C new file mode 100644 index 00000000000..aab9b34b69a --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/pr93458-2-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 {a coroutine must have a class or struct return type} } +{ + co_yield 5; + co_return; +}