diff mbox

Fix strtold on 32-bit sparc (and probably others) (BZ #16965)

Message ID 1400593238-10317-1-git-send-email-aurelien@aurel32.net
State Committed
Headers

Commit Message

Aurelien Jarno May 20, 2014, 1:40 p.m. UTC 
  This patch fixes an issue observed running the tst-strtod-round test on
32 bit sparc. In some conditions, strtold calls round_and_return, which in
turn calls __mpn_rshift with cnt = 0, while stdlib/rshift.c explicitly says
that cnts should satisfy 0 < CNT < BITS_PER_MP_LIMB. In this case, the code
end up doing a logical shift right of the same amount than the register,
which is undefined in the C standard.

Due to this bug, 32-bit sparc does not correctly convert the value
"0x1p-16446", but it is likely that other architectures are also
affected for other input values.
---
 ChangeLog         |  6 ++++++
 stdlib/strtod_l.c | 11 ++++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

Comments

Joseph Myers May 20, 2014, 2:34 p.m. UTC | #1 
On Tue, 20 May 2014, Aurelien Jarno wrote:

> This patch fixes an issue observed running the tst-strtod-round test on
> 32 bit sparc. In some conditions, strtold calls round_and_return, which in
> turn calls __mpn_rshift with cnt = 0, while stdlib/rshift.c explicitly says
> that cnts should satisfy 0 < CNT < BITS_PER_MP_LIMB. In this case, the code
> end up doing a logical shift right of the same amount than the register,
> which is undefined in the C standard.

OK (presuming you at least ran the stdlib tests on 32-bit sparc, and that 
you include the usual NEWS update and bug closing when committing).
Aurelien Jarno May 20, 2014, 4:48 p.m. UTC | #2 
On Tue, May 20, 2014 at 02:34:14PM +0000, Joseph S. Myers wrote:
> On Tue, 20 May 2014, Aurelien Jarno wrote:
> 
> > This patch fixes an issue observed running the tst-strtod-round test on
> > 32 bit sparc. In some conditions, strtold calls round_and_return, which in
> > turn calls __mpn_rshift with cnt = 0, while stdlib/rshift.c explicitly says
> > that cnts should satisfy 0 < CNT < BITS_PER_MP_LIMB. In this case, the code
> > end up doing a logical shift right of the same amount than the register,
> > which is undefined in the C standard.
> 
> OK (presuming you at least ran the stdlib tests on 32-bit sparc, and that 
> you include the usual NEWS update and bug closing when committing).

Yes I ran the full tests on both 32-bit sparc and x86-64, I haven't
seen any regression, and a progression for tst-strtod-round on sparc.

Thanks for the review.
diff mbox

Patch

diff --git a/ChangeLog b/ChangeLog
index fc75ff2..5755561 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@ 
+2014-05-20  Aurelien Jarno  <aurelien@aurel32.net>
+
+	[BZ #16965]
+	* stdlib/strtod_l.c (round_and_return): Add code to shift limbs
+	when the shift amount is modulo the limb size.
+
 2014-05-20  Will Newton  <will.newton@linaro.org>
 
 	* sysdeps/unix/sysv/linux/aarch64/nptl/sysdep-cancel.h (PSEUDO):
diff --git a/stdlib/strtod_l.c b/stdlib/strtod_l.c
index 6707e48..3c449c7 100644
--- a/stdlib/strtod_l.c
+++ b/stdlib/strtod_l.c
@@ -243,9 +243,14 @@  round_and_return (mp_limb_t *retval, intmax_t exponent, int negative,
 	  more_bits |= ((round_limb & ((((mp_limb_t) 1) << round_bit) - 1))
 			!= 0);
 
-	  (void) __mpn_rshift (retval, &retval[shift / BITS_PER_MP_LIMB],
-			       RETURN_LIMB_SIZE - (shift / BITS_PER_MP_LIMB),
-			       shift % BITS_PER_MP_LIMB);
+	  /* __mpn_rshift requires 0 < shift < BITS_PER_MP_LIMB.  */
+	  if ((shift % BITS_PER_MP_LIMB) != 0)
+	    (void) __mpn_rshift (retval, &retval[shift / BITS_PER_MP_LIMB],
+			         RETURN_LIMB_SIZE - (shift / BITS_PER_MP_LIMB),
+			         shift % BITS_PER_MP_LIMB);
+	  else
+	    for (i = 0; i < RETURN_LIMB_SIZE - (shift / BITS_PER_MP_LIMB); i++)
+	      retval[i] = retval[i + (shift / BITS_PER_MP_LIMB)];
 	  MPN_ZERO (&retval[RETURN_LIMB_SIZE - (shift / BITS_PER_MP_LIMB)],
 		    shift / BITS_PER_MP_LIMB);
 	}