From patchwork Fri Dec 6 16:28:36 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 1205198 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-515353-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="xl2H7PeU"; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="RLnNUZwm"; 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 47TyhR4S2jz9sR7 for ; Sat, 7 Dec 2019 03:28:54 +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:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; q=dns; s=default; b=MLngSp9j8eeGC3M63I3bvNsTwXlRx 5KXP5xAyWKvDseCvESAmR956cVx4vVzXKqWeUIjZ2i75LeTSyBjsiWaCLd14+m0w V7GC92i8eLkNWQTHcYFwgbopYyFdV4dFJ9IZ1SO/6EencAUZaBmGr9eTnBvIPqz+ t/C5W2YE2T2CWo= 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:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; s=default; bh=IxQ520v7v0zcUDBWvz7ekZoo6qg=; b=xl2 H7PeUo+nAcVbCU7y7DKOfGDr0geuKkJY23hFWmlLmqZqJkxrc+jmQspf+WMEyG7e BoNg5mo0GpUktJk13gADKT3XIYIM4E8EWEWvGF/NiSAtr8XgdS4ZMCEgG/7yCh+6 XNbKGBdV0iQ6SJZtlGudLx4g2DHxQHwIs6tf7lX4= Received: (qmail 89467 invoked by alias); 6 Dec 2019 16:28:46 -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 89457 invoked by uid 89); 6 Dec 2019 16:28:46 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-7.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3 autolearn=ham version=3.3.1 spammy=leaf, sk:type_ha, void_type_node X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (207.211.31.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 06 Dec 2019 16:28:44 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1575649722; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type; bh=HJ2+3ZjeRkjIZnEgizV0dXAUwI3ALaZXh8/Yvbbchog=; b=RLnNUZwm0XUp93uNQiDMFUztCGy9v/PoTKfTSSZm0fIO1FudlvhHIti724RKzfN+hoQZj6 zZewfohIRVetDBM7e4RT1eLP5FIzwabWWyo9nONq+owF2oZX+aEIT8Z8lE4jDZBCc51UPJ y97EOG7rt9dWUBnFI+rCxDkStOiy7pQ= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-159-TTezld2gOReqrGUqUm-ZNA-1; Fri, 06 Dec 2019 11:28:41 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 11CE0800EB9 for ; Fri, 6 Dec 2019 16:28:40 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-117-59.ams2.redhat.com [10.36.117.59]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9DE131001B30; Fri, 6 Dec 2019 16:28:39 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id xB6GSbxx025248; Fri, 6 Dec 2019 17:28:37 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id xB6GSauG025247; Fri, 6 Dec 2019 17:28:36 +0100 Date: Fri, 6 Dec 2019 17:28:36 +0100 From: Jakub Jelinek To: Jason Merrill Cc: gcc-patches@gcc.gnu.org Subject: [C++ PATCH] CWG 1299, not extending temporary lifetime for ?: (PR c++/92831) Message-ID: <20191206162836.GE10088@tucnak> Reply-To: Jakub Jelinek MIME-Version: 1.0 User-Agent: Mutt/1.11.3 (2019-02-01) X-Mimecast-Spam-Score: 0 Content-Disposition: inline X-IsSubscribed: yes Hi! This is a reason latest firefox is miscompiled by G++ 9, seems DR 1299 says in [class.temporary]/(6.7) that reference binding to a temporary object from the second or third operand of a conditional expression should have extended lifetime too, but extend_ref_init_temps_1 was apparently handling only comma expressions, some casts, . (COMPONENT_REF), [] (ARRAY_REF), but not COND_EXPR. The following patch handles COND_EXPR in there too, in a similar way how the gimplifier handles the cleanups of the expressions in the COND_EXPR operand if there is no lifetime extension due to reference binding. In particular there are bool temporaries added, initialized to false and set to true if the particular (leaf) COND_EXPR argument has been invoked and the corresponding cleanup guarded that way. I admit I haven't tried to construct testcases for all the things CWG 1299 added, e.g. don't know if ( expression ) will work, if all the *_cast cases that need to be extended work (some of them do, as the testcase shows), or if e.g. .* works, so I'm not claiming the DR is completely implemented. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and after a while for release branches? Note, just to double check g++ doesn't ICE on expr1 ? : expr2 GNU extension, I also wrote the attached testcase, but unlike the testcase included in the patch which behaves the same with patched g++ as does recent clang++, the testcase with expr1 ? : expr2 (omitted second operand) actually never extends the lifetime of any temporary (that is the baz function), unlike clang++ which extends the lifetime of expr2 if expr1 is false, and doesn't extend lifetime of anything if expr1 is true. This is outside of the scope of the standard, so no idea what is correct, so I'm just asking how it should behave. extend_ref_init_temps_1 is never called in that case when the expression is COND_EXPR. 2019-12-06 Jakub Jelinek PR c++/92831 - CWG 1299, not extending temporary lifetime for ?: * cp-tree.h (extend_ref_init_temps): Add a new argument with NULL default arg. * call.c (set_up_extended_ref_temp): Add COND_GUARD argument, pass it down to extend_ref_init_temps. Before pushing cleanup, if COND_GUARD is non-NULL, create a bool temporary if needed, initialize to false and guard the cleanup with the temporary being true. (extend_ref_init_temps_1): Add COND_GUARD argument, pass it down to recursive calls and set_up_extended_ref_temp. Handle COND_EXPR. (extend_ref_init_temps): Add COND_GUARD argument, pass it down to recursive calls and to extend_ref_init_temps_1. * g++.dg/cpp0x/temp-extend2.C: New test. Jakub template using id = T; struct S { S () : s (false) { ++c; } S (bool x) : s (x) { ++c; } ~S () { --c; }; bool s; static int c; }; int S::c = 0; extern "C" int printf (const char *, ...); void bar () { printf ("%d\n", S::c); } void foo (int i) { const bool&& a = id{false, true, false}[i].s ? id{true, false}[i].s : id{true, false, true, false}[i].s; bar (); } void baz (int i) { const bool&& a = id{false, true, false}[i].s ? : id{true, false, true, false}[i].s; bar (); } int main () { bar (); foo (0); bar (); foo (1); bar (); baz (0); bar (); baz (1); bar (); } --- gcc/cp/cp-tree.h.jj 2019-12-05 10:03:04.111181297 +0100 +++ gcc/cp/cp-tree.h 2019-12-06 11:00:43.230257916 +0100 @@ -6321,7 +6321,9 @@ extern tree convert_for_arg_passing (tr extern bool is_properly_derived_from (tree, tree); extern tree initialize_reference (tree, tree, int, tsubst_flags_t); -extern tree extend_ref_init_temps (tree, tree, vec**); +extern tree extend_ref_init_temps (tree, tree, + vec**, + tree * = NULL); extern tree make_temporary_var_for_ref_to_temp (tree, tree); extern bool type_has_extended_temps (tree); extern tree strip_top_quals (tree); --- gcc/cp/call.c.jj 2019-12-06 00:40:29.439856520 +0100 +++ gcc/cp/call.c 2019-12-06 11:00:17.111660971 +0100 @@ -11965,7 +11965,7 @@ make_temporary_var_for_ref_to_temp (tree static tree set_up_extended_ref_temp (tree decl, tree expr, vec **cleanups, - tree *initp) + tree *initp, tree *cond_guard) { tree init; tree type; @@ -11996,7 +11996,8 @@ set_up_extended_ref_temp (tree decl, tre /* Recursively extend temps in this initializer. */ TARGET_EXPR_INITIAL (expr) - = extend_ref_init_temps (decl, TARGET_EXPR_INITIAL (expr), cleanups); + = extend_ref_init_temps (decl, TARGET_EXPR_INITIAL (expr), cleanups, + cond_guard); /* Any reference temp has a non-trivial initializer. */ DECL_NONTRIVIALLY_INITIALIZED_P (var) = true; @@ -12037,7 +12038,24 @@ set_up_extended_ref_temp (tree decl, tre { tree cleanup = cxx_maybe_build_cleanup (var, tf_warning_or_error); if (cleanup) - vec_safe_push (*cleanups, cleanup); + { + if (cond_guard && cleanup != error_mark_node) + { + if (*cond_guard == NULL_TREE) + { + *cond_guard = build_local_temp (boolean_type_node); + add_decl_expr (*cond_guard); + tree set = cp_build_modify_expr (UNKNOWN_LOCATION, + *cond_guard, NOP_EXPR, + boolean_false_node, + tf_warning_or_error); + finish_expr_stmt (set); + } + cleanup = build3 (COND_EXPR, void_type_node, + *cond_guard, cleanup, NULL_TREE); + } + vec_safe_push (*cleanups, cleanup); + } } /* We must be careful to destroy the temporary only @@ -12142,7 +12160,8 @@ initialize_reference (tree type, tree ex which is bound either to a reference or a std::initializer_list. */ static tree -extend_ref_init_temps_1 (tree decl, tree init, vec **cleanups) +extend_ref_init_temps_1 (tree decl, tree init, vec **cleanups, + tree *cond_guard) { tree sub = init; tree *p; @@ -12150,20 +12169,52 @@ extend_ref_init_temps_1 (tree decl, tree if (TREE_CODE (sub) == COMPOUND_EXPR) { TREE_OPERAND (sub, 1) - = extend_ref_init_temps_1 (decl, TREE_OPERAND (sub, 1), cleanups); + = extend_ref_init_temps_1 (decl, TREE_OPERAND (sub, 1), cleanups, + cond_guard); + return init; + } + if (TREE_CODE (sub) == COND_EXPR) + { + tree cur_cond_guard = NULL_TREE; + if (TREE_OPERAND (sub, 1)) + TREE_OPERAND (sub, 1) + = extend_ref_init_temps_1 (decl, TREE_OPERAND (sub, 1), cleanups, + &cur_cond_guard); + if (cur_cond_guard) + { + tree set = cp_build_modify_expr (UNKNOWN_LOCATION, cur_cond_guard, + NOP_EXPR, boolean_true_node, + tf_warning_or_error); + TREE_OPERAND (sub, 1) + = cp_build_compound_expr (set, TREE_OPERAND (sub, 1), + tf_warning_or_error); + } + cur_cond_guard = NULL_TREE; + if (TREE_OPERAND (sub, 2)) + TREE_OPERAND (sub, 2) + = extend_ref_init_temps_1 (decl, TREE_OPERAND (sub, 2), cleanups, + &cur_cond_guard); + if (cur_cond_guard) + { + tree set = cp_build_modify_expr (UNKNOWN_LOCATION, cur_cond_guard, + NOP_EXPR, boolean_true_node, + tf_warning_or_error); + TREE_OPERAND (sub, 2) + = cp_build_compound_expr (set, TREE_OPERAND (sub, 2), + tf_warning_or_error); + } return init; } if (TREE_CODE (sub) != ADDR_EXPR) return init; /* Deal with binding to a subobject. */ for (p = &TREE_OPERAND (sub, 0); - (TREE_CODE (*p) == COMPONENT_REF - || TREE_CODE (*p) == ARRAY_REF); ) + TREE_CODE (*p) == COMPONENT_REF || TREE_CODE (*p) == ARRAY_REF; ) p = &TREE_OPERAND (*p, 0); if (TREE_CODE (*p) == TARGET_EXPR) { tree subinit = NULL_TREE; - *p = set_up_extended_ref_temp (decl, *p, cleanups, &subinit); + *p = set_up_extended_ref_temp (decl, *p, cleanups, &subinit, cond_guard); recompute_tree_invariant_for_addr_expr (sub); if (init != sub) init = fold_convert (TREE_TYPE (init), sub); @@ -12178,13 +12229,14 @@ extend_ref_init_temps_1 (tree decl, tree lifetime to match that of DECL. */ tree -extend_ref_init_temps (tree decl, tree init, vec **cleanups) +extend_ref_init_temps (tree decl, tree init, vec **cleanups, + tree *cond_guard) { tree type = TREE_TYPE (init); if (processing_template_decl) return init; if (TYPE_REF_P (type)) - init = extend_ref_init_temps_1 (decl, init, cleanups); + init = extend_ref_init_temps_1 (decl, init, cleanups, cond_guard); else { tree ctor = init; @@ -12203,7 +12255,8 @@ extend_ref_init_temps (tree decl, tree i /* The temporary array underlying a std::initializer_list is handled like a reference temporary. */ tree array = CONSTRUCTOR_ELT (ctor, 0)->value; - array = extend_ref_init_temps_1 (decl, array, cleanups); + array = extend_ref_init_temps_1 (decl, array, cleanups, + cond_guard); CONSTRUCTOR_ELT (ctor, 0)->value = array; } else @@ -12212,7 +12265,8 @@ extend_ref_init_temps (tree decl, tree i constructor_elt *p; vec *elts = CONSTRUCTOR_ELTS (ctor); FOR_EACH_VEC_SAFE_ELT (elts, i, p) - p->value = extend_ref_init_temps (decl, p->value, cleanups); + p->value = extend_ref_init_temps (decl, p->value, cleanups, + cond_guard); } recompute_constructor_flags (ctor); if (decl_maybe_constant_var_p (decl) && TREE_CONSTANT (ctor)) --- gcc/testsuite/g++.dg/cpp0x/temp-extend2.C.jj 2019-12-06 11:08:09.694368207 +0100 +++ gcc/testsuite/g++.dg/cpp0x/temp-extend2.C 2019-12-06 11:10:58.040768156 +0100 @@ -0,0 +1,36 @@ +// PR c++/92831 +// { dg-do run { target c++11 } } + +template using id = T; +struct S { S () { s++; } ~S () { s--; } S (int) { s++; } static int s; }; +int S::s = 0; + +void +bar (bool cond, bool cond2) +{ + if (S::s != (cond ? cond2 ? 7 : 5 : cond2 ? 8 : 9)) + __builtin_abort (); +} + +void +foo (bool cond, bool cond2) +{ + int i = 1; + // temporary array has same lifetime as a + S&& a = id{1, 2, 3}[i]; + // temporary S has same lifetime as b + const S& b = static_cast(0); + // exactly one of the four temporaries is lifetime-extended + S&& c = cond ? cond2 ? id{1, 2, 3}[i] : static_cast(0) + : cond2 ? id{1, 2, 3, 4}[i] : id{1, 2, 3, 4, 5}[i]; + bar (cond, cond2); +} + +int +main () +{ + foo (true, true); + foo (true, false); + foo (false, true); + foo (false, false); +}