Closed sisyphus closed 1 month ago
I don't see any errors building Cpanel::JSON::XS on blead built with USE_LONG_DOUBLE with gcc 13.1.0 ucrt.
I don't see any errors compiling Cpanel::JSON::XS on blead built with USE_QUADMATH with gcc 13.1.0 ucrt, but some tests do crash.
The crash I looked at crashed on on a ud2 (undefined opcode) instruction that appeared to be generated for the isinf(nv) calls, isinf() is defined:
/* 7.12.3.3 */
#define isinf(x) (fpclassify(x) == FP_INFINITE)
fpclassify:
#define fpclassify(x) \
__mingw_choose_expr ( \
__mingw_types_compatible_p (__typeof__ (x), double), \
__fpclassify(x), \
__mingw_choose_expr ( \
__mingw_types_compatible_p (__typeof__ (x), float), \
__fpclassifyf(x), \
__mingw_choose_expr ( \
__mingw_types_compatible_p (__typeof__ (x), long double), \
__fpclassifyl(x), \
__dfp_expansion(__fpclassify,(__builtin_trap(),0),x))))
and __dfp_expansion expands to it's second parameter (__builtin_trap(),0)
which generates the ud2.
So it's crashing because isinf() is called with a __float128.
Looking at the code I wonder why C::J::XS detects the raw functions rather than just using Perl_isinf()/Perl_isnan()?
Looking at the code I wonder why C::J::XS detects the raw functions rather than just using Perl_isinf()/Perl_isnan()?
Ok - thanks @tonycoz . I want to rewrite https://github.com/rurban/Cpanel-JSON-XS/pull/229 anyway - because, at the time, I hadn't picked up on the fact that HAVE_ISINFL/HAVE_ISNANL was not HAS_ISINFL/HAS_ISNANL. So I'll do that using Perl_isinf() and Perl_isnan(), which will hopefully do away with the need to examine NVTYPE at all.
As regards this issue I've raised here, should I just forget about what isinf()/isnan() and the like are doing .... and ignore those %Config settings .... and just settle for using Perl_isinf() and Perl_isnan() ?
In fact, I'll close this issue on the basis that the answer to that question is "yes". But feel free to reopen and/or reply if there's more that should be said.
AFTERTHOUGHT: I think that Cpanel::JSON::XS wants to support perl all the way back into the last century, so I'll need to check if Perl_isnan() and Perl_isinf() extend back far enough.
Another thing here is that perl doesn't define isinf(), isinfl() etc, that's up to the platform.
You have observed a change in behaviour though, so I thought maybe the "-std=c99" might be hiding isinfl(), isnanl(), but building with -std=gnu99 or without -std at all still produces crashes in those tests.
So I don't know what changed here.
The Cpanel::JSON::XS crashes in Windows quadmath builds are not related to any change. They occur in isinf() and isnan() on both 5.40 and 5.38 (and probably earlier quadmath builds, too).
It's only isinfl and isnanl that have changed between 5.38 and 5.40, and they have changed for all NVTYPES.
I knew my perl-5.40.0 build included https://github.com/Perl/perl5/pull/22257, which is why I mentioned it in the initial post. I've only just now realized that it's not actually part of the official perl release. It was not merged until after the release of 5.40.0. (I'm pretty sure Strawberry Perl 5.40.0 includes the same fix.) Also, that PR seems to affect only isnanl; no mention of isinfl that I could see.
If it keeps nagging at me, and I find the time, I'll dig into that change of behaviour some more. However, since Perl_isnan and Perl_isinf are working fine, I think this issue is of low importance and can remain closed.
Thanks for paying attention to it. @tonycoz.
With both gcc-13 and gcc-14, calls to isinfl() and isnanl() generate a [-Wimplicit-function-declaration] warning. In gcc-14 (but not gcc-13) that warning is a fatal error. I think that's all there is to the change in behaviour. (My 5.40.0 was built with gcc-14, but 5.38.0 was built with gcc-13.)
Ok thanks for the update.
UPDATE: I
needneeded to rewrite and recheck the parts of this that refer to HAVE* symbols. They are, of course, HAS* symbols. (I had been working on XS,xs from Cpanel-JSON-XS, which does check to see whether the symbol HAVE_ISINFL is defined.)I'll do that re-working of this post later today.The rewrite has now been completed.I suspect (only a guess) that https://github.com/Perl/perl5/pull/22257 is involved in the change of behaviour.
In perl-5.38.x we can call both isinfl() and isnanl() in an XSub, irrespective of NVTYPE. In perl-5.40.0 (and 5.41.3) calling of either isinfl() or isnanl() in an XSub will prevent the XSub from compiling - again, irrespective of NVTYPE.
With perl-5.40.0 and later, if we want to call an isinf/isnan function in an XSub, then we have to call isinfq/isnanq if NVTYPE is __float128. If NVTYPE is either double or long double, then we have to call isinf/isnan. Sticking to those rules seems to do the trick - though I haven't tested thoroughly.
On both perl-5.38.0 and perl-5.40.0 the 4 perl API symbols HAS_INF, HAS_INFL, HAS_NAN and HAS_NANL are defined, except for HAS_INFL - irrespective of NVTYPE. The 4 corresponding Config keys (d_isinf, d_isinfl, d_isnan and d_isnanl) match the setting of those 4 perl API symbols. It seems to me that those 4 settings are not always appropriate for all 3 NVTYPES - especially on 5.40.0.
I don't know how we should attack this. Is it sufficient to just return to the 5.38.0 behavior ?
Here is a little comparison between 5.38.0 and 5.40.0 that reports on the availability of the isinf() and isnan() functions in XSubs:
I noticed that the perl API symbols HAVE_ISINF, HAVE_ISINFL, HAVE_ISINFQ, HAVE_ISNAN, HAVE_ISNANL, and HAVE_ISNANQ are all undefined for Win32 (irrespective of NVTYPE), and I think it might have been that way for quite a while - perhaps forever ? It's therefore a bit surprising to see that $Config{d_isinf}, $Config{d_isnan} and $Config{d_isnanl} are all defined (irrespective of NVTYPE). The other 3 related Config keys are all undef, and it seems logical to me that all 6 should be undef ... but what would such a change break ?It seems that things don't fit together all that well at the moment, but I'm not sure what (if anything) to do about it.
Do we want those six HAVE* symbols to remain undefined for Windows ? I would have no objection to that, BTW. There's probably quite a bit of code kicking around that examines the definedness of those 6 HAVE* symbols. If that were to suddenly change for Windows, then it might get messy.
I'm not sure that "perl -V" output is going to be terribly helpful, but here's the one for my perl-5.40.0 (with NVTYPE of double):