Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ce6f496d720f6206455628425320badd95b07372 breaks float formatting under GTK #16898

Closed
p5pRT opened this issue Mar 20, 2019 · 31 comments
Closed

ce6f496d720f6206455628425320badd95b07372 breaks float formatting under GTK #16898

p5pRT opened this issue Mar 20, 2019 · 31 comments

Comments

@p5pRT
Copy link

p5pRT commented Mar 20, 2019

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

Searchable as RT133945$

@p5pRT
Copy link
Author

p5pRT commented Mar 20, 2019

From @dk

Created by @dk

ce6f496 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
Copy link
Author

p5pRT commented Mar 20, 2019

From @dk

localebug-1.00.tar.gz

@p5pRT
Copy link
Author

p5pRT commented Mar 20, 2019

From @dk

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

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2019

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]

ce6f496 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
Copy link
Author

p5pRT commented Mar 21, 2019

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

@p5pRT
Copy link
Author

p5pRT commented Mar 21, 2019

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]

ce6f496 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
Copy link
Author

p5pRT commented Mar 21, 2019

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
Copy link
Author

p5pRT commented Mar 21, 2019

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
Copy link
Author

p5pRT commented Mar 21, 2019

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
Copy link
Author

p5pRT commented Mar 22, 2019

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​:

<quote>
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.
</quote>

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
Copy link
Author

p5pRT commented Mar 22, 2019

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
Copy link
Author

p5pRT commented Mar 22, 2019

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
Copy link
Author

p5pRT commented Mar 22, 2019

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
Copy link
Author

p5pRT commented Mar 23, 2019

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
Copy link
Author

p5pRT commented Mar 25, 2019

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
Copy link
Author

p5pRT commented Mar 25, 2019

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
Copy link
Author

p5pRT commented Mar 25, 2019

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
Copy link
Author

p5pRT commented Apr 15, 2019

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]

ce6f496 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
Copy link
Author

p5pRT commented Apr 15, 2019

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
Copy link
Author

p5pRT commented Apr 15, 2019

From @khwilliamson

This should be fixed by
commit 4ac6fab
Author​: Karl Williamson <khw@​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
Copy link
Author

p5pRT commented Apr 15, 2019

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

@p5pRT
Copy link
Author

p5pRT commented Apr 16, 2019

From @sisyphus

Unfortunately this latest commit (4ac6fab)
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 4ac6fab
Author​: Karl Williamson <khw@​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
Copy link
Author

p5pRT commented Apr 16, 2019

From @khwilliamson

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

Unfortunately this latest commit
(4ac6fab) 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​:perlbug-followup@​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
Copy link
Author

p5pRT commented Apr 16, 2019

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@​khwilliamson.com>
wrote​:

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

Unfortunately this latest commit
(4ac6fab) 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​:perlbug-followup@​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
Copy link
Author

p5pRT commented Apr 16, 2019

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​:public@​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
Copy link
Author

p5pRT commented Apr 16, 2019

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@​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​:public@​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&#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
Copy link
Author

p5pRT commented Apr 16, 2019

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@​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> 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​:public@​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
Copy link
Author

p5pRT commented Apr 16, 2019

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​:sisyphus359@​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
Copy link
Author

p5pRT commented Apr 16, 2019

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​:sisyphus359@​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​:jkeenan@​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​:public@​khwilliamson.com>
> <mailto​:public@​khwilliamson.com
<mailto​:public@​khwilliamson.com>>> wrote​:
>
>      On 4/15/19 8​:39 PM, sisyphus wrote​:
>       > Unfortunately this latest commit
>       > (4ac6fab) 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​:perlbug-followup@​perl.org>
<mailto​:perlbug-followup@​perl.org
<mailto​:perlbug-followup@​perl.org>>
>      <mailto​:perlbug-followup@​perl.org
<mailto​:perlbug-followup@​perl.org>
>      <mailto​:perlbug-followup@​perl.org
<mailto​:perlbug-followup@​perl.org>>>> wrote​:
>       >
>       >     This should be fixed by
>       >     commit
> 4ac6fab
>       >       Author​: Karl Williamson <khw@​cpan.org
<mailto​:khw@​cpan.org> <mailto​:khw@​cpan.org <mailto​:khw@​cpan.org>>
>      <mailto​:khw@​cpan.org <mailto​:khw@​cpan.org>
<mailto​:khw@​cpan.org <mailto​:khw@​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
Copy link
Author

p5pRT commented May 22, 2019

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
Copy link
Author

p5pRT commented May 22, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant