[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: GNU poke 3.90.1 on musl libc
|
From: |
Jose E. Marchesi |
|
Subject: |
Re: GNU poke 3.90.1 on musl libc |
|
Date: |
Mon, 22 Jan 2024 22:43:48 +0100 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Hi Bruno.
> On Alpine Linux 3.19/x86_64, which uses musl libc, there are test failures:
>
> Making a new site.exp file ...
> WARNING: Couldn't find the global config file.
> Using /home/bruno/poke-3.90.1.1/testsuite/lib/poke.exp as tool init file.
> Test run by bruno on Mon Jan 22 17:33:34 2024
> Native configuration is x86_64-pc-linux-musl
>
> === poke tests ===
>
> Schedule of variations:
> unix
>
> Running target unix
> Using /usr/share/dejagnu/baseboards/unix.exp as board description file for
> target.
> Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
> Using /home/bruno/poke-3.90.1.1/testsuite/config/unix.exp as
> tool-and-target-specific interface file.
> Running /home/bruno/poke-3.90.1.1/testsuite/poke.cmd/cmd.exp ...
> Running /home/bruno/poke-3.90.1.1/testsuite/poke.libpoke/libpoke.exp ...
> Running /home/bruno/poke-3.90.1.1/testsuite/poke.map/map.exp ...
> Running /home/bruno/poke-3.90.1.1/testsuite/poke.pickles/pickles.exp ...
> Running /home/bruno/poke-3.90.1.1/testsuite/poke.pkl/pkl.exp ...
> Running /home/bruno/poke-3.90.1.1/testsuite/poke.pktest/pktest.exp ...
> Running /home/bruno/poke-3.90.1.1/testsuite/poke.pokefmt/pokefmt.exp ...
> Running /home/bruno/poke-3.90.1.1/testsuite/poke.pvm/pvm.exp ...
> FAIL: poke.pvm/pvm-insns-test.pk: 31 stof: assertion failure at
> /home/bruno/poke-3.90.1.1/testsuite/poke.pvm/pvm-insns-test.pk:379:9: asm
> int<32>: ("stof; nn; nip" : "")
> FAIL: poke.pvm/pvm-insns-test.pk: 32 stod: assertion failure at
> /home/bruno/poke-3.90.1.1/testsuite/poke.pvm/pvm-insns-test.pk:401:9: asm
> int<32>: ("stod; nn; nip" : "")
> FAIL: ./../poke/poke: non-zero exit code: 24811 exp6 0 1
> Running /home/bruno/poke-3.90.1.1/testsuite/poke.repl/repl.exp ...
> Running /home/bruno/poke-3.90.1.1/testsuite/poke.std/std.exp ...
> FAIL: poke.std/std-test.pk: 21 string to floating-point conversion: assertion
> failure at /home/bruno/poke-3.90.1.1/testsuite/poke.std/std-test.pk:540:9:
> !(stof ("") ?! E_conv)
> FAIL: ./../poke/poke: non-zero exit code: 25164 exp32 0 1
> Running /home/bruno/poke-3.90.1.1/testsuite/poke.time/time.exp ...
>
> === poke Summary ===
>
> # of expected passes 7973
> # of unexpected failures 5
> # of expected failures 3
> # of unsupported tests 7
> make[4]: *** [Makefile:5896: check-DEJAGNU] Error 1
>
>
> Find attached the testsuite/poke.sum file.
>
>
> The problems appear to be with the 'stof' and 'stod' instructions. A look into
> common/pk-utils.c tells me that that are implemented through the functions
> 'strtof' and 'strtod', respectively.
>
> $ nm --dynamic libpoke/.libs/libpoke.so.1.0.0 | grep strto
> U strtod
> U strtof
> U strtok
> U strtol
> U strtoll
> U strtoull
> shows that these functions are taken from libc.
>
> More precisely, the problem is with the empty string passed as argument.
> POSIX:2018
> <https://pubs.opengroup.org/onlinepubs/9699919799/functions/strtod.html>
> says that
> "If no conversion could be performed, 0 shall be returned, and errno may be
> set to [EINVAL]."
> And that's precisely where the portability problem is. This program
> ------------------------------------------------------------------------
> #include <stdlib.h>
> #include <stdio.h>
> #include <errno.h>
> int main ()
> {
> const char *arg = "";
> char *endptr;
> errno = 0;
> double result = strtod (arg, &endptr);
> printf ("result = %g, endptr = arg+%d, errno = %d\n", result,
> (int)(endptr-arg), errno);
> }
> ------------------------------------------------------------------------
> prints
> result = 0, endptr = arg+0, errno = 0
> on glibc systems but
> result = 0, endptr = arg+0, errno = 22
> on musl libc systems.
> The Gnulib module 'strtod' does not fix this portability problem
> <https://www.gnu.org/software/gnulib/manual/html_node/strtod.html>.
> So, the fix needs to be in poke/common/pk-utils.c.
>
> Now, should the empty string be treated as valid or invalid, for the
> 'stof' and 'stod' instructions?
>
> IMO, it makes no sense to treat "" as valid and " " or "+" as invalid.
> Only strings that contain a number should be considered as valid here.
>
> Therefore, find attached a proposed patch. With it, "make check" passes
> both on glibc and musl libc systems (Ubuntu and Alpine Linux).
I agree with that solution, and just applied your patch to both
maint/poke-4 and master.
Thanks!
> Bruno
>
>
>>From 2352a1f437fd1aa0e464d7953cb0ead90933378f Mon Sep 17 00:00:00 2001
> From: Bruno Haible <bruno@clisp.org>
> Date: Mon, 22 Jan 2024 20:49:46 +0100
> Subject: [PATCH] stof, stod: Treat the empty string argument as invalid.
>
> * common/pk-utils.c (pvm_stof, pvm_stod): If nothing gets converted, return
> true.
> * testsuite/poke.pvm/pvm-insns-test.pk: Verify that the empty string argument
> is invalid for stof and stod.
> * testsuite/poke.std/std-test.pk: Likewise.
> ---
> common/pk-utils.c | 8 ++++----
> testsuite/poke.pvm/pvm-insns-test.pk | 4 ++--
> testsuite/poke.std/std-test.pk | 4 ++--
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/common/pk-utils.c b/common/pk-utils.c
> index 4f2b54c8..d4cd7b18 100644
> --- a/common/pk-utils.c
> +++ b/common/pk-utils.c
> @@ -206,8 +206,8 @@ int pvm_stof (const char *str, float *f)
> assert (f);
> errno = 0;
> *f = strtof (str, &end);
> - /* No ERANGE and it should consume the whole string. */
> - return errno != 0 || *end != '\0';
> + /* No ERANGE and it should do a conversion and consume the whole string.
> */
> + return errno != 0 || end == str || *end != '\0';
> }
>
> int pvm_stod (const char *str, double *d)
> @@ -218,8 +218,8 @@ int pvm_stod (const char *str, double *d)
> assert (d);
> errno = 0;
> *d = strtod (str, &end);
> - /* No ERANGE and it should consume the whole string. */
> - return errno != 0 || *end != '\0';
> + /* No ERANGE and it should do a conversion and consume the whole string.
> */
> + return errno != 0 || end == str || *end != '\0';
> }
>
> void
> diff --git a/testsuite/poke.pvm/pvm-insns-test.pk
> b/testsuite/poke.pvm/pvm-insns-test.pk
> index d61da143..1c87c668 100644
> --- a/testsuite/poke.pvm/pvm-insns-test.pk
> +++ b/testsuite/poke.pvm/pvm-insns-test.pk
> @@ -373,10 +373,10 @@ var tests = [
> assert (asm int<32>: ("stof; nnn; nip" : " +"));
> assert (asm int<32>: ("stof; nnn; nip" : " +1 "));
> assert (asm int<32>: ("stof; nnn; nip" : "a"));
> + assert (asm int<32>: ("stof; nnn; nip" : ""));
>
> /* OK cases. */
> assert (asm int<32>: ("stof; nn; nip" : " +1"));
> - assert (asm int<32>: ("stof; nn; nip" : ""));
> assert (asm int<32>: ("stof; nn; nip" : "-1E9"));
> }
> },
> @@ -395,10 +395,10 @@ var tests = [
> assert (asm int<32>: ("stod; nnn; nip" : " +"));
> assert (asm int<32>: ("stod; nnn; nip" : " +1 "));
> assert (asm int<32>: ("stod; nnn; nip" : "a"));
> + assert (asm int<32>: ("stod; nnn; nip" : ""));
>
> /* OK cases. */
> assert (asm int<32>: ("stod; nn; nip" : " +1"));
> - assert (asm int<32>: ("stod; nn; nip" : ""));
> assert (asm int<32>: ("stod; nn; nip" : "-1E9"));
> }
> },
> diff --git a/testsuite/poke.std/std-test.pk b/testsuite/poke.std/std-test.pk
> index 22b91f44..a76a19a1 100644
> --- a/testsuite/poke.std/std-test.pk
> +++ b/testsuite/poke.std/std-test.pk
> @@ -537,16 +537,16 @@ var tests = [
> assert (pi4 == 0x400921cac083126fUL);
>
> /* Valid conversions. */
> - assert (!(stof ("") ?! E_conv));
> assert (!(stof (" 3") ?! E_conv));
> - assert (!(stod ("") ?! E_conv));
> assert (!(stod (" 3") ?! E_conv));
> assert (!(stod ("+1e9") ?! E_conv));
>
> /* Invalid conversions. */
> + assert (stof ("") ?! E_conv);
> assert (stof (" ") ?! E_conv);
> assert (stof ("3 ") ?! E_conv);
> assert (stof ("+ 3") ?! E_conv);
> + assert (stod ("") ?! E_conv);
> assert (stod (" ") ?! E_conv);
> assert (stod ("3 ") ?! E_conv);
> assert (stod ("+ 3") ?! E_conv);