Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.96k stars 555 forks source link

ce6f496d720f6206455628425320badd95b07372 breaks float formatting under GTK #16898

Closed p5pRT closed 5 years ago

p5pRT commented 5 years ago

Migrated from rt.perl.org#133945 (status was 'resolved')

Searchable as RT133945$

p5pRT commented 5 years ago

From @dk

Created by @dk

ce6f496d720f6206455628425320badd95b07372 changed float/double formatting from strtoflt128 to Perl_strtod\, which had a nasty side effect when running under non-english locales (or I suspect with locales that use comma for decimal point) together with GTK or any other library that calls setlocale() itself.

The test for this is rather elaborate\, so I'm attaching a localebug module that tests for it when run 'make test'. One needs the following (for Ubuntu\, for example)​:

* apt-get install libgtk2.0-dev (gtk development files) * cpan ExtUtils​::PkgConfig * locale-gen de_DE.UTF-8 && update-locale (tests need de_DE.UTF-8)

When running with a faulty configuration\, the core issue is basically this​:

  my $s = sprintf "%s"\, 4.9999;   die "bug​:$s" if $s !~ /^4[\,.]9999$/;

which prints just "4".

The module attached can also be found at http​://karasik.eu.org/misc/localebug-1.00.tar.gz

Perl Info ``` Flags: category=core severity=medium This perlbug was built using Perl 5.29.3 - Wed Mar 20 18:13:09 CET 2019 It is being executed now by Perl 5.29.2 - Wed Mar 20 18:25:02 CET 2019. Site configuration information for perl 5.29.2: Configured by dk at Wed Mar 20 18:25:02 CET 2019. Summary of my perl5 (revision 5 version 29 subversion 1) configuration: Commit id: c7ea9f039c0e7c2333adfcb3b9f1e3f2b25693a1 Platform: osname=linux osvers=4.15.0-45-generic archname=x86_64-linux uname='linux kraken 4.15.0-45-generic #48-ubuntu smp tue jan 29 16:28:13 utc 2019 x86_64 x86_64 x86_64 gnulinux ' config_args='' hint=previous useposix=true d_sigaction=define useithreads=undef usemultiplicity=undef use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n default_inc_excludes_dot=define bincompat5005=undef Compiler: cc='cc' ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' optimize='-O2' cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='' gccversion='7.3.0' gccosandvers='' intsize=4 longsize=8 ptrsize=8 doublesize=8 byteorder=12345678 doublekind=3 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=3 ivtype='long' ivsize=8 nvtype='double' nvsize=8 Off_t='off_t' lseeksize=8 alignbytes=8 prototype=define Linker and Libraries: ld='cc' ldflags =' -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/7/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/7/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/7/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/7/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/7/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/7/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/7/include-fixed /usr/include/x86_64-linux-gnu /usr/lib libs=-lpthread -lnsl -ldb -ldl -lm -lcrypt -lutil -lc perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc libc=libc-2.27.so so=so useshrplib=false libperl=libperl.a gnulibc_version='2.27' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags='-Wl,-E' cccdlflags='-fPIC' lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong' @INC for perl 5.29.2: lib /usr/local/lib/perl5/site_perl/5.29.9/x86_64-linux /usr/local/lib/perl5/site_perl/5.29.9 /usr/local/lib/perl5/5.29.9/x86_64-linux /usr/local/lib/perl5/5.29.9 Environment for perl 5.29.2: HOME=/home/dk LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/dk/perl5/perlbrew/bin:/home/dk/bin:/home/dk/bin:/home/dk/.local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/opt/thinlinc/bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:/usr/local/site/bin:/usr/local/site/sbin:/opt/ibm/db2/V9.7/bin PERLBREW_HOME=/home/dk/.perlbrew PERLBREW_ROOT=/home/dk/perl5/perlbrew PERLBREW_SHELLRC_VERSION=0.84 PERL_BADLANG (unset) SHELL=/usr/bin/zsh ```
p5pRT commented 5 years ago

From @dk

localebug-1.00.tar.gz

p5pRT commented 5 years ago

From @dk

Very similar issue (fixed in 5.20.1) https://rt.perl.org/Public/Bug/Display.html?id=122105

p5pRT commented 5 years ago

From @jkeenan

On Wed\, 20 Mar 2019 18​:03​:23 GMT\, int32 wrote​:

This is a bug report for perl from dmitry@​karasik.eu.org\, generated with the help of perlbug 1.41 running under perl 5.29.2.

----------------------------------------------------------------- [Please describe your issue here]

ce6f496d720f6206455628425320badd95b07372 changed float/double formatting from strtoflt128 to Perl_strtod\, which had a nasty side effect when running under non-english locales (or I suspect with locales that use comma for decimal point) together with GTK or any other library that calls setlocale() itself.

The test for this is rather elaborate\, so I'm attaching a localebug module that tests for it when run 'make test'. One needs the following (for Ubuntu\, for example)​:

* apt-get install libgtk2.0-dev (gtk development files) * cpan ExtUtils​::PkgConfig * locale-gen de_DE.UTF-8 && update-locale (tests need de_DE.UTF-8)

When running with a faulty configuration\, the core issue is basically this​:

my $s = sprintf "%s"\, 4.9999; die "bug​:$s" if $s !~ /^4[\,.]9999$/;

which prints just "4".

The module attached can also be found at http​://karasik.eu.org/misc/localebug-1.00.tar.gz

bcc-ing relevant parties.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 5 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 5 years ago

From @jkeenan

On Wed\, 20 Mar 2019 18​:03​:23 GMT\, int32 wrote​:

This is a bug report for perl from dmitry@​karasik.eu.org\, generated with the help of perlbug 1.41 running under perl 5.29.2.

----------------------------------------------------------------- [Please describe your issue here]

ce6f496d720f6206455628425320badd95b07372 changed float/double formatting from strtoflt128 to Perl_strtod\, which had a nasty side effect when running under non-english locales (or I suspect with locales that use comma for decimal point) together with GTK or any other library that calls setlocale() itself.

The test for this is rather elaborate\, so I'm attaching a localebug module that tests for it when run 'make test'. One needs the following (for Ubuntu\, for example)​:

* apt-get install libgtk2.0-dev (gtk development files) * cpan ExtUtils​::PkgConfig * locale-gen de_DE.UTF-8 && update-locale (tests need de_DE.UTF-8)

When running with a faulty configuration\, the core issue is basically this​:

my $s = sprintf "%s"\, 4.9999; die "bug​:$s" if $s !~ /^4[\,.]9999$/;

which prints just "4".

The module attached can also be found at http​://karasik.eu.org/misc/localebug-1.00.tar.gz

In order to run the tests in your tarball\, I would have to become the root user (or sudo) and change the default locale. I am reluctant to do that and\, in any case\, we have to be able to run the Perl 5 core test suite as non-root users.

Could your tests be modified so that an ordinary user could run them?

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 5 years ago

From @dk

In order to run the tests in your tarball\, I would have to become the root user (or sudo) and change the default locale. I am reluctant to do that and\, in any case\, we have to be able to run the Perl 5 core test suite as non-root users.

That's not entirely correct. You may need to become root to install the prerequisites (gtk headers and ExtUtils​::PkgConfig you can install locally though)\, but locales are OS-specific by design\, and if yours has a way to use one without root\, that's great. However the tests themselves don't need root.

Could your tests be modified so that an ordinary user could run them?

If p5 core has already tests for another locale\, simulating file access required to load proper de_DE.UTF-8 or similar\, please point me to them and I can try to hack on. Otherwise I know about locale-dependend tests as much as the next guy\, unfortunately\, but again\, tests are completely fine with an ordinary user\, provided proper prerequisites are installed.

/dk

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

-- Sincerely\,   Dmitry Karasik

p5pRT commented 5 years ago

From @jkeenan

On Thu\, 21 Mar 2019 14​:28​:37 GMT\, int32 wrote​:

In order to run the tests in your tarball\, I would have to become the root user (or sudo) and change the default locale. I am reluctant to do that and\, in any case\, we have to be able to run the Perl 5 core test suite as non-root users.

That's not entirely correct. You may need to become root to install the prerequisites (gtk headers and ExtUtils​::PkgConfig you can install locally though)\, but locales are OS-specific by design\, and if yours has a way to use one without root\, that's great. However the tests themselves don't need root.

Could your tests be modified so that an ordinary user could run them?

If p5 core has already tests for another locale\, simulating file access required to load proper de_DE.UTF-8 or similar\, please point me to them and I can try to hack on. Otherwise I know about locale-dependend tests as much as the next guy\, unfortunately\, but again\, tests are completely fine with an ordinary user\, provided proper prerequisites are installed.

/dk

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

Is this expected output from running the tests in your tarball (where de_DE.utf8 is available but not default)?

##### $ locale | head -1 LANG=en_US.UTF-8

$ locale -a | grep '^de_DE.utf8' de_DE.utf8

$ prove -I. -vb t/*.t t/C-explicit.t ... 1..1 ok 1 ok t/C-implicit.t ... 1..1 ok 1 ok t/DE-explicit.t .. 1..1 ok 1 ok t/DE-implicit.t .. 1..1 ok 1 ok t/DE.t ........... 1..1 ok 1 - Setting locale failed​: please install de_DE.UTF-8 ok t/none.t ......... 1..1 ok 1 ok All tests successful. Files=6\, Tests=6\, 1 wallclock secs ( 0.04 usr 0.00 sys + 0.12 cusr 0.03 csys = 0.19 CPU) Result​: PASS #####

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 5 years ago

From @dk

Is this expected output from running the tests in your tarball (where de_DE.utf8 is available but not default)?

Hi James\,

Yes indeed it is. That's how it looks when everyting is fine and there's no bug. It's a bit misleading with

ok 1 - Setting locale failed​: please install de_DE.UTF-8

but when run with 'make test' it is suppressed\, and is only shown on failure

/dk

##### $ locale | head -1 LANG=en_US.UTF-8

$ locale -a | grep '^de_DE.utf8' de_DE.utf8

$ prove -I. -vb t/*.t t/C-explicit.t ... 1..1 ok 1 ok t/C-implicit.t ... 1..1 ok 1 ok t/DE-explicit.t .. 1..1 ok 1 ok t/DE-implicit.t .. 1..1 ok 1 ok t/DE.t ........... 1..1 ok 1 - Setting locale failed​: please install de_DE.UTF-8 ok t/none.t ......... 1..1 ok 1 ok All tests successful. Files=6\, Tests=6\, 1 wallclock secs ( 0.04 usr 0.00 sys + 0.12 cusr 0.03 csys = 0.19 CPU) Result​: PASS #####

Hi James\,

-- Sincerely\,   Dmitry Karasik

p5pRT commented 5 years ago

From @sisyphus

Complete locale numbnut here\, who is puzzled at how to account for the behaviour being demonstrated. (Feel free to educate me ... or to ignore me.)

In perllocale docs I see​:

\ Also Perl gives access to various C library functions through the POSIX module. Some of those functions are always affected by the current locale. For example\, "POSIX​::strftime()" uses "LC_TIME"; "POSIX​::strtod()" uses "LC_NUMERIC"; "POSIX​::strcoll()" and "POSIX​::strxfrm()" use "LC_COLLATE". All such functions will behave according to the current underlying locale\, even if that locale isn't exposed to Perl space. \

With the perl that Dmitry has used\, Perl_strtod is simply the C library function strtod(). Is the section I've just quoted stating that strtod (and hence Perl_strtod) "will behave according to the current underlying locale\, even if that locale isn't exposed to Perl space" ?

If so\, is that what's happening here ? Is Gtk2 altering the "current underlying locale" in such a way that C's strtod() is affected\, while perl's atof functionality (which Perl_strtod replaces) would be unaffected ?

FYI​: For a perl whose nvtype is "double" one can revert to using perl's less accurate atof functionality by configuring the build with '-Ud_strtod'. For -Duselongdouble builds\, to avoid Perl_strtod (ie C library's strtold) specify '-Ud_strtod -Ud_strtold'. For -Dusequadmath builds one is currently stuck with Perl_strtod (ie C library's strtoflt128).

Cheers\, Rob

p5pRT commented 5 years ago

From @dk

On Thu\, Mar 21\, 2019 at 08​:16​:52PM -0700\, sisyphus@​cpan.org via RT wrote​:

Complete locale numbnut here\, who is puzzled at how to account for the behaviour being demonstrated. (Feel free to educate me ... or to ignore me.)

Here's a good explanation on the similar issue​:

https://rt-archive.perl.org/perl5/Ticket/Display.html?id=122105

(it's buried somewhere in the discussion).

Basically Perl has the idea that it is master of setlocale()\, while GTK disagrees\, both for good reasons. So IIUC Perl syncs the locale state one in a while\, while I personally think authors integrating those libs should do that\, explicitly -- but that's too late now\, as it will break lots of modules.

Is Gtk2 altering the "current underlying locale" in such a way that C's strtod() is affected\, while perl's atof functionality (which Perl_strtod replaces) would be unaffected ?

It seems that way to me.

-- Sincerely\,   Dmitry Karasik

p5pRT commented 5 years ago

From @khwilliamson

On 3/22/19 4​:31 AM\, Dmitry Karasik wrote​:

On Thu\, Mar 21\, 2019 at 08​:16​:52PM -0700\, sisyphus@​cpan.org via RT wrote​:

Complete locale numbnut here\, who is puzzled at how to account for the behaviour being demonstrated. (Feel free to educate me ... or to ignore me.)

Here's a good explanation on the similar issue​:

https://rt-archive.perl.org/perl5/Ticket/Display.html?id=122105

(it's buried somewhere in the discussion).

You can start way down\, at https://rt-archive.perl.org/perl5/Ticket/Display.html?id=121930#txn-1298035

Basically Perl has the idea that it is master of setlocale()\, while GTK disagrees\, both for good reasons. So IIUC Perl syncs the locale state one in a while\, while I personally think authors integrating those libs should do that\, explicitly -- but that's too late now\, as it will break lots of modules.

Actually\, you have it somewhat wrong. See below

Is Gtk2 altering the "current underlying locale" in such a way that C's strtod() is affected\, while perl's atof functionality (which Perl_strtod replaces) would be unaffected ?

It seems that way to me.

See https://perldoc.pl/5.29.9/perlapi#Locale-related-functions-and-macros

Perl keeps the LC_NUMERIC locale generally set to the C locale. This is for the benefit of all the XS code that has been written over the years that assumes the decimal point character is a dot. It's too late to change that code.

But there are times when the radix character needs to be output in the locale-appropriate form\, which in many European locales is a comma. There are now various macros\, listed at the link above\, that are used in the perl core to switch to that locale for the purposes of output\, and then immediately switch back. Calling the underlying C function directly bypasses this logic.

I haven't looked at this ticket carefully enough to be sure\, but wrapping the strtod() calls with these might be the answer?

And then there's GTK which sets the locale itself. That can't be helped\, but XS code that calls it can call sync_locale() before returning to perl. There is no periodic syncing.

Things are further complicated by the fact that POSIX systems have a completely independent way of setting locales. Perl converted to use that for threaded builds in 5.28\, as that system is thread-safe\, while the older setlocale() isn't. I was expecting to get some bug reports about this versus GTK\, but haven't. (Maybe no one is using threaded perls with that\, or maybe GTK got smarter and uses the new set under threads). XS code that wants to change the locale itself should instead use Perl_setlocale() starting in 5.28\, as that knows which set to use.

The bottom line is you can't use a LC_NUMERIC locale-sensitive library function directly from perl. It has to be wrapped with the macro calls.   I chose not to wrap the functions in the POSIX module\, like POSIX​::strtod() because it hadn't been and I didn't want to break things that actually relied on that. That actually means I had to "unwrap" them\, to switch LC_NUMERIC to the real underlying one before calling the function\, and then switching back.

p5pRT commented 5 years ago

From @dk

On Fri\, Mar 22\, 2019 at 09​:53​:40AM -0600\, Karl Williamson wrote​:

And then there's GTK which sets the locale itself. That can't be helped\, but XS code that calls it can call sync_locale() before returning to perl. There is no periodic syncing.

Thanks for the explanation\, that makes sense. This will help me fixing my affected code; I wish I knew about Perl_sync_locale() before.

-- Sincerely\,   Dmitry Karasik

p5pRT commented 5 years ago

From @khwilliamson

On Fri\, 22 Mar 2019 15​:11​:00 -0700\, int32 wrote​:

On Fri\, Mar 22\, 2019 at 09​:53​:40AM -0600\, Karl Williamson wrote​:

And then there's GTK which sets the locale itself. That can't be helped\, but XS code that calls it can call sync_locale() before returning to perl. There is no periodic syncing.

Thanks for the explanation\, that makes sense. This will help me fixing my affected code; I wish I knew about Perl_sync_locale() before.

Where would you have read that putting a link to perlapi for this would have shown you it existed? -- Karl Williamson

p5pRT commented 5 years ago

From @dk

On Sat\, Mar 23\, 2019 at 11​:26​:22AM -0700\, Karl Williamson via RT wrote​:

On Fri\, 22 Mar 2019 15​:11​:00 -0700\, int32 wrote​:

On Fri\, Mar 22\, 2019 at 09​:53​:40AM -0600\, Karl Williamson wrote​:

And then there's GTK which sets the locale itself. That can't be helped\, but XS code that calls it can call sync_locale() before returning to perl. There is no periodic syncing.

Thanks for the explanation\, that makes sense. This will help me fixing my affected code; I wish I knew about Perl_sync_locale() before.

Where would you have read that putting a link to perlapi for this would have shown you it existed?

"Read" assumes active monitoring of such news\, which I almost never do. Reaching to authors that have GTK-related modules would be nice though; there aren't that many of those. I've got a heads up last time about a similar issue from Slaven Rezic who runs cpantests massively\, and I'm grateful for that.

-- Sincerely\,   Dmitry Karasik

p5pRT commented 5 years ago

From @khwilliamson

On 3/25/19 9​:30 AM\, Dmitry Karasik wrote​:

On Sat\, Mar 23\, 2019 at 11​:26​:22AM -0700\, Karl Williamson via RT wrote​:

On Fri\, 22 Mar 2019 15​:11​:00 -0700\, int32 wrote​:

On Fri\, Mar 22\, 2019 at 09​:53​:40AM -0600\, Karl Williamson wrote​:

And then there's GTK which sets the locale itself. That can't be helped\, but XS code that calls it can call sync_locale() before returning to perl. There is no periodic syncing.

Thanks for the explanation\, that makes sense. This will help me fixing my affected code; I wish I knew about Perl_sync_locale() before.

Where would you have read that putting a link to perlapi for this would have shown you it existed?

"Read" assumes active monitoring of such news\, which I almost never do. Reaching to authors that have GTK-related modules would be nice though; there aren't that many of those. I've got a heads up last time about a similar issue from Slaven Rezic who runs cpantests massively\, and I'm grateful for that.

I meant\, where in the perl docs would you have gone when you noticed the problem. I'm presuming you looked at them\, but didn't find a reference to this function.

p5pRT commented 5 years ago

From @dk

Thanks for the explanation\, that makes sense. This will help me fixing my affected code; I wish I knew about Perl_sync_locale() before.

Where would you have read that putting a link to perlapi for this would have shown you it existed?

"Read" assumes active monitoring of such news\, which I almost never do. Reaching to authors that have GTK-related modules would be nice though; there aren't that many of those. I've got a heads up last time about a similar issue from Slaven Rezic who runs cpantests massively\, and I'm grateful for that. I meant\, where in the perl docs would you have gone when you noticed the problem. I'm presuming you looked at them\, but didn't find a reference to this function.

I didn't go to the docs at all. My timeline was something like this​:

- Slaven sent a pull request for my module (Prima) to workaround perl bug in 5.20.1 that failed tests - Between 5.20.1 and 5.28 everything was fine - I started to see failures again after 5.29.2\, that I could reproduce and fix by the same workaround as did for 5.20.1 - I bisected and submitted a patch

Even if I did go for the docs\, I wouldn't have a slightest idea where to look for\, because I wasn't familiar that locale syncing is a problem at all. I though think that this is okay\, Perl is full of such hidden pieces one simply has to know. By the same token I look into C code generated by XS (because I also generate C/Perl glue code but without XS) to see what have changed\, and these changes I don't expect to see in the docs either.

For the constructive note\, it might be a (probably even good) idea to issue a warning if perl detects discrepancy between real C locale and expected locale\, pointing that authors should read about sync_locale().

-- Sincerely\,   Dmitry Karasik

p5pRT commented 5 years ago

From @khwilliamson

On 3/20/19 12​:03 PM\, Dmitry Karasik (via RT) wrote​:

# New Ticket Created by Dmitry Karasik # Please include the string​: [perl #133945] # in the subject line of all future correspondence about this issue. # \<URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133945 >

This is a bug report for perl from dmitry@​karasik.eu.org\, generated with the help of perlbug 1.41 running under perl 5.29.2.

----------------------------------------------------------------- [Please describe your issue here]

ce6f496d720f6206455628425320badd95b07372 changed float/double formatting from strtoflt128 to Perl_strtod\, which had a nasty side effect when running under non-english locales (or I suspect with locales that use comma for decimal point) together with GTK or any other library that calls setlocale() itself.

The test for this is rather elaborate\, so I'm attaching a localebug module that tests for it when run 'make test'. One needs the following (for Ubuntu\, for example)​:

* apt-get install libgtk2.0-dev (gtk development files) * cpan ExtUtils​::PkgConfig * locale-gen de_DE.UTF-8 && update-locale (tests need de_DE.UTF-8)

When running with a faulty configuration\, the core issue is basically this​:

 my $s = sprintf "%s"\, 4\.9999;
 die "bug&#8203;:$s" if $s \!~ /^4\[\,\.\]9999$/;

which prints just "4".

The module attached can also be found at http​://karasik.eu.org/misc/localebug-1.00.tar.gz

[Please do not change anything below this line]

I tried your (thoughtfully included) test bed\, making sure that perl.h was changed to call straight strtod()\, and couldn't get it to fail.

But I created a branch where I wrap Perl_strtod() with calls to use the proper radix character. Please try it out and report back.

It's at https://perl5.git.perl.org/perl.git/shortlog/refs/heads/smoke-me/khw-strtod

p5pRT commented 5 years ago

From @dk

I tried your (thoughtfully included) test bed\, making sure that perl.h was changed to call straight strtod()\, and couldn't get it to fail.

But I created a branch where I wrap Perl_strtod() with calls to use the proper radix character. Please try it out and report back.

It's at https://perl5.git.perl.org/perl.git/shortlog/refs/heads/smoke-me/khw-strtod

Hi Karl\,

That branch seems to be passing the tests okay

-- Sincerely\,   Dmitry Karasik

p5pRT commented 5 years ago

From @khwilliamson

This should be fixed by commit 4ac6fab20b8950ee14756c6f2438809c572082cd Author​: Karl Williamson \khw@&#8203;cpan\.org Date​: Mon Apr 15 11​:10​:31 2019 -0600

  PATCH​: [perl #133945] Perl_strtod failures  
  This commit wraps Perl_strtod() in macros that cause the proper radix   character to be used.

-- Karl Williamson

p5pRT commented 5 years ago

@khwilliamson - Status changed from 'open' to 'pending release'

p5pRT commented 5 years ago

From @sisyphus

Unfortunately this latest commit (4ac6fab20b8950ee14756c6f2438809c572082cd) gets us back to assigning values incorrectly. (Perl's test suite will not presently detect this.) I guess we've now somehow gone back to using perl's atof() function for assignment of floating point values. At least\, it seems that the values being assigned are now the same as the values that perl's atof() assigns.

A quick check that will catch the problem is to run something like​: $ perl -MPOSIX -le 'print "wtf" if POSIX​::strtod("8.87359152e-6") != 8.87359152e-6;'

When nvtype is 'double\, the value of 8.87359152e-6 is assigned incorrectly by perl's atof on a number of platforms\, including Windows and Linux - so it's a good value to test.

Perhaps we should even add that test to the POSIX test suite ? We'd need to test a different value for -Duselongdouble builds - I'll write a patch for posix.t and submit it later today in a separate bug report. That way\, if Perl_strtod() is defined but perl's atof() is being used\, we'd find out about it when perl's test suite is run.

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 7​:53 AM Karl Williamson via RT \< perlbug-followup@​perl.org> wrote​:

This should be fixed by commit 4ac6fab20b8950ee14756c6f2438809c572082cd Author​: Karl Williamson \khw@&#8203;cpan\.org Date​: Mon Apr 15 11​:10​:31 2019 -0600

 PATCH&#8203;: \[perl \#133945\] Perl\_strtod failures

 This commit wraps Perl\_strtod\(\) in macros that cause the proper radix
 character to be used\.

-- Karl Williamson

--- via perlbug​: queue​: perl5 status​: open https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133945

p5pRT commented 5 years ago

From @khwilliamson

On 4/15/19 8​:39 PM\, sisyphus wrote​:

Unfortunately this latest commit (4ac6fab20b8950ee14756c6f2438809c572082cd) gets us back to assigning values incorrectly. (Perl's test suite will not presently detect this.) I guess we've now somehow gone back to using perl's atof() function for assignment of floating point values. At least\, it seems that the values being assigned are now the same as the values that perl's atof() assigns.

I re-inspected the patch and don't see anyway that it could have made this switcheroo.

A quick check that will catch the problem is to run something like​: $ perl -MPOSIX -le 'print "wtf" if POSIX​::strtod("8.87359152e-6") != 8.87359152e-6;'

When nvtype is 'double\, the value of 8.87359152e-6 is assigned incorrectly by perl's atof on a number of platforms\, including Windows and Linux - so it's a good value to test.

Perhaps we should even add that test to the POSIX test suite ? We'd need to test a different value for -Duselongdouble builds - I'll write a patch for posix.t and submit it later today in a separate bug report.

That sounds good.

That way\, if Perl_strtod() is defined but perl's atof() is being used\, we'd find out about it when perl's test suite is run.

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 7​:53 AM Karl Williamson via RT \<perlbug-followup@​perl.org \mailto&#8203;:perlbug\-followup@&#8203;perl\.org> wrote​:

This should be fixed by
commit 4ac6fab20b8950ee14756c6f2438809c572082cd
  Author&#8203;: Karl Williamson \<khw@&#8203;cpan\.org \<mailto&#8203;:khw@&#8203;cpan\.org>>
  Date&#8203;:   Mon Apr 15 11&#8203;:10&#8203;:31 2019 \-0600

      PATCH&#8203;: \[perl \#133945\] Perl\_strtod failures

      This commit wraps Perl\_strtod\(\) in macros that cause the
proper radix
      character to be used\.

\-\- 
Karl Williamson

\-\-\-
via perlbug&#8203;:  queue&#8203;: perl5 status&#8203;: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133945
p5pRT commented 5 years ago

From @sisyphus

Something in the last couple of days has definitely caused a reversion to use of perl's atof().

This Inline​::C script​: ######################## use warnings; use Inline C => \<\<'EOC'; void foo() { #ifdef Perl_strtod printf("Perl_strtod defined\n"); #else printf("Perl_strtod NOT defined\n"); #endif } EOC foo(); ########################

reports (on all of my Windows and Linux builds) that current blead has Perl_strtod NOT defined. If Perl_strtod is not defined then I don't think that Perl_strtod can be assigning the values - it must surely be perl's atof() that's doing the assigning.

My -Dusequadmath builds are also afflicted with the issue (as they now also do not define Perl_strtod). That's not a reversion - this is the first time ever that quadmath builds have used perl's atof()\, and the result is not pretty.

Anyway - perhaps it's not your commit that's the culprit. It has to be something fairly recent because I built blead a couple of days ago and it was fine. (My builds do check that strtod/strtold/strtoflt128 are being used.) I haven't even yet glanced at the current perl source. I've instead been trying to gauge the scope of the breakage - which is something that took me far longer than it ought.

I'll prepare and post that patch to posix.t tonight\, and try to get my head around the perl source tomorrow.

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 1​:50 PM Karl Williamson \public@&#8203;khwilliamson\.com wrote​:

On 4/15/19 8​:39 PM\, sisyphus wrote​:

Unfortunately this latest commit (4ac6fab20b8950ee14756c6f2438809c572082cd) gets us back to assigning values incorrectly. (Perl's test suite will not presently detect this.) I guess we've now somehow gone back to using perl's atof() function for assignment of floating point values. At least\, it seems that the values being assigned are now the same as the values that perl's atof() assigns.

I re-inspected the patch and don't see anyway that it could have made this switcheroo.

A quick check that will catch the problem is to run something like​: $ perl -MPOSIX -le 'print "wtf" if POSIX​::strtod("8.87359152e-6") != 8.87359152e-6;'

When nvtype is 'double\, the value of 8.87359152e-6 is assigned incorrectly by perl's atof on a number of platforms\, including Windows and Linux - so it's a good value to test.

Perhaps we should even add that test to the POSIX test suite ? We'd need to test a different value for -Duselongdouble builds - I'll write a patch for posix.t and submit it later today in a separate bug report.

That sounds good.

That way\, if Perl_strtod() is defined but perl's atof() is being used\, we'd find out about it when perl's test suite is run.

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 7​:53 AM Karl Williamson via RT \<perlbug-followup@​perl.org \mailto&#8203;:perlbug\-followup@&#8203;perl\.org> wrote​:

This should be fixed by
commit 4ac6fab20b8950ee14756c6f2438809c572082cd
  Author&#8203;: Karl Williamson \<khw@&#8203;cpan\.org \<mailto&#8203;:khw@&#8203;cpan\.org>>
  Date&#8203;:   Mon Apr 15 11&#8203;:10&#8203;:31 2019 \-0600

      PATCH&#8203;: \[perl \#133945\] Perl\_strtod failures

      This commit wraps Perl\_strtod\(\) in macros that cause the
proper radix
      character to be used\.

\-\-
Karl Williamson

\-\-\-
via perlbug&#8203;:  queue&#8203;: perl5 status&#8203;: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133945
p5pRT commented 5 years ago

From @jkeenan

On 4/16/19 4​:53 AM\, sisyphus wrote​:

Something in the last couple of days has definitely caused a reversion to use of perl's atof().

This Inline​::C script​: ######################## use warnings; use Inline C => \<\<'EOC'; void foo() { #ifdef Perl_strtod  printf("Perl_strtod defined\n"); #else  printf("Perl_strtod NOT defined\n"); #endif } EOC foo(); ########################

I can confirm this result on Linux.

Is there any way your C program could be rewritten such that we get a Perl exit 0 on defined and a Perl exit 1 on NOT defined? If we had that\, then we could plug it into Porting/bisect.pl.

reports (on all of my Windows and Linux builds) that current blead has Perl_strtod NOT defined. If Perl_strtod is not defined then I don't think that Perl_strtod can be assigning the values - it must surely be perl's atof() that's doing the assigning.

My -Dusequadmath builds are also afflicted with the issue (as they now also do not define Perl_strtod). That's not a reversion - this is the first time ever that quadmath builds have used perl's atof()\, and the result is not pretty.

Anyway - perhaps it's not your commit that's the culprit. It has to be something fairly recent because I built blead a couple of days ago and it was fine. (My builds do check that strtod/strtold/strtoflt128 are being used.) I haven't even yet glanced at the current perl source. I've instead been trying to gauge the scope of the breakage - which is something that took me far longer than it ought.

I'll prepare and post that patch to posix.t tonight\, and try to get my head around the perl source tomorrow.

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 1​:50 PM Karl Williamson \<public@​khwilliamson.com \mailto&#8203;:public@&#8203;khwilliamson\.com> wrote​:

On 4/15/19 8&#8203;:39 PM\, sisyphus wrote&#8203;:
 > Unfortunately this latest commit
 > \(4ac6fab20b8950ee14756c6f2438809c572082cd\) gets us back to assigning
 > values incorrectly\. \(Perl's test suite will not presently detect
this\.\)
 > I guess we've now somehow gone back to using perl's atof\(\)
function for
 > assignment of floating point values\.
 > At least\, it seems that the values being assigned are now the
same as
 > the values that perl's atof\(\) assigns\.

I re\-inspected the patch and don't see anyway that it could have made
this switcheroo\.
 >
 > A quick check that will catch the problem is to run something like&#8203;:
 > $ perl \-MPOSIX \-le 'print "wtf" if POSIX&#8203;::strtod\("8\.87359152e\-6"\) \!=
 > 8\.87359152e\-6;'
 >
 > When nvtype is 'double\, the value of 8\.87359152e\-6 is assigned
 > incorrectly by perl's atof on a number of platforms\, including
Windows
 > and Linux \- so it's a good value to test\.
 >
 > Perhaps we should even add that test to the POSIX test suite ?
 > We'd need to test a different value for \-Duselongdouble builds \-
I'll
 > write a patch for posix\.t and submit it later today in a separate
bug
 > report\.

That sounds good\.

 > That way\, if Perl\_strtod\(\) is defined but perl's atof\(\) is being
used\,
 > we'd find out about it when perl's test suite is run\.
 >
 > Cheers\,
 > Rob
 >
 > On Tue\, Apr 16\, 2019 at 7&#8203;:53 AM Karl Williamson via RT
 > \<perlbug\-followup@&#8203;perl\.org \<mailto&#8203;:perlbug\-followup@&#8203;perl\.org>
\<mailto&#8203;:perlbug\-followup@&#8203;perl\.org
\<mailto&#8203;:perlbug\-followup@&#8203;perl\.org>>> wrote&#8203;:
 >
 >     This should be fixed by
 >     commit 4ac6fab20b8950ee14756c6f2438809c572082cd
 >       Author&#8203;: Karl Williamson \<khw@&#8203;cpan\.org \<mailto&#8203;:khw@&#8203;cpan\.org>
\<mailto&#8203;:khw@&#8203;cpan\.org \<mailto&#8203;:khw@&#8203;cpan\.org>>>
 >       Date&#8203;:   Mon Apr 15 11&#8203;:10&#8203;:31 2019 \-0600
 >
 >           PATCH&#8203;: \[perl \#133945\] Perl\_strtod failures
 >
 >           This commit wraps Perl\_strtod\(\) in macros that cause the
 >     proper radix
 >           character to be used\.
 >
 >     \-\-
 >     Karl Williamson
 >
 >     \-\-\-
 >     via perlbug&#8203;:  queue&#8203;: perl5 status&#8203;: open
 > https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133945
 >
p5pRT commented 5 years ago

From @sisyphus

James\, Does the following behave as desired ?

##################### use warnings; use Inline C => \<\<'EOC'; int foo() { #ifdef Perl_strtod return 0; #else return 1; #endif } EOC exit(foo()); ####################

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 9​:59 PM James E Keenan \jkeenan@&#8203;pobox\.com wrote​:

On 4/16/19 4​:53 AM\, sisyphus wrote​:

Something in the last couple of days has definitely caused a reversion to use of perl's atof().

This Inline​::C script​: ######################## use warnings; use Inline C => \<\<'EOC'; void foo() { #ifdef Perl_strtod printf("Perl_strtod defined\n"); #else printf("Perl_strtod NOT defined\n"); #endif } EOC foo(); ########################

I can confirm this result on Linux.

Is there any way your C program could be rewritten such that we get a Perl exit 0 on defined and a Perl exit 1 on NOT defined? If we had that\, then we could plug it into Porting/bisect.pl.

reports (on all of my Windows and Linux builds) that current blead has Perl_strtod NOT defined. If Perl_strtod is not defined then I don't think that Perl_strtod can be assigning the values - it must surely be perl's atof() that's doing the assigning.

My -Dusequadmath builds are also afflicted with the issue (as they now also do not define Perl_strtod). That's not a reversion - this is the first time ever that quadmath builds have used perl's atof()\, and the result is not pretty.

Anyway - perhaps it's not your commit that's the culprit. It has to be something fairly recent because I built blead a couple of days ago and it was fine. (My builds do check that strtod/strtold/strtoflt128 are being used.) I haven't even yet glanced at the current perl source. I've instead been trying to gauge the scope of the breakage - which is something that took me far longer than it ought.

I'll prepare and post that patch to posix.t tonight\, and try to get my head around the perl source tomorrow.

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 1​:50 PM Karl Williamson \<public@​khwilliamson.com \mailto&#8203;:public@&#8203;khwilliamson\.com> wrote​:

On 4/15/19 8&#8203;:39 PM\, sisyphus wrote&#8203;:
 > Unfortunately this latest commit
 > \(4ac6fab20b8950ee14756c6f2438809c572082cd\) gets us back to

assigning

values incorrectly. (Perl's test suite will not presently detect this.) I guess we've now somehow gone back to using perl's atof() function for assignment of floating point values. At least\, it seems that the values being assigned are now the same as the values that perl's atof() assigns.

I re\-inspected the patch and don't see anyway that it could have made
this switcheroo\.
 >
 > A quick check that will catch the problem is to run something

like​:

$ perl -MPOSIX -le 'print "wtf" if POSIX​::strtod("8.87359152e-6") != 8.87359152e-6;'

When nvtype is 'double\, the value of 8.87359152e-6 is assigned incorrectly by perl's atof on a number of platforms\, including Windows and Linux - so it's a good value to test.

Perhaps we should even add that test to the POSIX test suite ? We'd need to test a different value for -Duselongdouble builds - I'll write a patch for posix.t and submit it later today in a separate bug report.

That sounds good\.

 > That way\, if Perl\_strtod\(\) is defined but perl's atof\(\) is being
used\,
 > we'd find out about it when perl's test suite is run\.
 >
 > Cheers\,
 > Rob
 >
 > On Tue\, Apr 16\, 2019 at 7&#8203;:53 AM Karl Williamson via RT
 > \<perlbug\-followup@&#8203;perl\.org \<mailto&#8203;:perlbug\-followup@&#8203;perl\.org>
\<mailto&#8203;:perlbug\-followup@&#8203;perl\.org
\<mailto&#8203;:perlbug\-followup@&#8203;perl\.org>>> wrote&#8203;:
 >
 >     This should be fixed by
 >     commit 4ac6fab20b8950ee14756c6f2438809c572082cd
 >       Author&#8203;: Karl Williamson \<khw@&#8203;cpan\.org \<mailto&#8203;:khw@&#8203;cpan\.org>
\<mailto&#8203;:khw@&#8203;cpan\.org \<mailto&#8203;:khw@&#8203;cpan\.org>>>
 >       Date&#8203;:   Mon Apr 15 11&#8203;:10&#8203;:31 2019 \-0600
 >
 >           PATCH&#8203;: \[perl \#133945\] Perl\_strtod failures
 >
 >           This commit wraps Perl\_strtod\(\) in macros that cause the
 >     proper radix
 >           character to be used\.
 >
 >     \-\-
 >     Karl Williamson
 >
 >     \-\-\-
 >     via perlbug&#8203;:  queue&#8203;: perl5 status&#8203;: open
 > https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133945
 >
p5pRT commented 5 years ago

From @sisyphus

I see that line 6555 of perl.h is presently​: #define Perl_strotod(s\, e) \

I wonder if that was meant to be​: #define Perl_strtod(s\, e) \

(I'll test it.)

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 10​:26 PM sisyphus \sisyphus359@&#8203;gmail\.com wrote​:

James\, Does the following behave as desired ?

##################### use warnings; use Inline C => \<\<'EOC'; int foo() { #ifdef Perl_strtod return 0; #else return 1; #endif } EOC exit(foo()); ####################

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 9​:59 PM James E Keenan \jkeenan@&#8203;pobox\.com wrote​:

On 4/16/19 4​:53 AM\, sisyphus wrote​:

Something in the last couple of days has definitely caused a reversion to use of perl's atof().

This Inline​::C script​: ######################## use warnings; use Inline C => \<\<'EOC'; void foo() { #ifdef Perl_strtod printf("Perl_strtod defined\n"); #else printf("Perl_strtod NOT defined\n"); #endif } EOC foo(); ########################

I can confirm this result on Linux.

Is there any way your C program could be rewritten such that we get a Perl exit 0 on defined and a Perl exit 1 on NOT defined? If we had that\, then we could plug it into Porting/bisect.pl.

reports (on all of my Windows and Linux builds) that current blead has Perl_strtod NOT defined. If Perl_strtod is not defined then I don't think that Perl_strtod can be assigning the values - it must surely be perl's atof() that's doing the assigning.

My -Dusequadmath builds are also afflicted with the issue (as they now also do not define Perl_strtod). That's not a reversion - this is the first time ever that quadmath builds have used perl's atof()\, and the result is not pretty.

Anyway - perhaps it's not your commit that's the culprit. It has to be something fairly recent because I built blead a couple of days ago and it was fine. (My builds do check that strtod/strtold/strtoflt128 are being used.) I haven't even yet glanced at the current perl source. I've instead been trying to gauge the scope of the breakage - which is something that took me far longer than it ought.

I'll prepare and post that patch to posix.t tonight\, and try to get my head around the perl source tomorrow.

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 1​:50 PM Karl Williamson \< public@​khwilliamson.com \mailto&#8203;:public@&#8203;khwilliamson\.com> wrote​:

On 4/15/19 8&#8203;:39 PM\, sisyphus wrote&#8203;:
 > Unfortunately this latest commit
 > \(4ac6fab20b8950ee14756c6f2438809c572082cd\) gets us back to

assigning

values incorrectly. (Perl's test suite will not presently detect this.) I guess we've now somehow gone back to using perl's atof() function for assignment of floating point values. At least\, it seems that the values being assigned are now the same as the values that perl's atof() assigns.

I re\-inspected the patch and don't see anyway that it could have

made this switcheroo.

A quick check that will catch the problem is to run something like​: $ perl -MPOSIX -le 'print "wtf" if POSIX​::strtod("8.87359152e-6") != 8.87359152e-6;'

When nvtype is 'double\, the value of 8.87359152e-6 is assigned incorrectly by perl's atof on a number of platforms\, including Windows and Linux - so it's a good value to test.

Perhaps we should even add that test to the POSIX test suite ? We'd need to test a different value for -Duselongdouble builds - I'll write a patch for posix.t and submit it later today in a separate bug report.

That sounds good\.

 > That way\, if Perl\_strtod\(\) is defined but perl's atof\(\) is being
used\,
 > we'd find out about it when perl's test suite is run\.
 >
 > Cheers\,
 > Rob
 >
 > On Tue\, Apr 16\, 2019 at 7&#8203;:53 AM Karl Williamson via RT
 > \<perlbug\-followup@&#8203;perl\.org \<mailto&#8203;:perlbug\-followup@&#8203;perl\.org>
\<mailto&#8203;:perlbug\-followup@&#8203;perl\.org
\<mailto&#8203;:perlbug\-followup@&#8203;perl\.org>>> wrote&#8203;:
 >
 >     This should be fixed by
 >     commit 4ac6fab20b8950ee14756c6f2438809c572082cd
 >       Author&#8203;: Karl Williamson \<khw@&#8203;cpan\.org \<mailto&#8203;:khw@&#8203;cpan\.org

\<mailto&#8203;:khw@&#8203;cpan\.org \<mailto&#8203;:khw@&#8203;cpan\.org>>>
 >       Date&#8203;:   Mon Apr 15 11&#8203;:10&#8203;:31 2019 \-0600
 >
 >           PATCH&#8203;: \[perl \#133945\] Perl\_strtod failures
 >
 >           This commit wraps Perl\_strtod\(\) in macros that cause

the

proper radix
      character to be used\.

\-\-
Karl Williamson

\-\-\-
via perlbug&#8203;:  queue&#8203;: perl5 status&#8203;: open

https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133945

p5pRT commented 5 years ago

From @khwilliamson

On 4/16/19 7​:06 AM\, sisyphus wrote​:

I see that line 6555 of perl.h is presently​: #define Perl_strotod(s\, e) \

I wonder if that was meant to be​: #define Perl_strtod(s\, e) \

(I'll test it.)

Yes\, that's the typo. But fixing it opens a can of worms. Which I believe I have fixed in https://perl5.git.perl.org/perl.git/shortlog/refs/heads/smoke-me/khw-strtod

Please check it out.

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 10​:26 PM sisyphus \<sisyphus359@​gmail.com \mailto&#8203;:sisyphus359@&#8203;gmail\.com> wrote​:

James\,
Does the following behave as desired ?

\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#
use warnings;
use Inline C => \<\<'EOC';
int foo\(\) \{
\#ifdef Perl\_strtod
  return 0;
\#else
  return 1;
\#endif
\}
EOC
exit\(foo\(\)\);
\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#

Cheers\,
Rob

On Tue\, Apr 16\, 2019 at 9&#8203;:59 PM James E Keenan \<jkeenan@&#8203;pobox\.com
\<mailto&#8203;:jkeenan@&#8203;pobox\.com>> wrote&#8203;:

    On 4/16/19 4&#8203;:53 AM\, sisyphus wrote&#8203;:
     > Something in the last couple of days has definitely caused a
    reversion
     > to use of perl's atof\(\)\.
     >
     > This Inline&#8203;::C script&#8203;:
     > \#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#
     > use warnings;
     > use Inline C => \<\<'EOC';
     > void foo\(\) \{
     > \#ifdef Perl\_strtod
     >   printf\("Perl\_strtod defined\\n"\);
     > \#else
     >   printf\("Perl\_strtod NOT defined\\n"\);
     > \#endif
     > \}
     > EOC
     > foo\(\);
     > \#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#\#
     >

    I can confirm this result on Linux\.

    Is there any way your C program could be rewritten such that we
    get a
    Perl exit 0 on defined and a Perl exit 1 on NOT defined?  If we had
    that\, then we could plug it into Porting/bisect\.pl
    \<http&#8203;://bisect\.pl>\.

     > reports \(on all of my Windows and Linux builds\) that current
    blead has
     > Perl\_strtod NOT defined\.
     > If Perl\_strtod is not defined then I don't think that
    Perl\_strtod can be
     > assigning the values \- it must surely be perl's atof\(\) that's
    doing the
     > assigning\.
     >
     > My \-Dusequadmath builds are also afflicted with the issue \(as
    they now
     > also do not define Perl\_strtod\)\. That's not a reversion \-
    this is the
     > first time ever that quadmath builds have used perl's atof\(\)\,
    and the
     > result is not pretty\.
     >
     > Anyway \- perhaps it's not your commit that's the culprit\.
     > It has to be something fairly recent because I built blead a
    couple of
     > days ago and it was fine\. \(My builds do check that
     > strtod/strtold/strtoflt128 are being used\.\)
     > I haven't even yet glanced at the current perl source\. I've
    instead been
     > trying to gauge the scope of the breakage \- which is
    something that took
     > me far longer than it ought\.
     >
     > I'll prepare and post that patch to posix\.t tonight\, and try
    to get my
     > head around the perl source tomorrow\.
     >
     > Cheers\,
     > Rob
     >
     > On Tue\, Apr 16\, 2019 at 1&#8203;:50 PM Karl Williamson
    \<public@&#8203;khwilliamson\.com \<mailto&#8203;:public@&#8203;khwilliamson\.com>
     > \<mailto&#8203;:public@&#8203;khwilliamson\.com
    \<mailto&#8203;:public@&#8203;khwilliamson\.com>>> wrote&#8203;:
     >
     >     On 4/15/19 8&#8203;:39 PM\, sisyphus wrote&#8203;:
     >      > Unfortunately this latest commit
     >      > \(4ac6fab20b8950ee14756c6f2438809c572082cd\) gets us
    back to assigning
     >      > values incorrectly\. \(Perl's test suite will not
    presently detect
     >     this\.\)
     >      > I guess we've now somehow gone back to using perl's atof\(\)
     >     function for
     >      > assignment of floating point values\.
     >      > At least\, it seems that the values being assigned are
    now the
     >     same as
     >      > the values that perl's atof\(\) assigns\.
     >
     >     I re\-inspected the patch and don't see anyway that it
    could have made
     >     this switcheroo\.
     >      >
     >      > A quick check that will catch the problem is to run
    something like&#8203;:
     >      > $ perl \-MPOSIX \-le 'print "wtf" if
    POSIX&#8203;::strtod\("8\.87359152e\-6"\) \!=
     >      > 8\.87359152e\-6;'
     >      >
     >      > When nvtype is 'double\, the value of 8\.87359152e\-6 is
    assigned
     >      > incorrectly by perl's atof on a number of platforms\,
    including
     >     Windows
     >      > and Linux \- so it's a good value to test\.
     >      >
     >      > Perhaps we should even add that test to the POSIX test
    suite ?
     >      > We'd need to test a different value for
    \-Duselongdouble builds \-
     >     I'll
     >      > write a patch for posix\.t and submit it later today in
    a separate
     >     bug
     >      > report\.
     >
     >     That sounds good\.
     >
     >      > That way\, if Perl\_strtod\(\) is defined but perl's
    atof\(\) is being
     >     used\,
     >      > we'd find out about it when perl's test suite is run\.
     >      >
     >      > Cheers\,
     >      > Rob
     >      >
     >      > On Tue\, Apr 16\, 2019 at 7&#8203;:53 AM Karl Williamson via RT
     >      > \<perlbug\-followup@&#8203;perl\.org
    \<mailto&#8203;:perlbug\-followup@&#8203;perl\.org>
    \<mailto&#8203;:perlbug\-followup@&#8203;perl\.org
    \<mailto&#8203;:perlbug\-followup@&#8203;perl\.org>>
     >     \<mailto&#8203;:perlbug\-followup@&#8203;perl\.org
    \<mailto&#8203;:perlbug\-followup@&#8203;perl\.org>
     >     \<mailto&#8203;:perlbug\-followup@&#8203;perl\.org
    \<mailto&#8203;:perlbug\-followup@&#8203;perl\.org>>>> wrote&#8203;:
     >      >
     >      >     This should be fixed by
     >      >     commit 4ac6fab20b8950ee14756c6f2438809c572082cd
     >      >       Author&#8203;: Karl Williamson \<khw@&#8203;cpan\.org
    \<mailto&#8203;:khw@&#8203;cpan\.org> \<mailto&#8203;:khw@&#8203;cpan\.org \<mailto&#8203;:khw@&#8203;cpan\.org>>
     >     \<mailto&#8203;:khw@&#8203;cpan\.org \<mailto&#8203;:khw@&#8203;cpan\.org>
    \<mailto&#8203;:khw@&#8203;cpan\.org \<mailto&#8203;:khw@&#8203;cpan\.org>>>>
     >      >       Date&#8203;:   Mon Apr 15 11&#8203;:10&#8203;:31 2019 \-0600
     >      >
     >      >           PATCH&#8203;: \[perl \#133945\] Perl\_strtod failures
     >      >
     >      >           This commit wraps Perl\_strtod\(\) in macros
    that cause the
     >      >     proper radix
     >      >           character to be used\.
     >      >
     >      >     \-\-
     >      >     Karl Williamson
     >      >
     >      >     \-\-\-
     >      >     via perlbug&#8203;:  queue&#8203;: perl5 status&#8203;: open
     >      > https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133945
     >      >
     >
p5pRT commented 5 years ago

From @jkeenan

On Tue\, 16 Apr 2019 16​:43​:42 GMT\, public@​khwilliamson.com wrote​:

On 4/16/19 7​:06 AM\, sisyphus wrote​:

I see that line 6555 of perl.h is presently​: #define Perl_strotod(s\, e) \

I wonder if that was meant to be​: #define Perl_strtod(s\, e) \

(I'll test it.)

Yes\, that's the typo. But fixing it opens a can of worms. Which I believe I have fixed in https://perl5.git.perl.org/perl.git/shortlog/refs/heads/smoke-me/khw- strtod

Please check it out.

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 10​:26 PM sisyphus \<sisyphus359@​gmail.com \mailto&#8203;:sisyphus359@&#8203;gmail\.com> wrote​:

James\, Does the following behave as desired ?

##################### use warnings; use Inline C => \<\<'EOC'; int foo() { #ifdef Perl_strtod  return 0; #else  return 1; #endif } EOC exit(foo()); ####################

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 9​:59 PM James E Keenan \<jkeenan@​pobox.com \mailto&#8203;:jkeenan@&#8203;pobox\.com> wrote​:

On 4/16/19 4​:53 AM\, sisyphus wrote​:

Something in the last couple of days has definitely caused a reversion to use of perl's atof().

This Inline​::C script​: ######################## use warnings; use Inline C => \<\<'EOC'; void foo() { #ifdef Perl_strtod    printf("Perl_strtod defined\n"); #else    printf("Perl_strtod NOT defined\n"); #endif } EOC foo(); ########################

I can confirm this result on Linux.

Is there any way your C program could be rewritten such that we get a Perl exit 0 on defined and a Perl exit 1 on NOT defined?  If we had that\, then we could plug it into Porting/bisect.pl \<http​://bisect.pl>.

     > reports \(on all of my Windows and Linux builds\) that
     > current

blead has

Perl_strtod NOT defined. If Perl_strtod is not defined then I don't think that Perl_strtod can be assigning the values - it must surely be perl's atof() that's doing the assigning.

My -Dusequadmath builds are also afflicted with the issue (as they now also do not define Perl_strtod). That's not a reversion - this is the first time ever that quadmath builds have used perl's atof()\, and the result is not pretty.

Anyway - perhaps it's not your commit that's the culprit. It has to be something fairly recent because I built blead a couple of days ago and it was fine. (My builds do check that strtod/strtold/strtoflt128 are being used.) I haven't even yet glanced at the current perl source. I've instead been trying to gauge the scope of the breakage - which is something that took me far longer than it ought.

I'll prepare and post that patch to posix.t tonight\, and try to get my head around the perl source tomorrow.

Cheers\, Rob

On Tue\, Apr 16\, 2019 at 1​:50 PM Karl Williamson \<public@​khwilliamson.com \mailto&#8203;:public@&#8203;khwilliamson\.com \<mailto​:public@​khwilliamson.com \mailto&#8203;:public@&#8203;khwilliamson\.com>> wrote​:

     On 4/15/19 8​:39 PM\, sisyphus wrote​:       > Unfortunately this latest commit       > (4ac6fab20b8950ee14756c6f2438809c572082cd) gets us back to assigning       > values incorrectly. (Perl's test suite will not presently detect      this.)       > I guess we've now somehow gone back to using perl's atof()      function for       > assignment of floating point values.       > At least\, it seems that the values being assigned are now the      same as       > the values that perl's atof() assigns.

     I re-inspected the patch and don't see anyway that it could have made      this switcheroo.       >       > A quick check that will catch the problem is to run something like​:       > $ perl -MPOSIX -le 'print "wtf" if POSIX​::strtod("8.87359152e-6") !=       > 8.87359152e-6;'       >       > When nvtype is 'double\, the value of 8.87359152e-6 is assigned       > incorrectly by perl's atof on a number of platforms\, including      Windows       > and Linux - so it's a good value to test.       >       > Perhaps we should even add that test to the POSIX test suite ?       > We'd need to test a different value for -Duselongdouble builds -      I'll       > write a patch for posix.t and submit it later today in a separate      bug       > report.

     That sounds good.

      > That way\, if Perl_strtod() is defined but perl's atof() is being      used\,       > we'd find out about it when perl's test suite is run.       >       > Cheers\,       > Rob       >       > On Tue\, Apr 16\, 2019 at 7​:53 AM Karl Williamson via RT       > \<perlbug-followup@​perl.org \mailto&#8203;:perlbug\-followup@&#8203;perl\.org \<mailto​:perlbug-followup@​perl.org \mailto&#8203;:perlbug\-followup@&#8203;perl\.org>      \<mailto​:perlbug-followup@​perl.org \mailto&#8203;:perlbug\-followup@&#8203;perl\.org      \<mailto​:perlbug-followup@​perl.org \mailto&#8203;:perlbug\-followup@&#8203;perl\.org>>> wrote​:       >       >     This should be fixed by       >     commit 4ac6fab20b8950ee14756c6f2438809c572082cd       >       Author​: Karl Williamson \<khw@​cpan.org \mailto&#8203;:khw@&#8203;cpan\.org \<mailto​:khw@​cpan.org \mailto&#8203;:khw@&#8203;cpan\.org>      \<mailto​:khw@​cpan.org \mailto&#8203;:khw@&#8203;cpan\.org \<mailto​:khw@​cpan.org \mailto&#8203;:khw@&#8203;cpan\.org>>>       >       Date​:   Mon Apr 15 11​:10​:31 2019 -0600       >       >           PATCH​: [perl #133945] Perl_strtod failures       >       >           This commit wraps Perl_strtod() in macros that cause the       >     proper radix       >           character to be used.       >       >     --       >     Karl Williamson       >       >     ---       >     via perlbug​:  queue​: perl5 status​: open       > https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133945       >

With a perl built in the smoke-me/khw_strtod branch\, I run Sisyphus's program and get​:

##### [khw-strtod] 506 $ ./bin/perl -Ilib ~/learn/perl/p5p/133945-strtod.pl Perl_strtod NOT defined #####

Is that the expected result?

Thank you very much. -- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 5 years ago

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release today of Perl 5.30.0\, this and 160 other issues have been resolved.

Perl 5.30.0 may be downloaded via​: https://metacpan.org/release/XSAWYERX/perl-5.30.0

If you find that the problem persists\, feel free to reopen this ticket.

p5pRT commented 5 years ago

@khwilliamson - Status changed from 'pending release' to 'resolved'