c++: Consider addresses of heap artificial vars always non-NULL [PR98988]

Hi!

With -fno-delete-null-pointer-checks which is e.g. implied by
-fsanitize=undefined or default on some embedded targets, the middle-end
folder doesn't consider addresses of global VAR_DECLs to be non-NULL, as one
of them could have address 0.  Still, I think malloc/operator new (at least
the nonthrowing) relies on NULL returns meaning allocation failure rather
than success.  Furthermore, the artificial VAR_DECLs we create for
constexpr new never actually live in the address space of the program,
so we can pretend they will never be NULL too.

The following patch does that, so that one can actually use constexpr
new/delete even with -fno-delete-null-pointer-checks or sanitizers
- otherwise delete itself tests whether the passed address is non-NULL and
that wouldn't fold to true if the pointer has been allocated with constexpr
new.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-02-09  Jakub Jelinek  <jakub@redhat.com>

	PR c++/98988
	* constexpr.c (cxx_eval_binary_expression): Fold equality comparison
	of address of heap artificial VAR_DECL against nullptr.

	* g++.dg/cpp2a/constexpr-new16.C: New test.


	Jakub

On 2/9/21 3:25 AM, Jakub Jelinek wrote:
> Hi!
> 
> With -fno-delete-null-pointer-checks which is e.g. implied by
> -fsanitize=undefined or default on some embedded targets, the middle-end
> folder doesn't consider addresses of global VAR_DECLs to be non-NULL, as one
> of them could have address 0.

Hmm, are these VAR_DECLs going into the symtab?  That seems undesirable.

> Still, I think malloc/operator new (at least
> the nonthrowing) relies on NULL returns meaning allocation failure rather
> than success.  Furthermore, the artificial VAR_DECLs we create for
> constexpr new never actually live in the address space of the program,
> so we can pretend they will never be NULL too.

> The following patch does that, so that one can actually use constexpr
> new/delete even with -fno-delete-null-pointer-checks or sanitizers
> - otherwise delete itself tests whether the passed address is non-NULL and
> that wouldn't fold to true if the pointer has been allocated with constexpr
> new.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2021-02-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/98988
> 	* constexpr.c (cxx_eval_binary_expression): Fold equality comparison
> 	of address of heap artificial VAR_DECL against nullptr.
> 
> 	* g++.dg/cpp2a/constexpr-new16.C: New test.
> 
> --- gcc/cp/constexpr.c.jj	2021-02-02 09:52:10.535234056 +0100
> +++ gcc/cp/constexpr.c	2021-02-08 17:24:28.774467744 +0100
> @@ -3157,6 +3157,32 @@ cxx_eval_binary_expression (const conste
>   	lhs = cplus_expand_constant (lhs);
>         else if (TREE_CODE (rhs) == PTRMEM_CST)
>   	rhs = cplus_expand_constant (rhs);
> +      if (r == NULL_TREE && POINTER_TYPE_P (TREE_TYPE (lhs)))
> +	{
> +	  tree op0 = lhs;
> +	  tree op1 = rhs;
> +	  if (integer_zerop (op0))
> +	    std::swap (op0, op1);
> +	  /* With -fno-delete-null-pointer-checks, generic folders
> +	     assume addresses of non-automatic VAR_DECLs could be NULL.
> +	     For constexpr new, as the vars aren't really living in
> +	     target memory, assume they will never be NULL.  */
> +	  if (integer_zerop (op1))
> +	    {
> +	      STRIP_NOPS (op0);
> +	      if (TREE_CODE (op0) == ADDR_EXPR)
> +		{
> +		  op0 = get_base_address (TREE_OPERAND (op0, 0));
> +		  if (op0
> +		      && VAR_P (op0)
> +		      && (DECL_NAME (op0) == heap_identifier
> +			  || DECL_NAME (op0) == heap_uninit_identifier
> +			  || DECL_NAME (op0) == heap_vec_identifier
> +			  || DECL_NAME (op0) == heap_vec_uninit_identifier))
> +		    r = constant_boolean_node (!is_code_eq, type);
> +		}
> +	    }
> +	}
>       }
>     if (code == POINTER_PLUS_EXPR && !*non_constant_p
>         && integer_zerop (lhs) && !integer_zerop (rhs))
> --- gcc/testsuite/g++.dg/cpp2a/constexpr-new16.C.jj	2021-02-08 17:25:37.130699213 +0100
> +++ gcc/testsuite/g++.dg/cpp2a/constexpr-new16.C	2021-02-08 17:26:49.175889198 +0100
> @@ -0,0 +1,13 @@
> +// PR c++/98988
> +// { dg-do compile { target c++20 } }
> +// { dg-options "-fno-delete-null-pointer-checks" }
> +
> +constexpr bool
> +foo ()
> +{
> +  auto ptr = new int();
> +  delete ptr;
> +  return true;
> +}
> +
> +static_assert (foo ());
> 
> 	Jakub
>

On Tue, Feb 09, 2021 at 03:00:20PM -0500, Jason Merrill wrote:
> > With -fno-delete-null-pointer-checks which is e.g. implied by
> > -fsanitize=undefined or default on some embedded targets, the middle-end
> > folder doesn't consider addresses of global VAR_DECLs to be non-NULL, as one
> > of them could have address 0.
> 
> Hmm, are these VAR_DECLs going into the symtab?  That seems undesirable.

They are not in the symtab from the C++ FE.  And we don't allow those VAR_DECLs or their
addresses to leak into the IL anyway.
But, they are TREE_STATIC because pretending they are automatic didn't
really work (e.g. address of automatic variables is not a constant expression).
The generic folding code uses maybe_nonzero_address predicate which
apprently creates a symtab entry for them but as nothing uses them they
won't be emitted.
If we wanted to avoid that, we'd need to either add some generic VAR_DECL
bit for those vars or add a target hook that would return true for them and
then handle them in the middle-end.

	Jakub

On 2/9/21 3:11 PM, Jakub Jelinek wrote:
> On Tue, Feb 09, 2021 at 03:00:20PM -0500, Jason Merrill wrote:
>>> With -fno-delete-null-pointer-checks which is e.g. implied by
>>> -fsanitize=undefined or default on some embedded targets, the middle-end
>>> folder doesn't consider addresses of global VAR_DECLs to be non-NULL, as one
>>> of them could have address 0.
>>
>> Hmm, are these VAR_DECLs going into the symtab?  That seems undesirable.
> 
> They are not in the symtab from the C++ FE.  And we don't allow those VAR_DECLs or their
> addresses to leak into the IL anyway.
> But, they are TREE_STATIC because pretending they are automatic didn't
> really work (e.g. address of automatic variables is not a constant expression).
> The generic folding code uses maybe_nonzero_address predicate which
> apprently creates a symtab entry for them but as nothing uses them they
> won't be emitted.

Ah.

> If we wanted to avoid that, we'd need to either add some generic VAR_DECL
> bit for those vars or add a target hook that would return true for them and
> then handle them in the middle-end.

I'm surprised that nonzero_address has such a limited set of things it 
will actually believe have non-zero addresses with 
-fno-delete-null-pointer-checks.  But it seems that we should be able to 
arrange to satisfy

>   if (definition && !DECL_EXTERNAL (decl)

since these "variables" are indeed defined within the current 
translation unit.

Jason