See <http://gcc.gnu.org/ml/gcc-patches/2008-06/msg01473.html>,
the message itself and the referred gcc PR. Here's the referred
fix. Assuming of course, that the reply from the gcc judges to
the RFC part will be "nay, that cast will not be blessed;
gcc-4.4 and possibly gcc-4.3.2 will have its way with it".
While preparing the _strtod_r fix, I noticed another
aliasing-flawed code in mprec.h, the Storeinc implementation, so
I included a fix for that. I haven't looked further...
I tried to stay close to the existing code to avoid problems
with future bugfix-imports, else I'd change to just use the
double_union and its accessors (located right above "U") instead
of "U".
Alternatively, we could change to compile newlib with
-fno-strict-aliasing. ;-) At any rate, passing
-Wstrict-aliasing=2 could have caught this and possibly other
similar bugs; I haven't checked if there are more. Level 3
doesn't catch this one, arguably a gcc limitation (refer to the
documentation).
Tested cross to cris-elf, crisv32-elf and mmix-knuth-mmixware
using the gcc testsuites.
Ok to commit?
2008-06-24 Hans-Peter Nilsson <hp@axis.com>
Fix strict-aliasing issues with _strtod_r and Storeinc.
* libc/stdlib/strtod.c (_strtod_r): Change local variables aadj,
rv, rv0 from double to type U. Use accessor macros dval, dword0
and dword1 for all accesses except for the ULtod call, where rv.i
replaces the pointer cast.
* libc/stdlib/mprec.h (U): Rename member L to i for easier re-use
of access macros. Tweak comment.
Remove #ifdef'd YES_ALIAS code.
(dword0, dword1, dval): Define in terms of uncast union member
access. Ditto for _DOUBLE_IS_32BITS variants.
(Storeinc): Replace aliasing-flawed microoptimized definition with
alternative suggested in comment. Remove now stale comment.
Index: libc/stdlib/mprec.h
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdlib/mprec.h,v
retrieving revision 1.7
diff -u -p -r1.7 mprec.h
--- libc/stdlib/mprec.h 31 Aug 2007 21:21:27 -0000 1.7
+++ libc/stdlib/mprec.h 24 Jun 2008 02:14:23 -0000
@@ -78,31 +78,18 @@ union double_union
#define word1(x) (x.i[1])
#endif
-
-/* The following is taken from gdtoaimp.h for use with new strtod. */
+/* The following is taken from gdtoaimp.h for use with new strtod, but
+ adjusted to avoid invalid type-punning. */
typedef __int32_t Long;
-typedef union { double d; __ULong L[2]; } U;
-
-#ifdef YES_ALIAS
-#define dval(x) x
-#ifdef IEEE_8087
-#define dword0(x) ((__ULong *)&x)[1]
-#define dword1(x) ((__ULong *)&x)[0]
-#else
-#define dword0(x) ((__ULong *)&x)[0]
-#define dword1(x) ((__ULong *)&x)[1]
-#endif
-#else /* !YES_ALIAS */
-#ifdef IEEE_8087
-#define dword0(x) ((U*)&x)->L[1]
-#define dword1(x) ((U*)&x)->L[0]
-#else
-#define dword0(x) ((U*)&x)->L[0]
-#define dword1(x) ((U*)&x)->L[1]
-#endif
-#define dval(x) ((U*)&x)->d
-#endif /* YES_ALIAS */
+/* Unfortunately, because __ULong might be a different type than
+ __uint32_t, we can't re-use union double_union as-is without
+ further edits in strtod.c. */
+typedef union { double d; __ULong i[2]; } U;
+
+#define dword0(x) word0(x)
+#define dword1(x) word1(x)
+#define dval(x) (x.d)
#undef SI
#ifdef Sudden_Underflow
@@ -111,17 +98,7 @@ typedef union { double d; __ULong L[2];
#define SI 0
#endif
-/* The following definition of Storeinc is appropriate for MIPS processors.
- * An alternative that might be better on some machines is
- * #define Storeinc(a,b,c) (*a++ = b << 16 | c & 0xffff)
- */
-#if defined (__IEEE_BYTES_LITTLE_ENDIAN) + defined (IEEE_8087) + defined (VAX)
-#define Storeinc(a,b,c) (((unsigned short *)a)[1] = (unsigned short)b, \
-((unsigned short *)a)[0] = (unsigned short)c, a++)
-#else
-#define Storeinc(a,b,c) (((unsigned short *)a)[0] = (unsigned short)b, \
-((unsigned short *)a)[1] = (unsigned short)c, a++)
-#endif
+#define Storeinc(a,b,c) (*(a)++ = (b) << 16 | (c) & 0xffff)
/* #define P DBL_MANT_DIG */
/* Ten_pmax = floor(P*log(2)/log(5)) */
@@ -167,11 +144,7 @@ typedef union { double d; __ULong L[2];
#define word0(x) (x.i[0])
#define word1(x) 0
-#ifdef YES_ALIAS
-#define dword0(x) ((__ULong *)&x)[0]
-#else
-#define dword0(x) ((U*)&x)->L[0]
-#endif
+#define dword0(x) word0(x)
#define dword1(x) 0
#else
Index: libc/stdlib/strtod.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdlib/strtod.c,v
retrieving revision 1.10
diff -u -p -r1.10 strtod.c
--- libc/stdlib/strtod.c 21 Feb 2008 17:14:14 -0000 1.10
+++ libc/stdlib/strtod.c 24 Jun 2008 02:14:23 -0000
@@ -218,7 +218,8 @@ _DEFUN (_strtod_r, (ptr, s00, se),
int bb2, bb5, bbe, bd2, bd5, bbbits, bs2, c, decpt, dsign,
e, e1, esign, i, j, k, nd, nd0, nf, nz, nz0, sign;
_CONST char *s, *s0, *s1;
- double aadj, aadj1, adj, rv, rv0;
+ double aadj, adj;
+ U aadj1, rv, rv0;
Long L;
__ULong y, z;
_Bigint *bb, *bb1, *bd, *bd0, *bs, *delta;
@@ -286,7 +287,7 @@ _DEFUN (_strtod_r, (ptr, s00, se),
copybits(bits, fpi.nbits, bb);
Bfree(ptr,bb);
}
- ULtod(((U*)&rv)->L, bits, exp, i);
+ ULtod(rv.i, bits, exp, i);
}}
goto ret;
}
@@ -469,7 +470,7 @@ _DEFUN (_strtod_r, (ptr, s00, se),
#ifdef Honor_FLT_ROUNDS
/* round correctly FLT_ROUNDS = 2 or 3 */
if (sign) {
- rv = -rv;
+ dval(rv) = -dval(rv);
sign = 0;
}
#endif
@@ -485,7 +486,7 @@ _DEFUN (_strtod_r, (ptr, s00, se),
#ifdef Honor_FLT_ROUNDS
/* round correctly FLT_ROUNDS = 2 or 3 */
if (sign) {
- rv = -rv;
+ dval(rv) = -dval(rv);
sign = 0;
}
#endif
@@ -513,7 +514,7 @@ _DEFUN (_strtod_r, (ptr, s00, se),
#ifdef Honor_FLT_ROUNDS
/* round correctly FLT_ROUNDS = 2 or 3 */
if (sign) {
- rv = -rv;
+ dval(rv) = -dval(rv);
sign = 0;
}
#endif
@@ -976,14 +977,14 @@ _DEFUN (_strtod_r, (ptr, s00, se),
}
if ((aadj = ratio(delta, bs)) <= 2.) {
if (dsign)
- aadj = aadj1 = 1.;
+ aadj = dval(aadj1) = 1.;
else if (dword1(rv) || dword0(rv) & Bndry_mask) {
#ifndef Sudden_Underflow
if (dword1(rv) == Tiny1 && !dword0(rv))
goto undfl;
#endif
aadj = 1.;
- aadj1 = -1.;
+ dval(aadj1) = -1.;
}
else {
/* special case -- power of FLT_RADIX to be */
@@ -993,24 +994,24 @@ _DEFUN (_strtod_r, (ptr, s00, se),
aadj = 1./FLT_RADIX;
else
aadj *= 0.5;
- aadj1 = -aadj;
+ dval(aadj1) = -aadj;
}
}
else {
aadj *= 0.5;
- aadj1 = dsign ? aadj : -aadj;
+ dval(aadj1) = dsign ? aadj : -aadj;
#ifdef Check_FLT_ROUNDS
switch(Rounding) {
case 2: /* towards +infinity */
- aadj1 -= 0.5;
+ dval(aadj1) -= 0.5;
break;
case 0: /* towards 0 */
case 3: /* towards -infinity */
- aadj1 += 0.5;
+ dval(aadj1) += 0.5;
}
#else
if (Flt_Rounds == 0)
- aadj1 += 0.5;
+ dval(aadj1) += 0.5;
#endif /*Check_FLT_ROUNDS*/
}
y = dword0(rv) & Exp_mask;
@@ -1020,7 +1021,7 @@ _DEFUN (_strtod_r, (ptr, s00, se),
if (y == Exp_msk1*(DBL_MAX_EXP+Bias-1)) {
dval(rv0) = dval(rv);
dword0(rv) -= P*Exp_msk1;
- adj = aadj1 * ulp(dval(rv));
+ adj = dval(aadj1) * ulp(dval(rv));
dval(rv) += adj;
if ((dword0(rv) & Exp_mask) >=
Exp_msk1*(DBL_MAX_EXP+Bias-P)) {
@@ -1042,18 +1043,18 @@ _DEFUN (_strtod_r, (ptr, s00, se),
if ((z = aadj) <= 0)
z = 1;
aadj = z;
- aadj1 = dsign ? aadj : -aadj;
+ dval(aadj1) = dsign ? aadj : -aadj;
}
dword0(aadj1) += (2*P+1)*Exp_msk1 - y;
}
- adj = aadj1 * ulp(dval(rv));
+ adj = dval(aadj1) * ulp(dval(rv));
dval(rv) += adj;
#else
#ifdef Sudden_Underflow
if ((dword0(rv) & Exp_mask) <= P*Exp_msk1) {
dval(rv0) = dval(rv);
dword0(rv) += P*Exp_msk1;
- adj = aadj1 * ulp(dval(rv));
+ adj = dval(aadj1) * ulp(dval(rv));
dval(rv) += adj;
#ifdef IBM
if ((dword0(rv) & Exp_mask) < P*Exp_msk1)
@@ -1076,7 +1077,7 @@ _DEFUN (_strtod_r, (ptr, s00, se),
dword0(rv) -= P*Exp_msk1;
}
else {
- adj = aadj1 * ulp(dval(rv));
+ adj = dval(aadj1) * ulp(dval(rv));
dval(rv) += adj;
}
#else /*Sudden_Underflow*/
@@ -1088,11 +1089,11 @@ _DEFUN (_strtod_r, (ptr, s00, se),
* example: 1.2e-307 .
*/
if (y <= (P-1)*Exp_msk1 && aadj > 1.) {
- aadj1 = (double)(int)(aadj + 0.5);
+ dval(aadj1) = (double)(int)(aadj + 0.5);
if (!dsign)
- aadj1 = -aadj1;
+ dval(aadj1) = -dval(aadj1);
}
- adj = aadj1 * ulp(dval(rv));
+ adj = dval(aadj1) * ulp(dval(rv));
dval(rv) += adj;
#endif /*Sudden_Underflow*/
#endif /*Avoid_Underflow*/
brgds, H-P