[perl #125770] Unsafe comparison of floats in SvTRUE

[James E Keenan via RT]

Hi,
Using SvTRUE() triggers a gcc warning (when building INN code source).

I am unfamiliar with 'INN code source'. Could you clarify?

Thank you very much.

[James E Keenan via RT]

Hi,
Using SvTRUE() triggers a gcc warning (when building INN code source).
In file included from /usr/lib/x86_64-linux-
gnu/perl/5.20/CORE/perl.h:3335:0,
comparing floating point with == or != is unsafe [-Werror=float-equal]
|| (SvNOK(sv) && SvNVX(sv) != 0.0)) \
^
/usr/lib/x86_64-linux-gnu/perl/5.20/CORE/sv.h:1762:85: note: in
expansion of macro ‘SvTRUE_common’
#define SvTRUE(sv) (LIKELY(sv) && (UNLIKELY(SvGMAGICAL(sv)) ?
sv_2bool(sv) : SvTRUE_common(sv, sv_2bool_nomg(sv))))
^
perl.c:117:9: note: in expansion of macro ‘SvTRUE’
if (SvTRUE(ERRSV)) {
^
Could the check in sv.h be changed or, if it can't be improved, could
a
#pragma GCC diagnostic ignored "-Wfloat-equal"
directive be added aroung it?
Thanks beforehand,

I suspect that people who try to reproduce this warning may have trouble doing so given the large number of configuration arguments you are using.

With that large number of config args, it becomes difficult to say which one might be implicated in the warning.

For example, using:

#####
-des -Dusedevel -Dusethreads -Duselargefiles
#####

to configure and build Perl 5 blead on linux/x86_64, I was unable to reproduce that warning. (See attached for the warnings I did get.)

Would it be possible for you to re-configure and re-build, starting with as few config args as possible, adding one at a time until you get the warning?

Thank you very much.

[Eric Brine via RT]

Post by James E Keenan via RT
I suspect that people who try to reproduce this warning may have
trouble doing so given the large number of configuration arguments you
are using.

Seems to me -Werror=float-equal should be enough to replicate the problem.

Floating point numbers are often slightly different than expected due to rounding of periodic numbers (such as 1/10, 2/10, 3/10, 4/10, 6/10, 7/10, 8/10 and 9/10). As such, one should usually check if two floating pointer numbers are equal by checking if their difference falls within a tolerance. This isn't done here, so a warning is issued.


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

[Julien ÉLIE (via RT)]

# New Ticket Created by Julien ÉLIE
# Please include the string: [perl #125770]
# in the subject line of all future correspondence about this issue.
# <URL: https://rt.perl.org/Ticket/Display.html?id=125770 >


Hi,

Using SvTRUE() triggers a gcc warning (when building INN code source).

In file included from /usr/lib/x86_64-linux-gnu/perl/5.20/CORE/perl.h:3335:0,
from perl.c:42:
perl.c: In function ‘PLartfilter’:
/usr/lib/x86_64-linux-gnu/perl/5.20/CORE/sv.h:1773:32: error: comparing floating point with == or != is unsafe [-Werror=float-equal]
|| (SvNOK(sv) && SvNVX(sv) != 0.0)) \
^
/usr/lib/x86_64-linux-gnu/perl/5.20/CORE/sv.h:1762:85: note: in expansion of macro ‘SvTRUE_common’
#define SvTRUE(sv) (LIKELY(sv) && (UNLIKELY(SvGMAGICAL(sv)) ? sv_2bool(sv) : SvTRUE_common(sv, sv_2bool_nomg(sv))))

^
perl.c:117:9: note: in expansion of macro ‘SvTRUE’
if (SvTRUE(ERRSV)) {
^



Could the check in sv.h be changed or, if it can't be improved, could a
#pragma GCC diagnostic ignored "-Wfloat-equal"
directive be added aroung it?

Thanks beforehand,
--
Julien ÉLIE



-----------------------------------------------------------------
---
Flags:
category=core
severity=low
---
Site configuration information for perl 5.20.2:

Configured by Debian Project at Sun May 3 16:16:25 UTC 2015.

Summary of my perl5 (revision 5 version 20 subversion 2) configuration:

Platform:
osname=linux, osvers=3.2.0-4-amd64, archname=x86_64-linux-gnu-thread-multi
uname='linux x86-csail-01 3.2.0-4-amd64 #1 smp debian 3.2.68-1+deb7u1 x86_64 gnulinux '
config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Dldflags= -Wl,-z,relro -Dlddlflags=-shared -Wl,-z,relro -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.20 -Darchlib=/usr/lib/x86_64-linux-gnu/perl/5.20 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/x86_64-linux-gnu/perl5/5.20 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.20.2 -Dsitearch=/usr/local/lib/x86_64-linux-gnu/perl/5.20.2 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Duse64bitint -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -Ui_libutil -Uversiononly -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.20.2 -des'
hint=recommended, useposix=true, d_sigaction=define
useithreads=define, usemultiplicity=define
use64bitint=define, use64bitall=define, uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
optimize='-O2 -g',
cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include'
ccversion='', gccversion='4.9.2', gccosandvers=''
intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
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 -L/usr/local/lib'
libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.9/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
libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
perllibs=-ldl -lm -lpthread -lc -lcrypt
libc=libc-2.19.so, so=so, useshrplib=true, libperl=libperl.so.5.20
gnulibc_version='2.19'
Dynamic Linking:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib -fstack-protector'

Locally applied patches:
DEBPKG:debian/cpan_definstalldirs - Provide a sensible INSTALLDIRS default for modules installed from CPAN.
DEBPKG:debian/db_file_ver - http://bugs.debian.org/340047 Remove overly restrictive DB_File version check.
DEBPKG:debian/doc_info - Replace generic man(1) instructions with Debian-specific information.
DEBPKG:debian/enc2xs_inc - http://bugs.debian.org/290336 Tweak enc2xs to follow symlinks and ignore missing @INC directories.
DEBPKG:debian/errno_ver - http://bugs.debian.org/343351 Remove Errno version check due to upgrade problems with long-running processes.
DEBPKG:debian/libperl_embed_doc - http://bugs.debian.org/186778 Note that libperl-dev package is required for embedded linking
DEBPKG:fixes/respect_umask - Respect umask during installation
DEBPKG:debian/writable_site_dirs - Set umask approproately for site install directories
DEBPKG:debian/extutils_set_libperl_path - EU:MM: set location of libperl.a under /usr/lib
DEBPKG:debian/no_packlist_perllocal - Don't install .packlist or perllocal.pod for perl or vendor
DEBPKG:debian/prefix_changes - Fiddle with *PREFIX and variables written to the makefile
DEBPKG:debian/fakeroot - Postpone LD_LIBRARY_PATH evaluation to the binary targets.
DEBPKG:debian/instmodsh_doc - Debian policy doesn't install .packlist files for core or vendor.
DEBPKG:debian/ld_run_path - Remove standard libs from LD_RUN_PATH as per Debian policy.
DEBPKG:debian/libnet_config_path - Set location of libnet.cfg to /etc/perl/Net as /usr may not be writable.
DEBPKG:debian/mod_paths - Tweak @INC ordering for Debian
DEBPKG:debian/module_build_man_extensions - http://bugs.debian.org/479460 Adjust Module::Build manual page extensions for the Debian Perl policy
DEBPKG:debian/prune_libs - http://bugs.debian.org/128355 Prune the list of libraries wanted to what we actually need.
DEBPKG:fixes/net_smtp_docs - [rt.cpan.org #36038] http://bugs.debian.org/100195 Document the Net::SMTP 'Port' option
DEBPKG:debian/perlivp - http://bugs.debian.org/510895 Make perlivp skip include directories in /usr/local
DEBPKG:debian/deprecate-with-apt - http://bugs.debian.org/747628 Point users to Debian packages of deprecated core modules
DEBPKG:debian/squelch-locale-warnings - http://bugs.debian.org/508764 Squelch locale warnings in Debian package maintainer scripts
DEBPKG:debian/skip-upstream-git-tests - Skip tests specific to the upstream Git repository
DEBPKG:debian/patchlevel - http://bugs.debian.org/567489 List packaged patches for 5.20.2-3+deb8u1 in patchlevel.h
DEBPKG:debian/skip-kfreebsd-crash - http://bugs.debian.org/628493 [perl #96272] Skip a crashing test case in t/op/threads.t on GNU/kFreeBSD
DEBPKG:fixes/document_makemaker_ccflags - http://bugs.debian.org/628522 [rt.cpan.org #68613] Document that CCFLAGS should include $Config{ccflags}
DEBPKG:debian/find_html2text - http://bugs.debian.org/640479 Configure CPAN::Distribution with correct name of html2text
DEBPKG:debian/perl5db-x-terminal-emulator.patch - http://bugs.debian.org/668490 Invoke x-terminal-emulator rather than xterm in perl5db.pl
DEBPKG:debian/cpan-missing-site-dirs - http://bugs.debian.org/688842 Fix CPAN::FirstTime defaults with nonexisting site dirs if a parent is writable
DEBPKG:fixes/memoize_storable_nstore - [rt.cpan.org #77790] http://bugs.debian.org/587650 Memoize::Storable: respect 'nstore' option not respected
DEBPKG:debian/regen-skip - Skip a regeneration check in unrelated git repositories
DEBPKG:fixes/regcomp-mips-optim - [perl #122817] http://bugs.debian.org/754054 Downgrade the optimization of regcomp.c on mips and mipsel due to a gcc-4.9 bug
DEBPKG:debian/makemaker-pasthru - http://bugs.debian.org/758471 Pass LD settings through to subdirectories
DEBPKG:fixes/perldoc-less-R - [rt.cpan.org #98636] http://bugs.debian.org/758689 Tell the 'less' pager to allow terminal escape sequences
DEBPKG:fixes/pod_man_reproducible_date - http://bugs.debian.org/759405 Support POD_MAN_DATE in Pod::Man for the left-hand footer
DEBPKG:fixes/io_uncompress_gunzip_inmemory - http://bugs.debian.org/747363 [rt.cpan.org #95494] Fix gunzip to in-memory file handle
DEBPKG:fixes/socket_test_recv_fix - http://bugs.debian.org/758718 [perl #122657] Compare recv return value to peername in socket test
DEBPKG:fixes/hurd_socket_recv_todo - http://bugs.debian.org/758718 [perl #122657] TODO checking the result of recv() on hurd
DEBPKG:fixes/regexp-performance - [0fa70a0] http://bugs.debian.org/777556 [perl #123743] simpify and speed up /.*.../ handling
DEBPKG:fixes/failed_require_diagnostics - http://bugs.debian.org/781120 [perl #123270] Report inaccesible file on failed require
DEBPKG:fixes/array-cloning - http://bugs.debian.org/779357 [perl #124127] [902d169] fix cloning arrays with unused elements
DEBPKG:fixes/perldb-threads - http://bugs.debian.org/779357 [perl #124127] [41ef2c6] lib/perl5db.pl: Restore noop lock prototype

---
@INC for perl 5.20.2:
/etc/perl
/usr/local/lib/x86_64-linux-gnu/perl/5.20.2
/usr/local/share/perl/5.20.2
/usr/lib/x86_64-linux-gnu/perl5/5.20
/usr/share/perl5
/usr/lib/x86_64-linux-gnu/perl/5.20
/usr/share/perl/5.20
/usr/local/lib/site_perl
.

---
Environment for perl 5.20.2:
HOME=/home/news
LANG=fr_FR.utf8
LANGUAGE=fr_FR:fr
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/usr/local/bin:/usr/local/sbin:/bin:/usr/bin:/usr/sbin:/sbin:/home/news/bin
PERL_BADLANG (unset)
SHELL=/bin/zsh

[Julien ÉLIE]

Hi Eric,

Post by Eric Brine via RT

Post by James E Keenan via RT
I suspect that people who try to reproduce this warning may have
trouble doing so given the large number of configuration arguments
you are using.

Seems to me -Werror=float-equal should be enough to replicate the
problem.

Yes, exactly.

Post by Eric Brine via RT
Floating point numbers are often slightly different than expected due
to rounding of periodic numbers (such as 1/10, 2/10, 3/10, 4/10,
6/10, 7/10, 8/10 and 9/10). As such, one should usually check if two
floating pointer numbers are equal by checking if their difference
falls within a tolerance. This isn't done here, so a warning is
issued.

SvTRUE currently has "SvNVX(sv) != 0.0" whereas something like
"fabs(SvNVX(sv)) < epsilon" should be tested, where epsilon could be
0.00001, or a more complicated test but maybe it is not worthwhile here.

More information here:

http://www.cygnus-software.com/papers/comparingfloats/comparingfloats.htm

https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/

[Julien ÉLIE]

Hi James,

Post by James E Keenan via RT

Using SvTRUE() triggers a gcc warning (when building INN code source).

I am unfamiliar with 'INN code source'. Could you clarify?

I should have been more precise, you're right.
I was speaking of the news server named INN (project main pages are
<https://www.isc.org/downloads/projects/> and
<http://www.eyrie.org/~eagle/software/inn/>).

The related code is at line 117 of:
https://inn.eyrie.org/trac/browser/trunk/innd/perl.c
"if (SvTRUE(ERRSV)) {"

[bulk88 via RT]

Post by Julien ÉLIE
https://inn.eyrie.org/trac/browser/trunk/innd/perl.c
"if (SvTRUE(ERRSV)) {"

That is bad Perl XS code. ERRSV macro has a getter C function internally, SvTRUE macro evaluates its arguments multiple times, leading to exponential calls of the getter function. Assign ERRSV to a C auto SV* before passing that SV * to SvTRUE macro.

[Julien ÉLIE]

Hi bulk88,

Post by bulk88 via RT

Post by Julien ÉLIE
https://inn.eyrie.org/trac/browser/trunk/innd/perl.c
"if (SvTRUE(ERRSV)) {"

That is bad Perl XS code. ERRSV macro has a getter C function
internally, SvTRUE macro evaluates its arguments multiple times,
leading to exponential calls of the getter function. Assign ERRSV to
a C auto SV* before passing that SV * to SvTRUE macro.

Thanks for the suggestion, I will fix it.

Could perlcall documentation also be fixed because SvTRUE(ERRSV) is an
example given in it:
http://perldoc.perl.org/perlcall.html

/* Check the eval first */
if (SvTRUE(ERRSV))
{
printf ("Uh oh - %s\n", SvPV_nolen(ERRSV));
POPs;
}



Would the following example be the right one to follow?

SV *errsv = NULL;

/* Check the eval first */
errsv = ERRSV;
if (SvTRUE(errsv))
{
printf ("Uh oh - %s\n", SvPV_nolen(ERRSV));
POPs;
}

[bulk88 via RT]

Post by Julien ÉLIE
Hi bulk88,

Post by bulk88 via RT

Post by Julien ÉLIE
https://inn.eyrie.org/trac/browser/trunk/innd/perl.c
"if (SvTRUE(ERRSV)) {"

That is bad Perl XS code. ERRSV macro has a getter C function
internally, SvTRUE macro evaluates its arguments multiple times,
leading to exponential calls of the getter function. Assign ERRSV to
a C auto SV* before passing that SV * to SvTRUE macro.

Thanks for the suggestion, I will fix it.
Could perlcall documentation also be fixed because SvTRUE(ERRSV) is an
http://perldoc.perl.org/perlcall.html

perlcall needs to be fixed. Upto 10-15 years ago ERRSV had no function calls inside, since 10-15 years ago when a change was made ERRSV now has a getter style function call inside.

Post by Julien ÉLIE
/* Check the eval first */
if (SvTRUE(ERRSV))
{
printf ("Uh oh - %s\n", SvPV_nolen(ERRSV));

SvPV_nolen(ERRSV) this is also bad, SvPV_nolen macro evaluates its argument multiple times.

Post by Julien ÉLIE
POPs;
}
Would the following example be the right one to follow?
SV *errsv = NULL;

^^^^^^^^^^^NULL initializer is redundant, ERRSV will always return something/evaluate to something

Post by Julien ÉLIE
/* Check the eval first */
errsv = ERRSV;
if (SvTRUE(errsv))

^^^^^^^^^^this is fine/correct

Post by Julien ÉLIE
{
printf ("Uh oh - %s\n", SvPV_nolen(ERRSV));

^^^^^^^^^^^^this needs to be fixed and split like SvTRUE and ERRSV was above

Post by Julien ÉLIE
POPs;
}

[Julien ÉLIE]

Hi bulk88,

Post by Julien ÉLIE
Could perlcall documentation also be fixed because SvTRUE(ERRSV) is
an example given in it: http://perldoc.perl.org/perlcall.html

perlcall needs to be fixed. Upto 10-15 years ago ERRSV had no
function calls inside, since 10-15 years ago when a change was made
ERRSV now has a getter style function call inside.

Same thing for INN. We even still have around a ppport.h version 1.0003
from 1999:
https://inn.eyrie.org/trac/browser/trunk/include/ppport.h

We defined only what we were actually using. I should have a look to
see whether it needs updating to new stuff present in the latest
ppport.h upstream version.

Post by Julien ÉLIE
Would the following example be the right one to follow?
errsv = ERRSV;
if (SvTRUE(errsv))

^^^^^^^^^^this is fine/correct

OK, thanks.
Patch applied to INN code source:
https://inn.eyrie.org/trac/changeset/9930

Post by Julien ÉLIE
{ printf ("Uh oh - %s\n", SvPV_nolen(ERRSV));

^^^^^^^^^^^^this needs to be fixed and split like SvTRUE and ERRSV

Regarding the update of perlcall, what is the process so that it is not
forgotten? Is the current ticket enough or should I open another one?

[James E Keenan via RT]

Post by Eric Brine via RT

Post by James E Keenan via RT
I suspect that people who try to reproduce this warning may have
trouble doing so given the large number of configuration arguments
you
are using.

Seems to me -Werror=float-equal should be enough to replicate the
problem.

Thank you for that suggestion. Here is the problem reproduced in Perl 5 blead with:

#####
sh ./Configure -des -Dusedevel -Dccflags=-Werror=float-equal
#####

See attached file (slight editing for clarity), where 'make' died very quickly.

[James E Keenan via RT]

Additional attachment: ack the core source code for potentially problematic code.

[Tony Cook via RT]

Post by Julien ÉLIE (via RT)
perl.c:117:9: note: in expansion of macro ‘SvTRUE’
if (SvTRUE(ERRSV)) {
^
Could the check in sv.h be changed or, if it can't be improved, could
a
#pragma GCC diagnostic ignored "-Wfloat-equal"
directive be added aroung it?

The floating point check done by the macro needs to remain as it is - the macro implements perl's definition of "truthiness" for values, changing it to fabs(SvNVx(sv)) < 0.00001 would change what perl considers true, and be wrong in other ways.[1]

Unfortunately using the diagnostic pragma on the macro is impractical.

Putting the diagnostic pragmas around the macro definition (which might have worked if GCC was being smart about dealing with where the code comes from) doesn't work.

So to suppress the warning we'd need to add the pragma around every single use of the SvTRUE() and SvTRUE_NN() macro, which isn't practical.

The only alternative I can see is to make SvTRUE_common() into an inline function, and add the pragmas around that instead, but we'd need to add probes to Configure to check that the pragma is available and guard the pragma with whatever macro we define to indicate the pragma is available.

I'm not sure it's worthwhile when you could just build without that option.

Tony

[1] using an epsilon of 0.00001 assumes we're working with values around 1.0, which isn't necessarily the case.

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

[Julien ÉLIE]

Hi Tony,

Post by Tony Cook via RT

Could the check in sv.h be changed or, if it can't be improved,
could a #pragma GCC diagnostic ignored "-Wfloat-equal" directive be
added aroung it?

The floating point check done by the macro needs to remain as it is -
the macro implements perl's definition of "truthiness" for values,
changing it to fabs(SvNVx(sv)) < 0.00001 would change what perl
considers true, and be wrong in other ways.[1]

Thanks for your answer.
My fault for "fabs(SvNVx(sv)) < 0.00001" that should instead have been
"fabs(SvNVx(sv)) > 0.00001" here.

Post by Tony Cook via RT
[1] using an epsilon of 0.00001 assumes we're working with values
around 1.0, which isn't necessarily the case.

Then FLT_MIN (defined in float.h) could be used.
fabs(SvNVx(sv)) > FLT_MIN

Post by Tony Cook via RT
The only alternative I can see is to make SvTRUE_common() into an
inline function, and add the pragmas around that instead, but we'd
need to add probes to Configure to check that the pragma is available
and guard the pragma with whatever macro we define to indicate the
pragma is available.
I'm not sure it's worthwhile when you could just build without that
option.

Well, the problem is that the current code is not totally right because
of the check against 0.0. James also pointed out other parts of the
code where a similar issue exists.
Maybe it could be worthwhile to have a isfloatequals(a,b) function that
could be used at all these places.

[Zefram]

Post by Julien ÉLIE
Then FLT_MIN (defined in float.h) could be used.
fabs(SvNVx(sv)) > FLT_MIN

Where a fuzzy comparison is desired, that's not the way to do it.
The amount of fuzz allowed has to be based on the magnitude of the
numbers that were subtracted to get this near-zero value, which isn't
available at this point. Ideally also on the error budget of the
preceding computations. SvTRUE() is way too late to apply fuzzing.
do_ncmp() would be slightly less too late, because it sees values that
may both have a positive magnitude. But...

Post by Julien ÉLIE
Well, the problem is that the current code is not totally right
because of the check against 0.0.

The code is totally right in this respect. It is implementing the
semantics of the Perl programming language, and those semantics call
for exact floating point comparison, not fuzzy comparison. There are
plenty of ways in which exact comparison can be used correctly. If you
want fuzzy comparison, the place to implement it is in your Perl program.

The float-equal warning is counterproductive when applied to the
perl source. Turning it into an error changes the C language beyond
the bounds of perl's portability. It would not be worth the effort to
implement the circumlocution that would be required for us to get the
required exact floating point comparison without triggering this warning.

-zefram

[bulk88 via RT]

Post by Tony Cook via RT

Post by Julien ÉLIE (via RT)
perl.c:117:9: note: in expansion of macro ‘SvTRUE’
if (SvTRUE(ERRSV)) {
^
Could the check in sv.h be changed or, if it can't be improved, could
a
#pragma GCC diagnostic ignored "-Wfloat-equal"
directive be added aroung it?

The floating point check done by the macro needs to remain as it is -
the macro implements perl's definition of "truthiness" for values,
changing it to fabs(SvNVx(sv)) < 0.00001 would change what perl
considers true, and be wrong in other ways.[1]

I'll add on, Perl assumes 0.0 is a double with all 0 bits on most (all?) platforms http://perl5.git.perl.org/perl.git/blob/6147b0088faa521d6b4bc8bb97279ad46f5365a0:/config_h.SH#l4981 . X87 instruction set also clearly points out what 0.0 is with its FLDZ "load zero" instruction http://docs.oracle.com/cd/E19455-01/806-3773/instructionset-183/index.html . +0.0 is all 0 bits.

There is a certain movie about working in an office space and retiring rich off of micropennies. Perl isn't going to change to allow that to happen. Perl's FP math results are always iffy

-float, double or long double or quad (not x86/not intel)
-do you do intermediate calculations at 80 bits (x87) or 64 bits (SSE) or 32 bits (float, special CC flag)?

All those permutations can cause the same PP code with same disk file inputs, to produce different bitpatterns/different precision results on different perl builds.

Post by Tony Cook via RT
Unfortunately using the diagnostic pragma on the macro is impractical.

I agree. Not every warning is good, not every option of GCC is sane. How about PDP-11 TSK executable with x86 machine code? Perl is a VM, not a C program. Diagnostics usually need to happen on VM level at runtime/interp time, not C compiler level. GCC has an option called -fstrict-aliasing on by default at -O2. This makes casting pointers illegal/impossible/crashing. -fstrict-aliasing has been explictly disabled for 15 years in Perl, since the minute it came out. That disabling isnt going to change, since Perl isn't going to agree to a change in the definition of the C language. Same way, I dont think perl is going to change the meaning of FP 0 just because the C compiler devs decided to change it for right or wrong.

Also how would you pick 0.00001 vs 0.000001 as the cut off rule for all Perl code in existence today?

[Julien ÉLIE]

Hi bulk88,

Post by bulk88 via RT
GCC has an option called
-fstrict-aliasing on by default at -O2. This makes casting pointers
illegal/impossible/crashing. -fstrict-aliasing has been explictly
disabled for 15 years in Perl, since the minute it came out. That
disabling isnt going to change, since Perl isn't going to agree to a
change in the definition of the C language.

Well, that's a choice.
As far as INN is concerned, enforcing strict aliasing rules permitted to
fix how sockaddr related stuff was delt with. Note that we also enforce
-Werror=strict-aliasing (with -Wall in fact).
https://inn.eyrie.org/trac/changeset/9863
https://inn.eyrie.org/trac/changeset/8586

[bulk88 via RT]

Post by Julien ÉLIE
Hi bulk88,

Post by Julien ÉLIE
Could perlcall documentation also be fixed because SvTRUE(ERRSV) is
an example given in it: http://perldoc.perl.org/perlcall.html

perlcall needs to be fixed. Upto 10-15 years ago ERRSV had no
function calls inside, since 10-15 years ago when a change was made
ERRSV now has a getter style function call inside.

Same thing for INN. We even still have around a ppport.h version 1.0003
https://inn.eyrie.org/trac/browser/trunk/include/ppport.h
We defined only what we were actually using. I should have a look to
see whether it needs updating to new stuff present in the latest
ppport.h upstream version.

In my personal opinion, don't upgrade ppport.h if you dont know why, it keeps the repo smaller. If you know why you want to upgrade, such as you want a backport of a newer API, then upgrade ppport.h. Newer and future perls come with all the compat macros/layers to run older XS source code. There was a time during 5.5 and 5.6 era where some macros wound up only existing in ppport.h and were removed from core headers, but that hasn't happened since 5.6 era.

Post by Julien ÉLIE

Post by Julien ÉLIE
{ printf ("Uh oh - %s\n", SvPV_nolen(ERRSV));

^^^^^^^^^^^^this needs to be fixed and split like SvTRUE and ERRSV

Regarding the update of perlcall, what is the process so that it is not
forgotten? Is the current ticket enough or should I open another one?

There is a ticket for it already https://rt.perl.org/Ticket/Display.html?id=120903