[PATCH] Configure leaves garbage in $Config{longdblinfbytes} #15725
Comments
From @ntyniThe Configure test for long double implementation details probes the % perl -V:longdblinfbytes Some of these bytes are currently essentially random and vary between The relevant Configure probe tries to initialize these bytes, but has The randomness seems to only have started to show on Debian with GCC-6 Perl Info |
From @ntyni0001-Configure-fix-garbage-filtering-with-80-bit-long-dou.patchFrom 70b5974daa4281a98ab22ba2cadc1801088e2e75 Mon Sep 17 00:00:00 2001
From: Niko Tyni <ntyni@debian.org>
Date: Fri, 18 Nov 2016 18:36:34 +0200
Subject: [PATCH] Configure: fix garbage filtering with 80-bit long doubles
The test had several problems that resulted in the excess
bytes not getting zeroed out. This caused random contents in
$Config{longdblinfbytes}, observed on Debian with GCC 6.2.0 (but not
5.4.1).
Bug-Debian: https://bugs.debian.org/844752
---
Configure | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/Configure b/Configure
index 6a3d617..f561a87 100755
--- a/Configure
+++ b/Configure
@@ -20661,8 +20661,8 @@ $cat >try.c <<EOP
#define DOUBLESIZE $doublesize
#$d_longdbl HAS_LONG_DOUBLE
#ifdef HAS_LONG_DOUBLE
-#define LONGDBLSIZE $longdblsize
-#define LONGDBLKIND $longdblkind
+#define LONG_DOUBLESIZE $longdblsize
+#define LONG_DOUBLEKIND $longdblkind
#endif
#$i_math I_MATH
#ifdef I_MATH
@@ -20699,16 +20699,14 @@ int main(int argc, char *argv[]) {
#ifdef HAS_LONG_DOUBLE
long double ldinf = (long double)exp(1e9);
long double ldnan = (long double)sqrt(-1.0);
-#endif
- if (argc == 2) {
- switch (argv[1][0]) {
- case '1': bytes(&dinf, sizeof(dinf)); break;
- case '2': bytes(&dnan, sizeof(dnan)); break;
-#ifdef HAS_LONG_DOUBLE
# if LONG_DOUBLEKIND == 3 || LONG_DOUBLEKIND == 4
/* the 80-bit long doubles might have garbage in their excess bytes */
memset((char *)&ldinf + 10, '\0', LONG_DOUBLESIZE - 10);
# endif
+ if (argc == 2) {
+ switch (argv[1][0]) {
+ case '1': bytes(&dinf, sizeof(dinf)); break;
+ case '2': bytes(&dnan, sizeof(dnan)); break;
case '3': bytes(&ldinf, sizeof(ldinf)); break;
case '4': bytes(&ldnan, sizeof(ldnan)); break;
#endif
--
2.10.2
|
From @jkeenanOn Fri, 18 Nov 2016 19:18:07 GMT, ntyni@debian.org wrote:
Do you expect that the impact of this will only show up if someone is using 'gcc' rather than, say, g++ or clang? I ask because I suspect we'll have to hunt up people to test this on multiple versions of multiple C compilers. Thank you very much. -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @jkeenanOn Fri, 18 Nov 2016 21:43:46 GMT, jkeenan wrote:
FWIW, here's the diff I got when applying your patch to blead using this 'gcc': #####
Is that what you would expect? Thank you very much. -- |
From pipcet@gmail.comThat patch fixes things for ldinf, but doesn't appear to do anything for ldnan. Instead of memsetting after the fact, we could clear an area of memory And maybe Configure C programs should be compiled with warnings |
From @tonycozOn Fri, Nov 18, 2016 at 10:23:10PM +0000, Pip Cet wrote:
That isn't reliable. While we might hope that the compiler generates code that stores See: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=123971 for a case where this bit us. Tony |
From @ntyniOn Fri, Nov 18, 2016 at 01:59:35PM -0800, James E Keenan via RT wrote:
My guess is that the garbage doesn't have any functional impact. Perhaps Jarkko (cc'd) knows more? I think he's the long double expert I noticed this because the Config.pm contents started to vary between
Yes; all of the last six bytes now get zeroed out like the code |
From @ntyniOn Fri, Nov 18, 2016 at 02:24:23PM -0800, Pip Cet via RT wrote:
I only patched the obvious bugs. I don't see randomness (or even non-zero But yes, ldnan should probably get the same treatment. Patch attached. |
From @ntyni0001-Configure-also-zero-out-high-bytes-of-80-bit-ldnan.patchFrom a45661a63a22630ba3578bc3854f3a2446a28c84 Mon Sep 17 00:00:00 2001
From: Niko Tyni <ntyni@debian.org>
Date: Sat, 19 Nov 2016 09:45:45 +0200
Subject: [PATCH] Configure: also zero out high bytes of 80-bit ldnan
These are currently zero anyway, but things are probably not guaranteed
to stay so.
---
Configure | 1 +
1 file changed, 1 insertion(+)
diff --git a/Configure b/Configure
index f561a87..845fc43 100755
--- a/Configure
+++ b/Configure
@@ -20702,6 +20702,7 @@ int main(int argc, char *argv[]) {
# if LONG_DOUBLEKIND == 3 || LONG_DOUBLEKIND == 4
/* the 80-bit long doubles might have garbage in their excess bytes */
memset((char *)&ldinf + 10, '\0', LONG_DOUBLESIZE - 10);
+ memset((char *)&ldnan + 10, '\0', LONG_DOUBLESIZE - 10);
# endif
if (argc == 2) {
switch (argv[1][0]) {
--
2.10.2
|
From @ntyniIn-Reply-To: <rt-4.0.24-6473-1479505426-193.130133-94-0@perl.org> On Fri, Nov 18, 2016 at 01:43:46PM -0800, James E Keenan via RT wrote:
I realize I probably didn't answer your question. Sorry. I've only tried with gcc 5 and 6 from Debian unstable. I'd expect g++ As for seeing the impact: with gcc 6 on Debian unstable/amd64 (=x86_64), % for i in $(seq 1 5); do ./a.out 3; done while the patched one consistently gives I'm attaching the (buggy) probe program for convenience. gcc is 'gcc version 6.2.0 20161103 (Debian 6.2.0-11)'. |
From @ntyni#define DOUBLESIZE 8
#define HAS_LONG_DOUBLE
#ifdef HAS_LONG_DOUBLE
#define LONG_DOUBLESIZE 16
#define LONG_DOUBLEKIND 3
#endif
#define I_MATH
#ifdef I_MATH
#include <math.h>
#endif
#include <stdio.h>
#include <string.h>
/* Note that whether the sign bit is on or off
* for NaN depends on the CPU/FPU, and possibly
* can be affected by the build toolchain.
*
* For example for older MIPS and HP-PA 2.0 the quiet NaN is:
* 0x7f, 0xf7, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff
* 0x7f, 0xf4, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
* (respectively) as opposed to the more usual
* 0x7f, 0xf8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
*/
static void bytes(unsigned char *p, unsigned int n) {
int i;
for (i = 0; i < n; i++) {
printf("0x%02x%s", p[i], i < n - 1 ? ", " : "\n");
}
}
int main(int argc, char *argv[]) {
/* We cannot use 1.0/0.0 and 0.0/0.0 (with L suffixes for long double)
* because some compilers are 'smart' and not only warn but refuse to
* compile such 'illegal' values. */
double dinf = exp(1e9);
double dnan = sqrt(-1.0);
#ifdef HAS_LONG_DOUBLE
long double ldinf = (long double)exp(1e9);
long double ldnan = (long double)sqrt(-1.0);
#endif
if (argc == 2) {
switch (argv[1][0]) {
case '1': bytes(&dinf, sizeof(dinf)); break;
case '2': bytes(&dnan, sizeof(dnan)); break;
#ifdef HAS_LONG_DOUBLE
# if LONG_DOUBLEKIND == 3 || LONG_DOUBLEKIND == 4
/* the 80-bit long doubles might have garbage in their excess bytes */
memset((char *)&ldinf + 10, '\0', LONG_DOUBLESIZE - 10);
# endif
case '3': bytes(&ldinf, sizeof(ldinf)); break;
case '4': bytes(&ldnan, sizeof(ldnan)); break;
#endif
}
}
return 0;
}
|
From @jhi
Yeah, beyond the 80 bits there might be any random garbage, and the (FWIW, I think there have been in the past x86-platform compilers
Didn't stop me getting the long doubles probing code wrong, though.
|
From pipcet@gmail.comOn Sat, Nov 19, 2016 at 7:51 AM, Niko Tyni <ntyni@debian.org> wrote:
I think that's just luck, though. The code generated by gcc here just
I think that's the best we can do, given the rather unruly way some |
From @tonycozOn Fri, 18 Nov 2016 23:52:12 -0800, ntyni@debian.org wrote:
Thanks, applied as dd68853 and 6b2c747. Tony |
|
@tonycoz - Status changed from 'open' to 'pending release' |
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release today of Perl 5.26.0, this and 210 other issues have been Perl 5.26.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
|
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#130133 (status was 'resolved')
Searchable as RT130133$
The text was updated successfully, but these errors were encountered: