The DVAL_TO_LVAL macro is quite weird, I'm not sure exactly what it's
supposed to be doing but it probably isn't doing it. If the integral
part of d is outside the range of a long, the conversion has undefined
behaviour by the C99 standard; an explicit cast makes no difference
AFAICT.
GCC on IA64 does wierd things with this macro, though I think there's a
GCC bug involved there too. This fixes the macro to have well-defined
behaviour for all values of 'd', and avoids triggering the GCC bug on
IA64 to boot (both PHP users on that platform will be happy):
Index: Zend/zend_operators.c
RCS file: /repository/ZendEngine2/zend_operators.c,v
retrieving revision 1.194
diff -u -r1.194 zend_operators.c
--- Zend/zend_operators.c 19 Jul 2004 07:19:02 -0000 1.194
+++ Zend/zend_operators.c 27 Aug 2004 12:15:12 -0000
@@ -183,7 +183,15 @@
}
-#define DVAL_TO_LVAL(d, l) (l) = (d) > LONG_MAX ? (unsigned long) (d) : (long) (d)
+#define DVAL_TO_LVAL(d, l) do { \
- if ((d) > LONG_MAX) { \
-
l = LONG_MAX; \
- } else if ((d) < LONG_MIN) { \
-
l = LONG_MIN; \
- } else { \
-
l = (d); \
- }
+} while (0)
#define zendi_convert_to_long(op, holder, result)
if (op==result) { \
Hi Joe,
It seems like your patch doesn't really fix anything. How is rounding to
LONG_MAX/LONG_MIN any better?
Maybe you can explain in more detail what this gcc bug you are hitting is?
Thanks,
Andi
At 01:25 PM 8/27/2004 +0100, Joe Orton wrote:
The DVAL_TO_LVAL macro is quite weird, I'm not sure exactly what it's
supposed to be doing but it probably isn't doing it. If the integral
part of d is outside the range of a long, the conversion has undefined
behaviour by the C99 standard; an explicit cast makes no difference
AFAICT.GCC on IA64 does wierd things with this macro, though I think there's a
GCC bug involved there too. This fixes the macro to have well-defined
behaviour for all values of 'd', and avoids triggering the GCC bug on
IA64 to boot (both PHP users on that platform will be happy):Index: Zend/zend_operators.c
RCS file: /repository/ZendEngine2/zend_operators.c,v
retrieving revision 1.194
diff -u -r1.194 zend_operators.c
--- Zend/zend_operators.c 19 Jul 2004 07:19:02 -0000 1.194
+++ Zend/zend_operators.c 27 Aug 2004 12:15:12 -0000
@@ -183,7 +183,15 @@
}-#define DVAL_TO_LVAL(d, l) (l) = (d) > LONG_MAX ? (unsigned long) (d) :
(long) (d)
+#define DVAL_TO_LVAL(d, l) do { \
if ((d) > LONG_MAX) { \
l = LONG_MAX; \
} else if ((d) < LONG_MIN) { \
l = LONG_MIN; \
} else { \
l = (d); \
} \
+} while (0)
#define zendi_convert_to_long(op, holder,
result)
if (op==result)
{
\
Hi Joe,
It seems like your patch doesn't really fix anything. How is rounding to
LONG_MAX/LONG_MIN any better?
The C standard says that when converting a double to a long, if the
integral part of the double is outside the range which can be
represented by a long, the conversion has undefined behaviour.
#define DVAL_TO_LVAL(d, l) (l) = (d) > LONG_MAX ? (unsigned long) (d) : (long) (d)
where d is a double and l is a long: so this has undefined behaviour for
values of d greater than ULONG_MAX or smaller than LONG_MIN.
#define DVAL_TO_LVAL(d, l) do {
if ((d) > LONG_MAX) {
l = LONG_MAX;
} else if ((d) < LONG_MIN) {
l = LONG_MIN;
} else {
l = (d);
}
} while (0)
has well-defined behaviour for all values of 'd', since it will never
attempt to assign a value to 'l' which cannot be represented by a long.
Maybe you can explain in more detail what this gcc bug you are hitting is?
It seems to be an IA64-specific optimisation bug, it's really incidental
to the fact that the current code is dubious. GCC evaluates the
expression as either 0 or LONG_MIN if d is negative, depending on -O
level. (Which is fairly nasty)
joe
Joe Orton wrote:
The DVAL_TO_LVAL macro is quite weird, I'm not sure exactly what it's
supposed to be doing but it probably isn't doing it. If the integral
part of d is outside the range of a long, the conversion has undefined
behaviour by the C99 standard; an explicit cast makes no difference
AFAICT.GCC on IA64 does wierd things with this macro, though I think there's a
GCC bug involved there too. This fixes the macro to have well-defined
behaviour for all values of 'd', and avoids triggering the GCC bug on
IA64 to boot (both PHP users on that platform will be happy):
This probably has to do with the fact that on 64-bit systems, doubles
lack the accuracy to distinguish LONG_MAX from LONG_MAX +1. To be on the
safe side here, you might want to use >= LONG_MAX instead of > LONG_MAX,
or cast the other way around.
--
Ard
Joe Orton wrote:
The DVAL_TO_LVAL macro is quite weird, I'm not sure exactly what it's
supposed to be doing but it probably isn't doing it. If the integral
part of d is outside the range of a long, the conversion has undefined
behaviour by the C99 standard; an explicit cast makes no difference
AFAICT.GCC on IA64 does wierd things with this macro, though I think there's a
GCC bug involved there too. This fixes the macro to have well-defined
behaviour for all values of 'd', and avoids triggering the GCC bug on
IA64 to boot (both PHP users on that platform will be happy):
This probably has to do with the fact that on 64-bit systems, doubles
lack the accuracy to distinguish LONG_MAX from LONG_MAX +1. To be on the
safe side here, you might want to use >= LONG_MAX instead of > LONG_MAX,
or cast the other way around.
--
Ard
Joe Orton wrote:
GCC on IA64 does wierd things with this macro, though I think there's a
GCC bug involved there too. This fixes the macro to have well-defined
behaviour for all values of 'd', and avoids triggering the GCC bug on
IA64 to boot (both PHP users on that platform will be happy):This probably has to do with the fact that on 64-bit systems, doubles
lack the accuracy to distinguish LONG_MAX from LONG_MAX +1. To be on the
safe side here, you might want to use >= LONG_MAX instead of > LONG_MAX,
or cast the other way around.
It actually was a GCC bug, in fact, one of our GCC developers tracked it
down: http://gcc.gnu.org/ml/gcc-patches/2004-08/msg02654.html
joe
Also <= LONG_MIN needed?
I guess yes?
At 03:22 PM 9/11/2004 +0200, Ard Biesheuvel wrote:
Joe Orton wrote:
The DVAL_TO_LVAL macro is quite weird, I'm not sure exactly what it's
supposed to be doing but it probably isn't doing it. If the integral
part of d is outside the range of a long, the conversion has undefined
behaviour by the C99 standard; an explicit cast makes no difference
AFAICT.
GCC on IA64 does wierd things with this macro, though I think there's a
GCC bug involved there too. This fixes the macro to have well-defined
behaviour for all values of 'd', and avoids triggering the GCC bug on
IA64 to boot (both PHP users on that platform will be happy):This probably has to do with the fact that on 64-bit systems, doubles lack
the accuracy to distinguish LONG_MAX from LONG_MAX +1. To be on the safe
side here, you might want to use >= LONG_MAX instead of > LONG_MAX, or
cast the other way around.--
Ard