Test errno setting on strtod overflow in tst-strtod-round
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Build passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Test passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Test passed
|
Commit Message
We have no tests that errno is set to ERANGE on overflow of
strtod-family functions (we do have some tests for underflow, in
tst-strtod-underflow). Add such tests to tst-strtod-round.
Tested for x86_64.
Comments
* Joseph Myers:
> We have no tests that errno is set to ERANGE on overflow of
> strtod-family functions (we do have some tests for underflow, in
> tst-strtod-underflow). Add such tests to tst-strtod-round.
>
> Tested for x86_64.
>
> diff --git a/stdlib/tst-strtod-round-skeleton.c b/stdlib/tst-strtod-round-skeleton.c
> index 6fba4b5228..184ec07b67 100644
> --- a/stdlib/tst-strtod-round-skeleton.c
> +++ b/stdlib/tst-strtod-round-skeleton.c
> @@ -21,6 +21,7 @@
> declared in the headers. */
> #define _LIBC_TEST 1
> #define __STDC_WANT_IEC_60559_TYPES_EXT__
> +#include <errno.h>
> #include <fenv.h>
> #include <float.h>
> #include <math.h>
> @@ -205,7 +206,9 @@ struct test {
> #define GEN_ONE_TEST(FSUF, FTYPE, FTOSTR, LSUF, CSUF) \
> { \
> feclearexcept (FE_ALL_EXCEPT); \
> + errno = 0; \
> FTYPE f = STRTO (FSUF) (s, NULL); \
> + int new_errno = errno; \
> if (f != expected->FSUF \
> || (copysign ## CSUF) (1.0 ## LSUF, f) \
> != (copysign ## CSUF) (1.0 ## LSUF, expected->FSUF)) \
> @@ -254,6 +257,13 @@ struct test {
> printf ("ignoring this exception error\n"); \
> } \
> } \
> + if (overflow->FSUF && new_errno != ERANGE) \
> + { \
> + printf (FNPFXS "to" #FSUF \
> + " (" STRM ") did not set errno to ERANGE\n", \
> + s); \
> + result = 1; \
> + } \
> } \
> }
Maybe print new_errno in the error message? Rest looks okay.
Thanks,
Florian
On Tue, Aug 13, 2024, at 1:52 PM, Joseph Myers wrote:
> We have no tests that errno is set to ERANGE on overflow of
> strtod-family functions (we do have some tests for underflow, in
> tst-strtod-underflow). Add such tests to tst-strtod-round.
Maybe we should also have tests that errno is left unchanged by
these functions if no overflow or underflow occurred? (I don't
know if these already exist.)
zw
On Wed, 14 Aug 2024, Zack Weinberg wrote:
> On Tue, Aug 13, 2024, at 1:52 PM, Joseph Myers wrote:
> > We have no tests that errno is set to ERANGE on overflow of
> > strtod-family functions (we do have some tests for underflow, in
> > tst-strtod-underflow). Add such tests to tst-strtod-round.
>
> Maybe we should also have tests that errno is left unchanged by
> these functions if no overflow or underflow occurred? (I don't
> know if these already exist.)
The difficulty there is the details of "no underflow". We do have
tst-strtod-underflow that checks this for various edge cases of underflow
(for double, I intend to extend it to cover other types as well). We also
have bug 32045 which notes we wrongly set errno for overflow in parsing a
NaN payload.
On Wed, Aug 14, 2024, at 1:51 PM, Joseph Myers wrote:
> On Wed, 14 Aug 2024, Zack Weinberg wrote:
>> On Tue, Aug 13, 2024, at 1:52 PM, Joseph Myers wrote:
>> > We have no tests that errno is set to ERANGE on overflow of strtod-
>> > family functions (we do have some tests for underflow, in tst-strtod-
>> > underflow). Add such tests to tst-strtod-round.
>>
>> Maybe we should also have tests that errno is left unchanged by these
>> functions if no overflow or underflow occurred? (I don't know if
>> these already exist.)
>
> The difficulty there is the details of "no underflow". We do have tst-strtod-
> underflow that checks this for various edge cases of underflow (for
> double, I intend to extend it to cover other types as well). We also
> have bug 32045 which notes we wrongly set errno for overflow in
> parsing a NaN payload.
I think the most valuable thing would be to check for unchanged errno
after parsing each of some uniformly distributed random sample of normal
numbers. Further, exploring the space of valid input strings is probably
more useful than exploring the space of normal-number return values, if
that makes sense?
zw
On Wed, 14 Aug 2024, Zack Weinberg wrote:
> >> Maybe we should also have tests that errno is left unchanged by these
> >> functions if no overflow or underflow occurred? (I don't know if
> >> these already exist.)
> >
> > The difficulty there is the details of "no underflow". We do have tst-strtod-
> > underflow that checks this for various edge cases of underflow (for
> > double, I intend to extend it to cover other types as well). We also
> > have bug 32045 which notes we wrongly set errno for overflow in
> > parsing a NaN payload.
>
> I think the most valuable thing would be to check for unchanged errno
> after parsing each of some uniformly distributed random sample of normal
> numbers. Further, exploring the space of valid input strings is probably
> more useful than exploring the space of normal-number return values, if
> that makes sense?
Most importantly we might want to verify that a successful call to strtod
does not reset errno to zero (that is where set to nonzero beforehand), as
required by ISO C and POSIX.
Maciej
On Wed, Aug 14, 2024, at 9:48 PM, Maciej W. Rozycki wrote:
> On Wed, 14 Aug 2024, Zack Weinberg wrote:
>> >> Maybe we should also have tests that errno is left unchanged by
>> >> these functions if no overflow or underflow occurred? (I don't
>> >> know if these already exist.)
>> >
>> > The difficulty there is the details of "no underflow". We do have
>> > tst-strtod- underflow that checks this for various edge cases of
>> > underflow (for double, I intend to extend it to cover other types
>> > as well). We also have bug 32045 which notes we wrongly set errno
>> > for overflow in parsing a NaN payload.
>>
>> I think the most valuable thing would be to check for unchanged errno
>> after parsing each of some uniformly distributed random sample of
>> normal numbers. Further, exploring the space of valid input strings
>> is probably more useful than exploring the space of normal-number
>> return values, if that makes sense?
>
> Most importantly we might want to verify that a successful call to
> strtod does not reset errno to zero (that is where set to nonzero
> beforehand), as required by ISO C and POSIX.
That's a requirement for all C library functions - and one that we
probably should be testing, in general - but for the strto* family the
requirement is stronger. As you probably already know, most C library
functions are allowed to set errno to an arbitrary nonzero value *even
if* they succeed. But strto* must not alter errno at all when they
succeed, because some error conditions for strto* are *only* reported by
setting errno to a nonzero value.
Thus the most important cases to test are the success cases that collide
with the value strto* returns on error: that is, *successful* parsing of
LONG_MAX for strtol, ULONG_MAX for strtoul, HUGE_VAL for strtod, etc.
zw
On Thu, 15 Aug 2024, Zack Weinberg wrote:
> >> I think the most valuable thing would be to check for unchanged errno
> >> after parsing each of some uniformly distributed random sample of
> >> normal numbers. Further, exploring the space of valid input strings
> >> is probably more useful than exploring the space of normal-number
> >> return values, if that makes sense?
> >
> > Most importantly we might want to verify that a successful call to
> > strtod does not reset errno to zero (that is where set to nonzero
> > beforehand), as required by ISO C and POSIX.
>
> That's a requirement for all C library functions - and one that we
> probably should be testing, in general - but for the strto* family the
> requirement is stronger. As you probably already know, most C library
> functions are allowed to set errno to an arbitrary nonzero value *even
> if* they succeed. But strto* must not alter errno at all when they
> succeed, because some error conditions for strto* are *only* reported by
> setting errno to a nonzero value.
That is true, but given that strto* functions return no error status
(other than setting errno) it might be tempting for an implementer to
reset errno to zero on success.
> Thus the most important cases to test are the success cases that collide
> with the value strto* returns on error: that is, *successful* parsing of
> LONG_MAX for strtol, ULONG_MAX for strtoul, HUGE_VAL for strtod, etc.
Agreed. This includes zero as well, which is also returned where no
conversion could be performed (and for which POSIX permits errno to be set
to EINVAL).
Maciej
@@ -21,6 +21,7 @@
declared in the headers. */
#define _LIBC_TEST 1
#define __STDC_WANT_IEC_60559_TYPES_EXT__
+#include <errno.h>
#include <fenv.h>
#include <float.h>
#include <math.h>
@@ -205,7 +206,9 @@ struct test {
#define GEN_ONE_TEST(FSUF, FTYPE, FTOSTR, LSUF, CSUF) \
{ \
feclearexcept (FE_ALL_EXCEPT); \
+ errno = 0; \
FTYPE f = STRTO (FSUF) (s, NULL); \
+ int new_errno = errno; \
if (f != expected->FSUF \
|| (copysign ## CSUF) (1.0 ## LSUF, f) \
!= (copysign ## CSUF) (1.0 ## LSUF, expected->FSUF)) \
@@ -254,6 +257,13 @@ struct test {
printf ("ignoring this exception error\n"); \
} \
} \
+ if (overflow->FSUF && new_errno != ERANGE) \
+ { \
+ printf (FNPFXS "to" #FSUF \
+ " (" STRM ") did not set errno to ERANGE\n", \
+ s); \
+ result = 1; \
+ } \
} \
}