[PATCH v2] c++: const_cast of null pointer in constant expr [PR99176]
Jason Merrill
jason@redhat.com
Thu Feb 25 21:24:21 GMT 2021
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<int*>(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
>>> <https://gcc.gnu.org/legacy-ml/gcc-patches/2016-06/msg01447.html>
>>> and here
>>> <https://gcc.gnu.org/legacy-ml/gcc-patches/2016-07/msg00280.html>.
>>>
>>> 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<B*>(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<T>::{,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<int*>(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
> <https://gcc.gnu.org/legacy-ml/gcc-patches/2016-06/msg01447.html>
> and here
> <https://gcc.gnu.org/legacy-ml/gcc-patches/2016-07/msg00280.html>.
>
> 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<B*>(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<T>::{,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<T>::{,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<int *>(vp0); // { dg-error "cast from .void\\*. is not allowed" }
> +constexpr int *p4 = static_cast<int *>(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<int*>(p);
> +constexpr int *q2 = (int *)(const int *) nullptr;
> +
> +struct B { };
> +struct D : B { };
> +constexpr B *q3 = static_cast<B*>(nullptr);
> +constexpr D *pd = nullptr;
> +constexpr B *pb = nullptr;
> +constexpr B *q4 = static_cast<B*>(pd);
> +constexpr D *q5 = static_cast<D*>(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<A*>(pd0); // { dg-error "ambiguous base" }
>
> constexpr B *pb1 = pa0; // { dg-error "invalid conversion|not a constant expression" }
> -constexpr B *pb2 = static_cast<B*>(pa0); // { dg-error "not a constant expression" }
> +constexpr B *pb2 = static_cast<B*>(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
>
More information about the Gcc-patches
mailing list