public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libgcc: Another __divmodbitint4 bug fix [PR114762]
@ 2024-04-19  6:17 Jakub Jelinek
  2024-04-19  6:33 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2024-04-19  6:17 UTC (permalink / raw)
  To: Richard Biener, Joseph S. Myers; +Cc: gcc-patches

Hi!

The following testcase is miscompiled because the code to decrement
vn on negative value with all ones in most significant limb (even partial)
and 0 in most significant bit of the second most significant limb doesn't
take into account the case where all bits below the most significant limb
are zero.  This has been a problem both in the version before yesterday's
commit where it has been done only if un was one shorter than vn before this
decrement, and is now problem even more often when it is done earlier.
When we decrement vn in such case and negate it, we end up with all 0s in
the v2 value, so have both the problems with UB on __builtin_clz* and the
expectations of the algorithm that the divisor has most significant bit set
after shifting, plus when the decremented vn is 1 it can SIGFPE on division
by zero even when it is not division by zero etc.  Other values shouldn't
get 0 in the new most significant limb after negation, because the
bitint_reduce_prec canonicalization should reduce prec if the second most
significant limb is all ones and if that limb is all zeros, if at least
one limb below it is non-zero, carry in will make it non-zero.

The following patch fixes it by checking if at least one bit below the
most significant limb is non-zero, in that case it decrements, otherwise
it will do nothing (but e.g. for the un < vn case that also means the
divisor is large enough that the result should be q 0 r u).

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

2024-04-19  Jakub Jelinek  <jakub@redhat.com>

	PR libgcc/114762
	* libgcc2.c (__divmodbitint4): Don't decrement vn if all bits
	below the most significant limb are zero.

	* gcc.dg/torture/bitint-70.c: New test.

--- libgcc/libgcc2.c.jj	2024-04-18 09:48:55.172538667 +0200
+++ libgcc/libgcc2.c	2024-04-18 12:17:28.893616007 +0200
@@ -1715,11 +1715,18 @@ __divmodbitint4 (UBILtype *q, SItype qpr
       && vn > 1
       && (Wtype) v[BITINT_END (1, vn - 2)] >= 0)
     {
-      vp = 0;
-      --vn;
+      /* Unless all bits below the most significant limb are zero.  */
+      SItype vn2;
+      for (vn2 = vn - 2; vn2 >= 0; --vn2)
+	if (v[BITINT_END (vn - 1 - vn2, vn2)])
+	  {
+	    vp = 0;
+	    --vn;
 #if __LIBGCC_BITINT_ORDER__ == __ORDER_BIG_ENDIAN__
-      ++v;
+	    ++v;
 #endif
+	    break;
+	  }
     }
   if (__builtin_expect (un < vn, 0))
     {
--- gcc/testsuite/gcc.dg/torture/bitint-70.c.jj	2024-04-18 12:26:09.406383158 +0200
+++ gcc/testsuite/gcc.dg/torture/bitint-70.c	2024-04-18 12:26:57.253718287 +0200
@@ -0,0 +1,22 @@
+/* PR libgcc/114762 */
+/* { dg-do run { target bitint } } */
+/* { dg-options "-std=c23" } */
+/* { dg-skip-if "" { ! run_expensive_tests }  { "*" } { "-O0" "-O2" } } */
+/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
+
+#if __BITINT_MAXWIDTH__ >= 255
+__attribute__((__noipa__)) signed _BitInt(255)
+foo (signed _BitInt(255) a, signed _BitInt(65) b)
+{
+  return a / b;
+}
+#endif
+
+int
+main ()
+{
+#if __BITINT_MAXWIDTH__ >= 255
+  if (foo (1, -0xffffffffffffffffwb - 1wb))
+    __builtin_abort ();
+#endif
+}

	Jakub


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] libgcc: Another __divmodbitint4 bug fix [PR114762]
  2024-04-19  6:17 [PATCH] libgcc: Another __divmodbitint4 bug fix [PR114762] Jakub Jelinek
@ 2024-04-19  6:33 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2024-04-19  6:33 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Joseph S. Myers, gcc-patches

On Fri, 19 Apr 2024, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase is miscompiled because the code to decrement
> vn on negative value with all ones in most significant limb (even partial)
> and 0 in most significant bit of the second most significant limb doesn't
> take into account the case where all bits below the most significant limb
> are zero.  This has been a problem both in the version before yesterday's
> commit where it has been done only if un was one shorter than vn before this
> decrement, and is now problem even more often when it is done earlier.
> When we decrement vn in such case and negate it, we end up with all 0s in
> the v2 value, so have both the problems with UB on __builtin_clz* and the
> expectations of the algorithm that the divisor has most significant bit set
> after shifting, plus when the decremented vn is 1 it can SIGFPE on division
> by zero even when it is not division by zero etc.  Other values shouldn't
> get 0 in the new most significant limb after negation, because the
> bitint_reduce_prec canonicalization should reduce prec if the second most
> significant limb is all ones and if that limb is all zeros, if at least
> one limb below it is non-zero, carry in will make it non-zero.
> 
> The following patch fixes it by checking if at least one bit below the
> most significant limb is non-zero, in that case it decrements, otherwise
> it will do nothing (but e.g. for the un < vn case that also means the
> divisor is large enough that the result should be q 0 r u).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2024-04-19  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR libgcc/114762
> 	* libgcc2.c (__divmodbitint4): Don't decrement vn if all bits
> 	below the most significant limb are zero.
> 
> 	* gcc.dg/torture/bitint-70.c: New test.
> 
> --- libgcc/libgcc2.c.jj	2024-04-18 09:48:55.172538667 +0200
> +++ libgcc/libgcc2.c	2024-04-18 12:17:28.893616007 +0200
> @@ -1715,11 +1715,18 @@ __divmodbitint4 (UBILtype *q, SItype qpr
>        && vn > 1
>        && (Wtype) v[BITINT_END (1, vn - 2)] >= 0)
>      {
> -      vp = 0;
> -      --vn;
> +      /* Unless all bits below the most significant limb are zero.  */
> +      SItype vn2;
> +      for (vn2 = vn - 2; vn2 >= 0; --vn2)
> +	if (v[BITINT_END (vn - 1 - vn2, vn2)])
> +	  {
> +	    vp = 0;
> +	    --vn;
>  #if __LIBGCC_BITINT_ORDER__ == __ORDER_BIG_ENDIAN__
> -      ++v;
> +	    ++v;
>  #endif
> +	    break;
> +	  }
>      }
>    if (__builtin_expect (un < vn, 0))
>      {
> --- gcc/testsuite/gcc.dg/torture/bitint-70.c.jj	2024-04-18 12:26:09.406383158 +0200
> +++ gcc/testsuite/gcc.dg/torture/bitint-70.c	2024-04-18 12:26:57.253718287 +0200
> @@ -0,0 +1,22 @@
> +/* PR libgcc/114762 */
> +/* { dg-do run { target bitint } } */
> +/* { dg-options "-std=c23" } */
> +/* { dg-skip-if "" { ! run_expensive_tests }  { "*" } { "-O0" "-O2" } } */
> +/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
> +
> +#if __BITINT_MAXWIDTH__ >= 255
> +__attribute__((__noipa__)) signed _BitInt(255)
> +foo (signed _BitInt(255) a, signed _BitInt(65) b)
> +{
> +  return a / b;
> +}
> +#endif
> +
> +int
> +main ()
> +{
> +#if __BITINT_MAXWIDTH__ >= 255
> +  if (foo (1, -0xffffffffffffffffwb - 1wb))
> +    __builtin_abort ();
> +#endif
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-04-19  6:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-19  6:17 [PATCH] libgcc: Another __divmodbitint4 bug fix [PR114762] Jakub Jelinek
2024-04-19  6:33 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).