From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 8F27B385703C for ; Thu, 11 May 2023 18:32:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8F27B385703C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1683829960; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=YhmaYRziP9jcsrxpjkyLqekEjrR9yh4k+dIeZx+kwVo=; b=hHg0N/jcJG1yS//ZniZjgnkNglLnWA2wssunGpKIN977pXiqSdCY1SYe3r0wyHooWMJ3vg oYv6TcwvhV76BIDHK4DExKndL9RolPue87KjgdbUT2oef4nIte4+D2rvTnXYsJgy2RTHhi Gn/3K5qSxKYn14FHkSH9sAktjUWQGho= Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-119-AD1svFyMNpacpX48vvaj4g-1; Thu, 11 May 2023 14:32:39 -0400 X-MC-Unique: AD1svFyMNpacpX48vvaj4g-1 Received: by mail-qk1-f197.google.com with SMTP id af79cd13be357-7578369dff3so1857529985a.0 for ; Thu, 11 May 2023 11:32:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683829958; x=1686421958; h=mime-version:references:message-id:in-reply-to:subject:cc:to:date :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=YhmaYRziP9jcsrxpjkyLqekEjrR9yh4k+dIeZx+kwVo=; b=GB9ULpM7ZznmE0xk9UVeE1p2xZuvFjn4CkZ+whg3E6plHlLG64CzR5SiUVcn4U0WVw 5i62pnFlEI6a+3n9I4G74NpOI5UpsWgQNa9ysbUTVSt3O87XNhEyI1XoJihsXPbb0WjT iY3xAi3kkLFhGzKQuwUhAo9guBXKjyGZtPmbmXrZcncxG1iuTqlmnO+7CcLWuc4tqeXV DkG424H90Rsf/GUF0+MQKoUHBzvaVgeMUKu16L8BEEPkzvpyvt0sLtGBgmvo3sHYNEgV IdeR6wc65lB1Olhk3Qhc6+iVWlP0jYnwsCrBcUtDSB4m90IxfnuNhxtw40v0jEX9IZA4 tZXg== X-Gm-Message-State: AC+VfDxZr3wtqpapVLYnC9Ai1I/YAFCv67vr1mY20jPcVl3SKBL2W4X+ OCBfAQ5UVi37g94KBTLf/2lJ5Pu1L2jsdfwKcg4clUr4KnBXf7Ag0ls1cLSi/1pLGCVfkvvfXNN OZnh2yznDe2t63F8KdQ== X-Received: by 2002:a05:6214:491:b0:616:7977:2460 with SMTP id pt17-20020a056214049100b0061679772460mr37563855qvb.24.1683829958476; Thu, 11 May 2023 11:32:38 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7N7EbWjl13oqBXTwsIgi2zJE10wfnDtTv9yeqjewj5WY1nQFlOwbs4dKlQxqbrMe17JVRdRw== X-Received: by 2002:a05:6214:491:b0:616:7977:2460 with SMTP id pt17-20020a056214049100b0061679772460mr37563818qvb.24.1683829958133; Thu, 11 May 2023 11:32:38 -0700 (PDT) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id p16-20020a0cf550000000b006166b169573sm2432441qvm.66.2023.05.11.11.32.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 May 2023 11:32:37 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Thu, 11 May 2023 14:32:36 -0400 (EDT) To: Patrick Palka cc: gcc-patches@gcc.gnu.org, jason@redhat.com Subject: Re: [PATCH] c++: 'mutable' subobject of constexpr variable [PR109745] In-Reply-To: <20230511183006.1565721-1-ppalka@redhat.com> Message-ID: References: <20230511183006.1565721-1-ppalka@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-13.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, 11 May 2023, Patrick Palka wrote: > r13-2701-g7107ea6fb933f1 made us correctly accept 'mutable' member > accesses during constexpr evaluation of objects constructed during that > evaluation, while continuing to reject such accesses for constexpr > objects constructed outside of that evaluation, by considering the > CONSTRUCTOR_MUTABLE_POISON flag during cxx_eval_component_reference. > > However, this flag is set only for the outermost CONSTRUCTOR of a > constexpr variable initializer, so if we're accessing a 'mutable' > subobject within a nested CONSTRUCTOR, the flag won't be set and > we'll incorrectly accept the access. This can lead to us rejecting > valid code, as in the first testcase, or even wrong code due to d'oh, this should say "this can lead to us accepting invalid code" > speculative constexpr evaluation as in the second and third testcase. > > This patch fixes this by setting CONSTRUCTOR_MUTABLE_POISON recursively > rather than only on the outermost CONSTRUCTOR. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk/13? > > PR c++/109745 > > gcc/cp/ChangeLog: > > * typeck2.cc (poison_mutable_constructors): Define. > (store_init_value): Use it instead of setting > CONSTRUCTOR_MUTABLE_POISON directly. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/constexpr-mutable4.C: New test. > * g++.dg/cpp0x/constexpr-mutable5.C: New test. > * g++.dg/cpp1y/constexpr-mutable2.C: New test. > --- > gcc/cp/typeck2.cc | 26 +++++++++++-- > .../g++.dg/cpp0x/constexpr-mutable4.C | 16 ++++++++ > .../g++.dg/cpp0x/constexpr-mutable5.C | 39 +++++++++++++++++++ > .../g++.dg/cpp1y/constexpr-mutable2.C | 20 ++++++++++ > 4 files changed, 97 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-mutable4.C > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-mutable5.C > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-mutable2.C > > diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc > index f5cc7c8371c..8a187708482 100644 > --- a/gcc/cp/typeck2.cc > +++ b/gcc/cp/typeck2.cc > @@ -776,6 +776,27 @@ split_nonconstant_init (tree dest, tree init) > return code; > } > > +/* T is the initializer of a constexpr variable. Set CONSTRUCTOR_MUTABLE_POISON > + for any CONSTRUCTOR within T that contains (directly or indirectly) a mutable > + member, thereby poisoning it so it can't be copied to another a constexpr > + variable, or read during constexpr evaluation. */ > + > +static void > +poison_mutable_constructors (tree t) > +{ > + if (TREE_CODE (t) != CONSTRUCTOR) > + return; > + > + if (cp_has_mutable_p (TREE_TYPE (t))) > + { > + CONSTRUCTOR_MUTABLE_POISON (t) = true; > + > + if (vec *elts = CONSTRUCTOR_ELTS (t)) > + for (const constructor_elt &ce : *elts) > + poison_mutable_constructors (ce.value); > + } > +} > + > /* Perform appropriate conversions on the initial value of a variable, > store it in the declaration DECL, > and print any error messages that are appropriate. > @@ -886,10 +907,7 @@ store_init_value (tree decl, tree init, vec** cleanups, int flags) > else > value = fold_non_dependent_init (value, tf_warning_or_error, > /*manifestly_const_eval=*/true, decl); > - if (TREE_CODE (value) == CONSTRUCTOR && cp_has_mutable_p (type)) > - /* Poison this CONSTRUCTOR so it can't be copied to another > - constexpr variable. */ > - CONSTRUCTOR_MUTABLE_POISON (value) = true; > + poison_mutable_constructors (value); > const_init = (reduced_constant_expression_p (value) > || error_operand_p (value)); > DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = const_init; > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-mutable4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-mutable4.C > new file mode 100644 > index 00000000000..01f32dea1bd > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-mutable4.C > @@ -0,0 +1,16 @@ > +// PR c++/109745 > +// { dg-do compile { target c++11 } } > + > +struct A { mutable int m = 0; }; > + > +struct B { A a; }; > + > +struct C { B b; }; > + > +int main() { > + constexpr B b; > + constexpr int bam = b.a.m; // { dg-error "mutable" } > + > + constexpr C c; > + constexpr int cbam = c.b.a.m; // { dg-error "mutable" } > +} > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-mutable5.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-mutable5.C > new file mode 100644 > index 00000000000..6a530e2abe6 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-mutable5.C > @@ -0,0 +1,39 @@ > +// PR c++/109745 > +// { dg-do run { target c++11 } } > +// { dg-additional-options "-O" } > + > +struct A { > + mutable int m = 0; > + void f() const { ++m; }; > + constexpr int get_m() const { return m; } > +}; > + > +struct B { A a; }; > + > +struct C { B b; }; > + > +int main() { > + constexpr A a; > + a.m++; > + if (a.get_m() != 1 || a.m != 1) > + __builtin_abort(); > + a.m++; > + if (a.get_m() != 2 || a.m != 2) > + __builtin_abort(); > + > + constexpr B b; > + b.a.m++; > + if (b.a.get_m() != 1 || b.a.m != 1) > + __builtin_abort(); > + b.a.m++; > + if (b.a.get_m() != 2 || b.a.m != 2) > + __builtin_abort(); > + > + constexpr C c; > + c.b.a.m++; > + if (c.b.a.get_m() != 1 || c.b.a.m != 1) > + __builtin_abort(); > + c.b.a.m++; > + if (c.b.a.get_m() != 2 || c.b.a.m != 2) > + __builtin_abort(); > +} > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-mutable2.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-mutable2.C > new file mode 100644 > index 00000000000..bb3429aea13 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-mutable2.C > @@ -0,0 +1,20 @@ > +// PR c++/109745 > +// { dg-do run { target c++14 } } > +// { dg-additional-options "-O" } > + > +template > +struct Foo { T val; }; > + > +struct Bar { > + constexpr Bar() = default; > + constexpr Bar(Bar const& other) { other.val_ = 42; } > + constexpr int val() const { return val_; } > + mutable int val_{}; > +}; > + > +int main() { > + constexpr Foo x{}; > + Foo y{x}; > + if (x.val.val() != 42 || x.val.val_ != 42) > + __builtin_abort(); > +} > -- > 2.40.1.552.g91428f078b > >