[PATCH] libgcc: Actually force divide by zero

[PATCH] libgcc: Actually force divide by zero

[Jakub Jelinek]

Hi!

Eric mentioned we have a code trying to divide by zero intentionally
in gcc (since r0-241 !).
But, it clearly doesn't do what it wanted (which I think is raise
SIGFPE if the target normally raises it upon division by zero)
since r7-4470-g606f928d3805614, because we replace the division instruction
by __builtin_trap (), which does raise some signal, but quite different,
and then sure, r12-6924-gc2b610e7c6c89fd optimizes away even that
__builtin_trap ().

So I think on the libgcc side we should just hide the fact we are dividing
by zero from the optimizers, but it raises the question about what Ada
actually mandates and wants for such cases.  Because in 7+ it can end up
with a different signal and supposedly different exception at least.

Ok for trunk if this passes bootstrap/regtest?

2022-02-03  Jakub Jelinek  <jakub@redhat.com>

	* libgcc2.c (__udivmoddi4): Add optimization barriers to actually
	ensure division by zero.

--- libgcc/libgcc2.c.jj	2022-01-11 23:11:23.810270199 +0100
+++ libgcc/libgcc2.c	2022-02-03 09:24:14.513682731 +0100
@@ -1022,8 +1022,13 @@ __udivmoddi4 (UDWtype n, UDWtype d, UDWt
 	{
 	  /* qq = NN / 0d */
 
-	  if (d0 == 0)
-	    d0 = 1 / d0;	/* Divide intentionally by zero.  */
+	  if (__builtin_expect (d0 == 0, 0))
+	    {
+	      UWtype one = 1;
+	      __asm ("" : "+g" (one));
+	      __asm ("" : "+g" (d0));
+	      d0 = one / d0;	/* Divide intentionally by zero.  */
+	    }
 
 	  udiv_qrnnd (q1, n1, 0, n1, d0);
 	  udiv_qrnnd (q0, n0, n1, n0, d0);
@@ -1068,8 +1073,13 @@ __udivmoddi4 (UDWtype n, UDWtype d, UDWt
 	{
 	  /* qq = NN / 0d */
 
-	  if (d0 == 0)
-	    d0 = 1 / d0;	/* Divide intentionally by zero.  */
+	  if (__builtin_expect (d0 == 0, 0))
+	    {
+	      UWtype one = 1;
+	      __asm ("" : "+g" (one));
+	      __asm ("" : "+g" (d0));
+	      d0 = one / d0;	/* Divide intentionally by zero.  */
+	    }
 
 	  count_leading_zeros (bm, d0);
 


	Jakub

Re: [PATCH] libgcc: Actually force divide by zero

[Richard Biener]

On Thu, 3 Feb 2022, Jakub Jelinek wrote:

> Hi!
> 
> Eric mentioned we have a code trying to divide by zero intentionally
> in gcc (since r0-241 !).
> But, it clearly doesn't do what it wanted (which I think is raise
> SIGFPE if the target normally raises it upon division by zero)
> since r7-4470-g606f928d3805614, because we replace the division instruction
> by __builtin_trap (), which does raise some signal, but quite different,
> and then sure, r12-6924-gc2b610e7c6c89fd optimizes away even that
> __builtin_trap ().
> 
> So I think on the libgcc side we should just hide the fact we are dividing
> by zero from the optimizers, but it raises the question about what Ada
> actually mandates and wants for such cases.  Because in 7+ it can end up
> with a different signal and supposedly different exception at least.
> 
> Ok for trunk if this passes bootstrap/regtest?

I think the special case we're trying hard to _not_ optimize
is literal 0/0 for the case where the user wants to preserve
a trap.  But I see 0/0 optimized to zero by EVRP even in GCC 7 ...

IMHO it makes sense to have a way to represent the div-by-zero
trap, either by a builtin or by such kind of a division (if only
for QOI and maybe documented as such).

That is, fold-const.c and now match.pd still have

/* Preserve explicit divisions by 0: the C++ front-end wants to detect
   undefined behavior in constexpr evaluation, and assuming that the 
division
   traps enables better optimizations than these anyway.  */
(for div (trunc_div ceil_div floor_div round_div exact_div)
 /* 0 / X is always zero.  */
 (simplify
  (div integer_zerop@0 @1)
  /* But not for 0 / 0 so that we can get the proper warnings and errors.  
*/
  (if (!integer_zerop (@1))
   @0))

it suggests we want to preserve all X / 0 when the 0 is literal but
I think we can go a bit further and require 0/0 to not unnecessarily
restrict other special cases.

Comments on the libgcc case below

> 2022-02-03  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* libgcc2.c (__udivmoddi4): Add optimization barriers to actually
> 	ensure division by zero.
> 
> --- libgcc/libgcc2.c.jj	2022-01-11 23:11:23.810270199 +0100
> +++ libgcc/libgcc2.c	2022-02-03 09:24:14.513682731 +0100
> @@ -1022,8 +1022,13 @@ __udivmoddi4 (UDWtype n, UDWtype d, UDWt
>  	{
>  	  /* qq = NN / 0d */
>  
> -	  if (d0 == 0)
> -	    d0 = 1 / d0;	/* Divide intentionally by zero.  */
> +	  if (__builtin_expect (d0 == 0, 0))
> +	    {
> +	      UWtype one = 1;
> +	      __asm ("" : "+g" (one));
> +	      __asm ("" : "+g" (d0));
> +	      d0 = one / d0;	/* Divide intentionally by zero.  */
> +	    }

I'm not sure why we even bother - division or modulo by zero is
undefined behavior and we are not emulating CPU behavior because
the wide instructions emulated here do not actually exist.  That
gives us the freedom of choosing the implementation defined
behavior.

That said, _if_ we choose to "care" I'd rather choose to
define the implementation to use the trap mechanism the
target provides and thus use __builtin_trap ().  That then
at least traps reliably, compared to the division by zero
which doesn't do that on all targets.

So I'm not sure there's anything to fix besides eventually
just deleting the d0 == 0 special case?

Richard.

Re: [PATCH] libgcc: Actually force divide by zero

[Michael Matz]

Hello,

On Thu, 3 Feb 2022, Richard Biener via Gcc-patches wrote:

> /* Preserve explicit divisions by 0: the C++ front-end wants to detect
>    undefined behavior in constexpr evaluation, and assuming that the 
> division
>    traps enables better optimizations than these anyway.  */
> (for div (trunc_div ceil_div floor_div round_div exact_div)
>  /* 0 / X is always zero.  */
>  (simplify
>   (div integer_zerop@0 @1)
>   /* But not for 0 / 0 so that we can get the proper warnings and errors.  
> */
>   (if (!integer_zerop (@1))
>    @0))
> 
> it suggests we want to preserve all X / 0 when the 0 is literal but
> I think we can go a bit further and require 0/0 to not unnecessarily
> restrict other special cases.

Just remember that 0/0 is completely different from X/0 (with X != 0), the 
latter is a sensible limit, the former is just non-sense.  There's a 
reason why one is a NaN and the other Inf in floating point.  So it does 
make sense to differ between both on integer side as well.

(i'm split mind on the usefullness of "1/x -> 0" vs. its effect on 
trapping behaviour)


Ciao,
Michael.

> 
> Comments on the libgcc case below
> 
> > 2022-02-03  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	* libgcc2.c (__udivmoddi4): Add optimization barriers to actually
> > 	ensure division by zero.
> > 
> > --- libgcc/libgcc2.c.jj	2022-01-11 23:11:23.810270199 +0100
> > +++ libgcc/libgcc2.c	2022-02-03 09:24:14.513682731 +0100
> > @@ -1022,8 +1022,13 @@ __udivmoddi4 (UDWtype n, UDWtype d, UDWt
> >  	{
> >  	  /* qq = NN / 0d */
> >  
> > -	  if (d0 == 0)
> > -	    d0 = 1 / d0;	/* Divide intentionally by zero.  */
> > +	  if (__builtin_expect (d0 == 0, 0))
> > +	    {
> > +	      UWtype one = 1;
> > +	      __asm ("" : "+g" (one));
> > +	      __asm ("" : "+g" (d0));
> > +	      d0 = one / d0;	/* Divide intentionally by zero.  */
> > +	    }
> 
> I'm not sure why we even bother - division or modulo by zero is
> undefined behavior and we are not emulating CPU behavior because
> the wide instructions emulated here do not actually exist.  That
> gives us the freedom of choosing the implementation defined
> behavior.
> 
> That said, _if_ we choose to "care" I'd rather choose to
> define the implementation to use the trap mechanism the
> target provides and thus use __builtin_trap ().  That then
> at least traps reliably, compared to the division by zero
> which doesn't do that on all targets.
> 
> So I'm not sure there's anything to fix besides eventually
> just deleting the d0 == 0 special case?
> 
> Richard.
>