Don't assume NV_INF / NV_NAN #22882

jbglaw · 2025-01-01T19:30:07Z

The pkgsrc repository (a large tree of build instructions for lots of software, used as a package source for NetBSD and many other unix-like distributions) recently updated Perl5 from 5.38 to 5.40, which now no longer builts for VAX targets:

gcc -c -DPERL_CORE -O2 -pthread -I/usr/include -fwrapv -fno-strict-aliasing -pipe -std=c99 -O2 -pthread -I/usr/include -Wall -Werror=pointer-arith -Werror=vla -Wextra -Wno-long-long -Wno-decl
aration-after-statement -Wc++-compat -Wwrite-strings -DPIC -fPIC builtin.c
In file included from perl.h:6225,
                 from builtin.c:16:
builtin.c: In function 'XS_builtin_inf':  
builtin.c:100:17: error: 'NV_INF' undeclared (first use in this function); did you mean 'NV_MIN'?
  100 |     XSRETURN_NV(NV_INF);
      |                 ^~~~~~
embed.h:649:71: note: in definition of macro 'sv_2mortal'
  649 | # define sv_2mortal(a)                          Perl_sv_2mortal(aTHX_ a)
embed.h:649:71: note: in definition of macro 'sv_2mortal'
  649 | # define sv_2mortal(a)                          Perl_sv_2mortal(aTHX_ a)
      |                                                                       ^
XSUB.h:317:43: note: in expansion of macro 'newSVnv'
  317 | #define XST_mNV(i,v)  (ST(i) = sv_2mortal(newSVnv(v))  )
      |                                           ^~~~~~~
XSUB.h:334:40: note: in expansion of macro 'XST_mNV'
  334 | #define XSRETURN_NV(v)    STMT_START { XST_mNV(0,v);    XSRETURN(1); } STMT_END
      |                                        ^~~~~~~
builtin.c:100:5: note: in expansion of macro 'XSRETURN_NV'
  100 |     XSRETURN_NV(NV_INF);
      |     ^~~~~~~~~~~  
builtin.c:100:17: note: each undeclared identifier is reported only once for each function it appears in
  100 |     XSRETURN_NV(NV_INF);
      |                 ^~~~~~
embed.h:649:71: note: in definition of macro 'sv_2mortal'
  649 | # define sv_2mortal(a)                          Perl_sv_2mortal(aTHX_ a)
      |                                                                       ^
XSUB.h:317:43: note: in expansion of macro 'newSVnv'
  317 | #define XST_mNV(i,v)  (ST(i) = sv_2mortal(newSVnv(v))  )
      |                                           ^~~~~~~
XSUB.h:334:40: note: in expansion of macro 'XST_mNV'
  334 | #define XSRETURN_NV(v)    STMT_START { XST_mNV(0,v);    XSRETURN(1); } STMT_END
      |                                        ^~~~~~~
builtin.c:100:5: note: in expansion of macro 'XSRETURN_NV'
  100 |     XSRETURN_NV(NV_INF);
      |     ^~~~~~~~~~~
builtin.c: In function 'XS_builtin_nan':
builtin.c:110:17: error: 'NV_NAN' undeclared (first use in this function); did you mean 'FP_NAN'?
  110 |     XSRETURN_NV(NV_NAN);
      |                 ^~~~~~
embed.h:649:71: note: in definition of macro 'sv_2mortal'
  649 | # define sv_2mortal(a)                          Perl_sv_2mortal(aTHX_ a)
      |                                                                       ^ 
XSUB.h:317:43: note: in expansion of macro 'newSVnv'
  317 | #define XST_mNV(i,v)  (ST(i) = sv_2mortal(newSVnv(v))  )
      |                                           ^~~~~~~
XSUB.h:334:40: note: in expansion of macro 'XST_mNV'
  334 | #define XSRETURN_NV(v)    STMT_START { XST_mNV(0,v);    XSRETURN(1); } STMT_END
      |                                        ^~~~~~~ 
builtin.c:110:5: note: in expansion of macro 'XSRETURN_NV' 
  110 |     XSRETURN_NV(NV_NAN);
      |     ^~~~~~~~~~~ 
builtin.c: In function 'ck_builtin_const':
builtin.c:138:54: error: 'NV_INF' undeclared (first use in this function); did you mean 'NV_MIN'?
  138 |         case BUILTIN_CONST_INF:   constval = newSVnv(NV_INF); break;
      |                                                      ^~~~~~
embed.h:437:68: note: in definition of macro 'newSVnv'
  437 | # define newSVnv(a)                             Perl_newSVnv(aTHX_ a)
      |                                                                    ^
builtin.c:139:54: error: 'NV_NAN' undeclared (first use in this function); did you mean 'FP_NAN'?
      |                                                                    ^
builtin.c:139:54: error: 'NV_NAN' undeclared (first use in this function); did you mean 'FP_NAN'?
  139 |         case BUILTIN_CONST_NAN:   constval = newSVnv(NV_NAN); break;  
      |                                                      ^~~~~~
embed.h:437:68: note: in definition of macro 'newSVnv'
  437 | # define newSVnv(a)                             Perl_newSVnv(aTHX_ a)
      |                                                                    ^
*** Error code 1

This was introduced with 5fdf6e9, where INF and NAN support was expected to always exist. However, this is an IEEE fp feature and the VAX CPU predates that spec. Perl used to reasonable support VAX systems (cf. d_double_style_vax, d_double_has_inf and d_double_has_nan.) So I guess that INF/NAN support wasn't tested on a non-IEEE fp system. Reverting this patch makes it build again. Simply guarding that functionality with any of the above config options should just do the trick.

Thanks a lot!

The text was updated successfully, but these errors were encountered:

jkeenan · 2025-01-01T20:44:38Z

The pkgsrc repository (a large tree of build instructions for lots of software, used as a package source for NetBSD and many other unix-like distributions) recently updated Perl5 from 5.38 to 5.40, which now no longer builts for VAX targets:
...
embed.h:437:68: note: in definition of macro 'newSVnv'
  437 | # define newSVnv(a)                             Perl_newSVnv(aTHX_ a)
      |                                                                    ^
*** Error code 1
This was introduced with 5fdf6e9, where INF and NAN support was expected to always exist. However, this is an IEEE fp feature and the VAX CPU predates that spec. Perl used to reasonable support VAX systems (cf. d_double_style_vax, d_double_has_inf and d_double_has_nan.) So I guess that INF/NAN support wasn't tested on a non-IEEE fp system. Reverting this patch makes it build again. Simply guarding that functionality with any of the above config options should just do the trick.

Two things would be helpful in correcting this problem:

If patching is as simple as you describe, could someone produce such a patch?
We probably overlooked this due to lack of smoke-testing reports from non-IEEE fp VAX systems. Can you suggest how we might get such reports going forward?

@leonerd, can you take a look?

jbglaw · 2025-01-01T21:51:33Z

I can try to work on a patch (and also test patches), but I really don't know anything about the Perl language itself. For example, it looks like the inf and nan keywords / values / names are exported in some way (lib/builtin.pm) I don't know how that could be handled conditionally. Or if it's more desireable to always have inf and nan available, but non-working for a host that doesn't actually support IEEE fp. Don't know either how a Perl script could/should check for IEEE fp compliance.

So working on this isn't probably really about making it compile in some way, but also to clearify the fp interface an application / script can expect.

jbglaw · 2025-01-01T22:00:53Z

OTOH VAX systems are quite outdated by today's standards. Making it build again without breaking when not using nan/inf would probably be quite the right thing to do I guess.

Perl/perl5#22882

jbglaw · 2025-01-02T18:55:55Z

John Klos added a patch to pkgsrc: https://github.com/NetBSD/pkgsrc/blob/trunk/lang/perl5/patches/patch-builtin.c

That seems to work for us VAX people. Would be nice to get that upstreamed. (While it would probably be more correct to explicitely test for specific fp features, VAX systems are the last survivors here that don't offer IEEE math.)

abs0 · 2025-01-03T18:23:50Z

* We probably overlooked this due to lack of smoke-testing reports from non-IEEE fp VAX systems.  Can you suggest how we might get such reports going forward?

The easiest accessible emulator for vax would be https://github.com/open-simh/simh - I suppose what would be needed was a version of https://github.com/marketplace/actions/netbsd-vm which uses simh internally to a NetBSD/VAX VM. I... may ask on the port-vax NetBSD list

tonycoz · 2025-01-06T01:36:01Z

Perl code can check whether double supports floating point with:

use Config;
if ($Config{d_double_has_inf}) {
  ...
}

The C code can check with:

#ifdef DOUBLE_HAS_INF
...
#endif

(though some code assumes DOUBLE_HAS_NAN implies DOUBLE_HAS_INF.)

As to behaviour, I think allowing inf/nan to be imported and throwing an error when called would be best.

I'll look at producing a patch, though I'm not sure how I'll be able to test it.

johnklos · 2025-01-06T01:51:41Z

Forgetting integration of VAX on simh to github for the moment, I can test any patches on a VAX system.

On some machines (VAX), the double type doesn't support Infinity/NaN values. Handle this case by making builtin::inf/builtin::nan throw a runtime error. Fixes Perl#22882.

tonycoz · 2025-01-06T23:01:35Z

Could you please test mauke's patch above?

The patch is at https://patch-diff.githubusercontent.com/raw/Perl/perl5/pull/22889.patch though this probably won't apply cleanly to 5.40.

If you have git available you can clone the repository and checkout the PR locally:

git fetch https://github.com/Perl/perl5.git
cd perl5
git fetch origin pull/22889/head:test-branch
git checkout test-branch
# build and test

johnklos · 2025-01-07T01:42:46Z

Testing now. Will report back tomorrow. Thanks!

johnklos · 2025-01-07T18:02:10Z

Compilation succeeded, and perl tests are running now. So far, no problems at all.

jbglaw added the Needs Triage label Jan 1, 2025

jkeenan added the distro-vax label Jan 1, 2025

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Jan 2, 2025

Void NaN and infinity on VAX. Only fixes build, not usage. See:

b385e9c

Perl/perl5#22882

tonycoz self-assigned this Jan 6, 2025

mauke linked a pull request Jan 6, 2025 that will close this issue

builtin: make inf/nan conditional depending on float support #22889

Open

jkeenan removed the Needs Triage label Jan 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't assume NV_INF / NV_NAN #22882

Don't assume NV_INF / NV_NAN #22882

jbglaw commented Jan 1, 2025

jkeenan commented Jan 1, 2025

jbglaw commented Jan 1, 2025

jbglaw commented Jan 1, 2025

jbglaw commented Jan 2, 2025

abs0 commented Jan 3, 2025

tonycoz commented Jan 6, 2025

johnklos commented Jan 6, 2025

tonycoz commented Jan 6, 2025

johnklos commented Jan 7, 2025

johnklos commented Jan 7, 2025

Don't assume NV_INF / NV_NAN #22882

Don't assume NV_INF / NV_NAN #22882

Comments

jbglaw commented Jan 1, 2025

jkeenan commented Jan 1, 2025

jbglaw commented Jan 1, 2025

jbglaw commented Jan 1, 2025

jbglaw commented Jan 2, 2025

abs0 commented Jan 3, 2025

tonycoz commented Jan 6, 2025

johnklos commented Jan 6, 2025

tonycoz commented Jan 6, 2025

johnklos commented Jan 7, 2025

johnklos commented Jan 7, 2025