[1/2] c++: Make __builtin_launder reject invalid types [PR116673]

Tested x86_64-linux. OK for trunk?

-- >8 --

The standard says that std::launder is ill-formed for function pointers
and cv void pointers, so there's no reason for __builtin_launder to
accept them. This change allows implementations of std::launder to defer
to the built-in for error checking, although libstdc++ will continue to
diagnose it directly for more user-friendly diagnostics.

	PR c++/116673

gcc/cp/ChangeLog:

	* semantics.cc (finish_builtin_launder): Diagnose function
	pointers and cv void pointers.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp1z/launder10.C: New test.
---
 gcc/cp/semantics.cc                    | 17 +++++++++++++----
 gcc/testsuite/g++.dg/cpp1z/launder10.C | 15 +++++++++++++++
 2 files changed, 28 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/launder10.C

On 9/12/24 4:49 AM, Jonathan Wakely wrote:
> Tested x86_64-linux. OK for trunk?
> 
> -- >8 --
> 
> The standard says that std::launder is ill-formed for function pointers
> and cv void pointers, so there's no reason for __builtin_launder to
> accept them. This change allows implementations of std::launder to defer
> to the built-in for error checking, although libstdc++ will continue to
> diagnose it directly for more user-friendly diagnostics.
> 
> 	PR c++/116673
> 
> gcc/cp/ChangeLog:
> 
> 	* semantics.cc (finish_builtin_launder): Diagnose function
> 	pointers and cv void pointers.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp1z/launder10.C: New test.
> ---
>   gcc/cp/semantics.cc                    | 17 +++++++++++++----
>   gcc/testsuite/g++.dg/cpp1z/launder10.C | 15 +++++++++++++++
>   2 files changed, 28 insertions(+), 4 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/launder10.C
> 
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index 63212afafb3..b194b01f865 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -13482,11 +13482,20 @@ finish_builtin_launder (location_t loc, tree arg, tsubst_flags_t complain)
>       arg = decay_conversion (arg, complain);
>     if (error_operand_p (arg))
>       return error_mark_node;
> -  if (!type_dependent_expression_p (arg)
> -      && !TYPE_PTR_P (TREE_TYPE (arg)))
> +  if (!type_dependent_expression_p (arg))
>       {
> -      error_at (loc, "non-pointer argument to %<__builtin_launder%>");
> -      return error_mark_node;
> +      tree type = TREE_TYPE (arg);
> +      if (!TYPE_PTR_P (type))
> +	{
> +	  error_at (loc, "non-pointer argument to %<__builtin_launder%>");
> +	  return error_mark_node;
> +	}
> +      else if (!object_type_p (TREE_TYPE (type)))
> +	{
> +	  // std::launder is ill-formed for function and cv void pointers.
> +	  error_at (loc, "invalid argument to %<__builtin_launder%>");

Let's be more specific by combining both errors into

"type %qT of argument to %<__builtin_launder"> is not a pointer to 
object type"

The tests can also be combined to !TYPE_PTROB_P.

OK with that change.

> +	  return error_mark_node;
> +	}
>       }
>     if (processing_template_decl)
>       arg = orig_arg;
> diff --git a/gcc/testsuite/g++.dg/cpp1z/launder10.C b/gcc/testsuite/g++.dg/cpp1z/launder10.C
> new file mode 100644
> index 00000000000..7c15eeb891f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/launder10.C
> @@ -0,0 +1,15 @@
> +// PR c++/116673
> +// { dg-do compile }
> +
> +void
> +bar (void *p)
> +{
> +  __builtin_launder (bar); // { dg-error {invalid argument to '__builtin_launder'} }
> +  __builtin_launder (p);   // { dg-error {invalid argument to '__builtin_launder'} }
> +  const void* cp = p;
> +  __builtin_launder (cp);  // { dg-error {invalid argument to '__builtin_launder'} }
> +  volatile void* vp = p;
> +  __builtin_launder (vp);  // { dg-error {invalid argument to '__builtin_launder'} }
> +  const volatile void* cvp = p;
> +  __builtin_launder (cvp); // { dg-error {invalid argument to '__builtin_launder'} }
> +}

On Thu, 12 Sep 2024, Jonathan Wakely wrote:

> Tested x86_64-linux. OK for trunk?
> 
> -- >8 --
> 
> The standard says that std::launder is ill-formed for function pointers
> and cv void pointers, so there's no reason for __builtin_launder to
> accept them. This change allows implementations of std::launder to defer
> to the built-in for error checking, although libstdc++ will continue to
> diagnose it directly for more user-friendly diagnostics.
> 
> 	PR c++/116673
> 
> gcc/cp/ChangeLog:
> 
> 	* semantics.cc (finish_builtin_launder): Diagnose function
> 	pointers and cv void pointers.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp1z/launder10.C: New test.
> ---
>  gcc/cp/semantics.cc                    | 17 +++++++++++++----
>  gcc/testsuite/g++.dg/cpp1z/launder10.C | 15 +++++++++++++++
>  2 files changed, 28 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp1z/launder10.C
> 
> diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> index 63212afafb3..b194b01f865 100644
> --- a/gcc/cp/semantics.cc
> +++ b/gcc/cp/semantics.cc
> @@ -13482,11 +13482,20 @@ finish_builtin_launder (location_t loc, tree arg, tsubst_flags_t complain)
>      arg = decay_conversion (arg, complain);
>    if (error_operand_p (arg))
>      return error_mark_node;
> -  if (!type_dependent_expression_p (arg)
> -      && !TYPE_PTR_P (TREE_TYPE (arg)))
> +  if (!type_dependent_expression_p (arg))
>      {
> -      error_at (loc, "non-pointer argument to %<__builtin_launder%>");
> -      return error_mark_node;
> +      tree type = TREE_TYPE (arg);
> +      if (!TYPE_PTR_P (type))
> +	{
> +	  error_at (loc, "non-pointer argument to %<__builtin_launder%>");

Do we care about making this builtin SFINAE-friendly by guarding these
errors with tf_error?

> +	  return error_mark_node;
> +	}
> +      else if (!object_type_p (TREE_TYPE (type)))
> +	{
> +	  // std::launder is ill-formed for function and cv void pointers.
> +	  error_at (loc, "invalid argument to %<__builtin_launder%>");
> +	  return error_mark_node;
> +	}
>      }
>    if (processing_template_decl)
>      arg = orig_arg;
> diff --git a/gcc/testsuite/g++.dg/cpp1z/launder10.C b/gcc/testsuite/g++.dg/cpp1z/launder10.C
> new file mode 100644
> index 00000000000..7c15eeb891f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/launder10.C
> @@ -0,0 +1,15 @@
> +// PR c++/116673
> +// { dg-do compile }
> +
> +void
> +bar (void *p)
> +{
> +  __builtin_launder (bar); // { dg-error {invalid argument to '__builtin_launder'} }
> +  __builtin_launder (p);   // { dg-error {invalid argument to '__builtin_launder'} }
> +  const void* cp = p;
> +  __builtin_launder (cp);  // { dg-error {invalid argument to '__builtin_launder'} }
> +  volatile void* vp = p;
> +  __builtin_launder (vp);  // { dg-error {invalid argument to '__builtin_launder'} }
> +  const volatile void* cvp = p;
> +  __builtin_launder (cvp); // { dg-error {invalid argument to '__builtin_launder'} }
> +}
> -- 
> 2.46.0
> 
>

On Thu, 12 Sept 2024 at 19:38, Patrick Palka <ppalka@redhat.com> wrote:
>
> On Thu, 12 Sep 2024, Jonathan Wakely wrote:
>
> > Tested x86_64-linux. OK for trunk?
> >
> > -- >8 --
> >
> > The standard says that std::launder is ill-formed for function pointers
> > and cv void pointers, so there's no reason for __builtin_launder to
> > accept them. This change allows implementations of std::launder to defer
> > to the built-in for error checking, although libstdc++ will continue to
> > diagnose it directly for more user-friendly diagnostics.
> >
> >       PR c++/116673
> >
> > gcc/cp/ChangeLog:
> >
> >       * semantics.cc (finish_builtin_launder): Diagnose function
> >       pointers and cv void pointers.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * g++.dg/cpp1z/launder10.C: New test.
> > ---
> >  gcc/cp/semantics.cc                    | 17 +++++++++++++----
> >  gcc/testsuite/g++.dg/cpp1z/launder10.C | 15 +++++++++++++++
> >  2 files changed, 28 insertions(+), 4 deletions(-)
> >  create mode 100644 gcc/testsuite/g++.dg/cpp1z/launder10.C
> >
> > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> > index 63212afafb3..b194b01f865 100644
> > --- a/gcc/cp/semantics.cc
> > +++ b/gcc/cp/semantics.cc
> > @@ -13482,11 +13482,20 @@ finish_builtin_launder (location_t loc, tree arg, tsubst_flags_t complain)
> >      arg = decay_conversion (arg, complain);
> >    if (error_operand_p (arg))
> >      return error_mark_node;
> > -  if (!type_dependent_expression_p (arg)
> > -      && !TYPE_PTR_P (TREE_TYPE (arg)))
> > +  if (!type_dependent_expression_p (arg))
> >      {
> > -      error_at (loc, "non-pointer argument to %<__builtin_launder%>");
> > -      return error_mark_node;
> > +      tree type = TREE_TYPE (arg);
> > +      if (!TYPE_PTR_P (type))
> > +     {
> > +       error_at (loc, "non-pointer argument to %<__builtin_launder%>");
>
> Do we care about making this builtin SFINAE-friendly by guarding these
> errors with tf_error?

I don't think so. I don't think there's any use case for using
__builtin_launder (or std::launder for that matter) in deduction
contexts.

>
> > +       return error_mark_node;
> > +     }
> > +      else if (!object_type_p (TREE_TYPE (type)))
> > +     {
> > +       // std::launder is ill-formed for function and cv void pointers.
> > +       error_at (loc, "invalid argument to %<__builtin_launder%>");
> > +       return error_mark_node;
> > +     }
> >      }
> >    if (processing_template_decl)
> >      arg = orig_arg;
> > diff --git a/gcc/testsuite/g++.dg/cpp1z/launder10.C b/gcc/testsuite/g++.dg/cpp1z/launder10.C
> > new file mode 100644
> > index 00000000000..7c15eeb891f
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp1z/launder10.C
> > @@ -0,0 +1,15 @@
> > +// PR c++/116673
> > +// { dg-do compile }
> > +
> > +void
> > +bar (void *p)
> > +{
> > +  __builtin_launder (bar); // { dg-error {invalid argument to '__builtin_launder'} }
> > +  __builtin_launder (p);   // { dg-error {invalid argument to '__builtin_launder'} }
> > +  const void* cp = p;
> > +  __builtin_launder (cp);  // { dg-error {invalid argument to '__builtin_launder'} }
> > +  volatile void* vp = p;
> > +  __builtin_launder (vp);  // { dg-error {invalid argument to '__builtin_launder'} }
> > +  const volatile void* cvp = p;
> > +  __builtin_launder (cvp); // { dg-error {invalid argument to '__builtin_launder'} }
> > +}
> > --
> > 2.46.0
> >
> >
>