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 [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 505D5398B425 for ; Thu, 25 Feb 2021 21:24:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 505D5398B425 Received: from mail-qv1-f72.google.com (mail-qv1-f72.google.com [209.85.219.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-217-FZvFycOQPsO4NQkXui4C-Q-1; Thu, 25 Feb 2021 16:24:24 -0500 X-MC-Unique: FZvFycOQPsO4NQkXui4C-Q-1 Received: by mail-qv1-f72.google.com with SMTP id h10so5282528qvf.19 for ; Thu, 25 Feb 2021 13:24:23 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=z0z1hKDiunw4Lnoc8kDPrycVztMXeeHMn2M8IXoHdCc=; b=U4PXfU5R7amhBiQ/Vu1UYfS0RE4x1ctfW1TPVrlaU9qFykHZq039E/i7O7Skeji9Yw QC6+5ZV1nr3sZoWX9Apc5dqakyOLQd6yDseysoGwtw85iRyl9ujDjK1LUJTTbiZaMlFC H4Qr9jkEayvr58uLmHO+aXc8r7/EQ84nkAfkL2JIcBIWFVJE1yzxiTZWLv4cJ1J42sgr n24RWqDp8AnEYxqA6qQHSHPhGEKaDLZ/jVYGlTCZ8mUk7+eCZ5CrAEIS7j96uE90/Tky mwIUR1DGhLZ6bikti+s7+VZ7zckcwr0PvUbVE8GkFExNBIv204Ud03pCjajdN9i9AoQP jYZA== X-Gm-Message-State: AOAM530MHRsuIU1aIo2pnSab6o9y4KaDJfW/w/7iHZ5Ar28CLCF3pjT4 0So+JE7PoYewZDlrnN7FsycSlN6KppDzFMRR7t+DJbEdEl/rirsgeXjTjxHByfH/KTpGjhshxkM g58lD680n/nDvAM0R8GlNLuUbiQ0K5Eqq7AdxvLYmK4bOhWF/PJ07ZFUhU7lpHPGJmQ== X-Received: by 2002:a05:620a:136e:: with SMTP id d14mr4620088qkl.456.1614288263028; Thu, 25 Feb 2021 13:24:23 -0800 (PST) X-Google-Smtp-Source: ABdhPJzFMIIVjDLb9X4eKGMjgkX/sjlgWlFlCcdwtKUWpT/tslZL1Xh+GsYU4MRAmWE3INFBzy1pJg== X-Received: by 2002:a05:620a:136e:: with SMTP id d14mr4620050qkl.456.1614288262482; Thu, 25 Feb 2021 13:24:22 -0800 (PST) Received: from [192.168.1.148] (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id x9sm4275854qtr.74.2021.02.25.13.24.21 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 25 Feb 2021 13:24:21 -0800 (PST) Subject: Re: [PATCH v2] c++: const_cast of null pointer in constant expr [PR99176] To: Marek Polacek Cc: GCC Patches References: <20210224223437.805775-1-polacek@redhat.com> From: Jason Merrill Message-ID: <96dca095-e809-57db-174b-4aa6b7b6dc1a@redhat.com> Date: Thu, 25 Feb 2021 16:24:21 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-16.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Feb 2021 21:24:28 -0000 On 2/25/21 4:20 PM, Marek Polacek wrote: > On Wed, Feb 24, 2021 at 10:32:59PM -0500, Jason Merrill wrote: >> On 2/24/21 5:34 PM, Marek Polacek wrote: >>> Here we reject >>> >>> constexpr const int *p = nullptr; >>> constexpr int *q = const_cast(p); >>> >>> with "conversion of 'const int*' null pointer to 'int*' is not a >>> constant expression", which seems bogus. This code has been rejected >>> since r238909 which added the can_convert check when converting a null >>> pointer. I'm not finding any standard rule that this check was supposed >>> to enforce. The original discussion was here >>> >>> and here >>> . >>> >>> Since can_convert never assumes a C-style cast, it rejects casting >>> away constness as in the test above and in: >>> >>> constexpr int *q = (int *)(const int *) nullptr; >>> >>> Removing the check only breaks constexpr-nullptr-2.C by not giving any >>> diagnostic for line 229: >>> >>> constexpr B *pb2 = static_cast(pa0); // { dg-error "not a constant expression" } >>> >>> but the cast seems to be valid: we do [expr.static.cast]/7, and >>> [expr.const] only says that a reinterpreter_cast and converting from >>> void* is invalid in constexpr. The can_convert check rejected convering >>> from void *, but only when converting from a null pointer, so it's not >>> good enough. So I've added a check to catch conversions from cv void*. >>> I realize it's not a great time to be adding additional checking, but >>> removing the can_convert check would then technically be a regression. >>> (I could perhaps limit the new check to only trigger for integer_zerop >>> and then remove it in GCC 12.) >> >> That sounds safest. > > Done then. > >>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? OK. >>> gcc/cp/ChangeLog: >>> >>> DR 1312 >>> PR c++/99176 >>> * constexpr.c (cxx_eval_constant_expression): Reject casting >>> from void * as per DR 1312. Don't check can_convert. >>> >>> gcc/testsuite/ChangeLog: >>> >>> DR 1312 >>> PR c++/99176 >>> * g++.dg/cpp0x/constexpr-nullptr-2.C: Adjust dg-error. >>> * g++.dg/cpp0x/constexpr-cast2.C: New test. >>> * g++.dg/cpp0x/constexpr-cast3.C: New test. >>> --- >>> gcc/cp/constexpr.c | 49 ++++++++++++------- >>> gcc/testsuite/g++.dg/cpp0x/constexpr-cast2.C | 16 ++++++ >>> gcc/testsuite/g++.dg/cpp0x/constexpr-cast3.C | 14 ++++++ >>> .../g++.dg/cpp0x/constexpr-nullptr-2.C | 4 +- >>> 4 files changed, 64 insertions(+), 19 deletions(-) >>> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast2.C >>> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast3.C >>> >>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c >>> index 377fe322ee8..adf575d3dc6 100644 >>> --- a/gcc/cp/constexpr.c >>> +++ b/gcc/cp/constexpr.c >>> @@ -6653,6 +6653,37 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, >>> return t; >>> } >>> + /* [expr.const]: a conversion from type cv void* to a pointer-to-object >>> + type cannot be part of a core constant expression as a resolution to >>> + DR 1312. */ >>> + if (TYPE_PTROB_P (type) >>> + && TYPE_PTR_P (TREE_TYPE (op)) >>> + && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (op))) >>> + /* Inside a call to std::construct_at or to >>> + std::allocator::{,de}allocate, we permit casting from void* >>> + because that is compiler-generated code. */ >>> + && !(ctx->call >>> + && ctx->call->fundef >>> + && (is_std_construct_at (ctx->call->fundef->decl) >>> + || is_std_allocator_allocate (ctx->call->fundef->decl)))) >> >> I wonder about adding overloads that take constexpr_call* so you don't need >> the non-null checks here. Up to you. > > That seems convenient. The downside is that then we'll check > ctx->call and ctx->call->fundef twice, but I guess that's not too bad. > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > -- >8 -- > Here we reject > > constexpr const int *p = nullptr; > constexpr int *q = const_cast(p); > > with "conversion of 'const int*' null pointer to 'int*' is not a > constant expression", which seems bogus. This code has been rejected > since r238909 which added the can_convert check when converting a null > pointer. I'm not finding any standard rule that this check was supposed > to enforce. The original discussion was here > > and here > . > > Since can_convert never assumes a C-style cast, it rejects casting > away constness as in the test above and in: > > constexpr int *q = (int *)(const int *) nullptr; > > Removing the check only breaks constexpr-nullptr-2.C by not giving any > diagnostic for line 229: > > constexpr B *pb2 = static_cast(pa0); // { dg-error "not a constant expression" } > > but the cast seems to be valid: we do [expr.static.cast]/7, and > [expr.const] only says that a reinterpreter_cast and converting from > void* is invalid in constexpr. The can_convert check rejected convering > from void *, but only when converting from a null pointer, so it's not > good enough. So I've added a check to catch conversions from cv void*. > I realize it's not a great time to be adding additional checking, but > removing the can_convert check would then technically be a regression. > > Let's limit the new check to only trigger for integer_zerop and then remove > it in GCC 12. > > gcc/cp/ChangeLog: > > DR 1312 > PR c++/99176 > * constexpr.c (is_std_construct_at): New overload. > (is_std_allocator_allocate): New overload. > (cxx_eval_call_expression): Use the new overloads. > (cxx_eval_constant_expression): Reject casting > from void * as per DR 1312. Don't check can_convert. > > gcc/testsuite/ChangeLog: > > DR 1312 > PR c++/99176 > * g++.dg/cpp0x/constexpr-nullptr-2.C: Adjust dg-error. > * g++.dg/cpp0x/constexpr-cast2.C: New test. > * g++.dg/cpp0x/constexpr-cast3.C: New test. > --- > gcc/cp/constexpr.c | 76 +++++++++++++------ > gcc/testsuite/g++.dg/cpp0x/constexpr-cast2.C | 16 ++++ > gcc/testsuite/g++.dg/cpp0x/constexpr-cast3.C | 14 ++++ > .../g++.dg/cpp0x/constexpr-nullptr-2.C | 4 +- > 4 files changed, 85 insertions(+), 25 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast2.C > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast3.C > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index 377fe322ee8..cd0a68e9fd6 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -1837,6 +1837,16 @@ is_std_construct_at (tree fndecl) > return name && id_equal (name, "construct_at"); > } > > +/* Overload for the above taking constexpr_call*. */ > + > +static inline bool > +is_std_construct_at (const constexpr_call *call) > +{ > + return (call > + && call->fundef > + && is_std_construct_at (call->fundef->decl)); > +} > + > /* Return true if FNDECL is std::allocator::{,de}allocate. */ > > static inline bool > @@ -1859,6 +1869,16 @@ is_std_allocator_allocate (tree fndecl) > return decl_in_std_namespace_p (decl); > } > > +/* Overload for the above taking constexpr_call*. */ > + > +static inline bool > +is_std_allocator_allocate (const constexpr_call *call) > +{ > + return (call > + && call->fundef > + && is_std_allocator_allocate (call->fundef->decl)); > +} > + > /* Return true if FNDECL is __dynamic_cast. */ > > static inline bool > @@ -2313,9 +2333,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, > if (TREE_CODE (t) == CALL_EXPR > && cxx_replaceable_global_alloc_fn (fun) > && (CALL_FROM_NEW_OR_DELETE_P (t) > - || (ctx->call > - && ctx->call->fundef > - && is_std_allocator_allocate (ctx->call->fundef->decl)))) > + || is_std_allocator_allocate (ctx->call))) > { > const int nargs = call_expr_nargs (t); > tree arg0 = NULL_TREE; > @@ -2423,9 +2441,7 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, > argument. */ > if (TREE_CODE (t) == CALL_EXPR > && cxx_placement_new_fn (fun) > - && ctx->call > - && ctx->call->fundef > - && is_std_construct_at (ctx->call->fundef->decl)) > + && is_std_construct_at (ctx->call)) > { > const int nargs = call_expr_nargs (t); > tree arg1 = NULL_TREE; > @@ -6653,6 +6669,36 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, > return t; > } > > + /* [expr.const]: a conversion from type cv void* to a pointer-to-object > + type cannot be part of a core constant expression as a resolution to > + DR 1312. */ > + if (integer_zerop (op) /* FIXME: Remove in GCC 12. */ > + && TYPE_PTROB_P (type) > + && TYPE_PTR_P (TREE_TYPE (op)) > + && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (op))) > + /* Inside a call to std::construct_at or to > + std::allocator::{,de}allocate, we permit casting from void* > + because that is compiler-generated code. */ > + && !is_std_construct_at (ctx->call) > + && !is_std_allocator_allocate (ctx->call)) > + { > + /* Likewise, don't error when casting from void* when OP is > + &heap uninit and similar. */ > + tree sop = tree_strip_nop_conversions (op); > + if (TREE_CODE (sop) == ADDR_EXPR > + && VAR_P (TREE_OPERAND (sop, 0)) > + && DECL_ARTIFICIAL (TREE_OPERAND (sop, 0))) > + /* OK */; > + else > + { > + if (!ctx->quiet) > + error_at (loc, "cast from %qT is not allowed", > + TREE_TYPE (op)); > + *non_constant_p = true; > + return t; > + } > + } > + > if (TREE_CODE (op) == PTRMEM_CST && !TYPE_PTRMEM_P (type)) > op = cplus_expand_constant (op); > > @@ -6671,26 +6717,10 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, > if (TYPE_REF_P (type)) > { > if (!ctx->quiet) > - error_at (loc, > - "dereferencing a null pointer"); > + error_at (loc, "dereferencing a null pointer"); > *non_constant_p = true; > return t; > } > - else if (TYPE_PTR_P (TREE_TYPE (op))) > - { > - tree from = TREE_TYPE (op); > - > - if (!can_convert (type, from, tf_none)) > - { > - if (!ctx->quiet) > - error_at (loc, > - "conversion of %qT null pointer to %qT " > - "is not a constant expression", > - from, type); > - *non_constant_p = true; > - return t; > - } > - } > } > else > { > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-cast2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-cast2.C > new file mode 100644 > index 00000000000..7c37f6a3f5a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-cast2.C > @@ -0,0 +1,16 @@ > +// DR 1312 - Simulated reinterpret_cast in constant expressions. > +// PR c++/99176 > +// { dg-do compile { target c++11 } } > + > +static int i; > +constexpr void *vp0 = nullptr; > +constexpr void *vpi = &i; > +constexpr int *p1 = (int *) vp0; // { dg-error "cast from .void\\*. is not allowed" } > +constexpr int *p2 = (int *) vpi; // { dg-error "cast from .void\\*. is not allowed" "integer_zerop" { xfail *-*-* } } > +constexpr int *p3 = static_cast(vp0); // { dg-error "cast from .void\\*. is not allowed" } > +constexpr int *p4 = static_cast(vpi); // { dg-error "cast from .void\\*. is not allowed" "integer_zerop" { xfail *-*-* } } > +constexpr void *p5 = vp0; > +constexpr void *p6 = vpi; > + > +constexpr int *pi = &i; > +constexpr bool b = ((int *)(void *) pi == pi); // { dg-error "cast from .void\\*. is not allowed" "integer_zerop" { xfail *-*-* } } > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-cast3.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-cast3.C > new file mode 100644 > index 00000000000..a330a99f7de > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-cast3.C > @@ -0,0 +1,14 @@ > +// PR c++/99176 > +// { dg-do compile { target c++11 } } > + > +constexpr const int *p = nullptr; > +constexpr int *q1 = const_cast(p); > +constexpr int *q2 = (int *)(const int *) nullptr; > + > +struct B { }; > +struct D : B { }; > +constexpr B *q3 = static_cast(nullptr); > +constexpr D *pd = nullptr; > +constexpr B *pb = nullptr; > +constexpr B *q4 = static_cast(pd); > +constexpr D *q5 = static_cast(pb); > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-2.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-2.C > index afb4b37be5a..92f3bbdc0a6 100644 > --- a/gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-2.C > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-nullptr-2.C > @@ -163,7 +163,7 @@ constexpr const void *pv2 = pv0; > constexpr void *pv3 = pv2; // { dg-error "invalid conversion|not a constant expression" } > constexpr const void *pv4 = pv2; > > -constexpr X *px4 = pv0; // { dg-error "invalid conversion|not a constant expression" } > +constexpr X *px4 = pv0; // { dg-error "cast from|invalid conversion|not a constant expression" } > > } > > @@ -226,7 +226,7 @@ constexpr A *pa3 = pd0; // { dg-error "ambiguous base" } > constexpr A *pa4 = static_cast(pd0); // { dg-error "ambiguous base" } > > constexpr B *pb1 = pa0; // { dg-error "invalid conversion|not a constant expression" } > -constexpr B *pb2 = static_cast(pa0); // { dg-error "not a constant expression" } > +constexpr B *pb2 = static_cast(pa0); > > constexpr C *pc1 = pa0; // { dg-error "invalid conversion|not a constant expression" } > constexpr D *pd1 = pa0; // { dg-error "ambiguous base|invalid conversion" } > > base-commit: ed255fd5eda5e2530779bb69b8805c916ddfe0c2 >