Skip Menu |
Report information
Id: 121505
Status: open
Priority: 0/
Queue: perl5

Owner: tonyc <tony [at] develop-help.com>
Requestors: polacek [at] redhat.com
Cc:
AdminCc:

Operating System: Linux
PatchStatus: (no value)
Severity: High
Type: core
Perl Version: 5.14.3
Fixed In: (no value)

Attachments
0001-perl-121505-add-fwrapv-to-ccflags-for-gcc-4.9-and-la.patch
0001-perl-121505-include-fwrapv-by-default-for-GCC-4.3-an.patch
0001-perl-121505-include-fwrapv-by-default-for-GCC.patch



From: Marek Polacek <polacek [...] redhat.com>
Date: Tue, 25 Mar 2014 14:43:55 +0100
To: perlbug [...] perl.org
Subject: Can't build perl with gcc4.9 due to UB in sv.c
Download (untitled) / with headers
text/plain 7.3k
This is a bug report for perl from polacek@redhat.com, generated with the help of perlbug 1.39 running under perl 5.14.3. When doing mass rebuild of Fedora packages, rebuild of perl-5.18.2-296.fc21.src.rpm failed, because of undefined behavior in sv.c file. The problem here is that the code was trying to negate LONG_MIN, which is undefined behavior. GCC's optimizers then optimized the code in such a way that the perl testsuite and thus rebuild failed. A patch to fix the issue: --- sv.c.bak 2014-03-24 15:11:48.007595042 +0100 +++ sv.c 2014-03-25 11:57:41.154752451 +0100 @@ -2008,7 +2008,7 @@ S_sv_2iuv_common(pTHX_ SV *const sv) if (SvNVX(sv) == (NV) SvIVX(sv) #ifndef NV_PRESERVES_UV && (((UV)1 << NV_PRESERVES_UV_BITS) > - (UV)(SvIVX(sv) > 0 ? SvIVX(sv) : -SvIVX(sv))) + (UV)(SvIVX(sv) > 0 ? SvIVX(sv) : -SvUVX(sv))) /* Don't flag it as "accurately an integer" if the number came from a (by definition imprecise) NV operation, and we're outside the range of NV integer precision */ With this patch the rebuild passes even with new GCC. Moreover, GCC's -fsanitize=undefined found more UB in that file, here's a (untested) patch that should beat some sense in it. Again, you can't negate LONG_MIN. --- sv.c.bak 2014-03-24 15:11:48.007595042 +0100 +++ sv.c 2014-03-24 16:32:56.142470671 +0100 @@ -2117,7 +2117,7 @@ S_sv_2iuv_common(pTHX_ SV *const sv) } else { /* 2s complement assumption */ if (value <= (UV)IV_MIN) { - SvIV_set(sv, -(IV)value); + SvIV_set(sv, -(UV)value); } else { /* Too negative for an IV. This is a double upgrade, but I'm assuming it will be rare. */ @@ -2578,7 +2578,7 @@ Perl_sv_2nv_flags(pTHX_ SV *const sv, co SvIOKp_on(sv); if (numtype & IS_NUMBER_NEG) { - SvIV_set(sv, -(IV)value); + SvIV_set(sv, -(UV)value); } else if (value <= (UV)IV_MAX) { SvIV_set(sv, (IV)value); } else { @@ -2707,7 +2707,7 @@ S_uiv_2buf(char *const buf, const IV iv, uv = iv; sign = 0; } else { - uv = -iv; + uv = -(UV)iv; sign = 1; } do { @@ -10870,7 +10870,7 @@ Perl_sv_vcatpvfn_flags(pTHX_ SV *const s esignbuf[esignlen++] = plus; } else { - uv = -iv; + uv = -(UV)iv; esignbuf[esignlen++] = '-'; } } I checked the mainline in git and the bugs seem to be still there. Note that perlbug might send you information about my system perl, but that's completely irrelevant here. ----------------------------------------------------------------- [Please describe your issue here] [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=high --- This perlbug was built using Perl 5.14.3 in the Fedora build system. It is being executed now by Perl 5.14.3 - Fri Jan 11 13:09:43 UTC 2013. Site configuration information for perl 5.14.3: Configured by Red Hat, Inc. at Fri Jan 11 13:09:43 UTC 2013. Summary of my perl5 (revision 5 version 14 subversion 3) configuration: Platform: osname=linux, osvers=2.6.32-279.9.1.el6.x86_64, archname=x86_64-linux-thread-multi uname='linux buildvm-25.phx2.fedoraproject.org 2.6.32-279.9.1.el6.x86_64 #1 smp fri aug 31 09:04:24 edt 2012 x86_64 x86_64 x86_64 gnulinux ' config_args='-des -Doptimize=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -Dccdlflags=-Wl,--enable-new-dtags -DDEBUGGING=-g -Dversion=5.14.3 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl5 -Dsitearch=/usr/local/lib64/perl5 -Dprivlib=/usr/share/perl5 -Dvendorlib=/usr/share/perl5/vendor_perl -Darchlib=/usr/lib64/perl5 -Dvendorarch=/usr/lib64/perl5/vendor_perl -Darchname=x86_64-linux-thread-multi -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Duseshrplib -Dusethreads -Duseithreads -Dusedtrace=/usr/bin/dtrace -Duselargefiles -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto -Dscriptdir=/usr/bin' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic', cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='4.6.3 20120306 (Red Hat 4.6.3-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='gcc', ldflags =' -fstack-protector' libpth=/usr/local/lib64 /lib64 /usr/lib64 libs=-lresolv -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat perllibs=-lresolv -lnsl -ldl -lm -lcrypt -lutil -lpthread -lc libc=, so=so, useshrplib=true, libperl=libperl.so gnulibc_version='2.14.90' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,--enable-new-dtags -Wl,-rpath,/usr/lib64/perl5/CORE' cccdlflags='-fPIC', lddlflags='-shared -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic' Locally applied patches: --- @INC for perl 5.14.3: /home/marek/perl5/lib/perl5/x86_64-linux-thread-multi /home/marek/perl5/lib/perl5/x86_64-linux-thread-multi /home/marek/perl5/lib/perl5 /usr/local/lib64/perl5 /usr/local/share/perl5 /usr/lib64/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib64/perl5 /usr/share/perl5 . --- Environment for perl 5.14.3: HOME=/home/marek LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH=/home/marek/rh/x/trunk/gcc:/home/marek/rh/x/trunk/gcc/32:/home/marek/rh/x/trunk/x86_64-unknown-linux-gnu/32/libsanitizer/ubsan/.libs::/home/marek/rh/x/trunk/gcc:/home/marek/rh/x/trunk/gcc/32:/home/marek/rh/x/trunk/x86_64-unknown-linux-gnu/32/libsanitizer/ubsan/.libs:/usr/lib64/mpich2/lib LOGDIR (unset) PATH=/home/marek/perl5/bin:/home/marek/perl5/bin:/home/marek/perl5/bin:/usr/lib64/qt-3.3/bin:/usr/lib64/mpich2/bin:/usr/kerberos/sbin:/usr/kerberos/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/home/marek/.local/bin:/home/marek/bin PERL5LIB=/home/marek/perl5/lib/perl5/x86_64-linux-thread-multi:/home/marek/perl5/lib/perl5 PERL_BADLANG (unset) PERL_LOCAL_LIB_ROOT=/home/marek/perl5 PERL_MB_OPT=--install_base /home/marek/perl5 PERL_MM_OPT=INSTALL_BASE=/home/marek/perl5 SHELL=/bin/bash
CC: perl5-porters [...] perl.org
Subject: [perl #121505] Re: Perl 5.20.0 Blockers, 2014-03-31
To: Zefram <zefram [...] fysh.org>
Date: Wed, 2 Apr 2014 12:22:59 +1100
From: Tony Cook <tony [...] develop-help.com>
Download (untitled) / with headers
text/plain 2.8k
On Tue, Apr 01, 2014 at 07:24:37AM +0100, Zefram wrote: Show quoted text
> Tony Cook wrote:
> >-fwrapv does prevent the optimizations that cause the test failures.
> > Right. It might be an interesting exercise to check what other parts > of the code it affects. Diffing the assembler may yield a comprehensive > list of points where this `optimisation' breaks the semantics we expect. > This would be more complete than you'd get by just looking at test > failures. (Still not likely to be a complete list of places where we > rely on wrapping, though.)
Unfortunately there's a lot of code re-ordering differences between the two that makes it difficult to compare. Interestingly enough the code that the patch updates has no changes between no -fwrapv and with -fwrapv. Adding the patch produces a significant change in the code - closer to a simple translation of the code rather than what appears to be some fancy 2's complment processing: Without the patch: ; the .loc lines are line number information .loc 1 2054 0 cqto ; sign extend rax into rdx:rax .loc 1 2053 0 movabsq $9007199254740991, %rcx ; ( 1 << NV_PRESERVES_UV_BITS ) - 1 .loc 1 2054 0 xorq %rdx, %rax subq %rdx, %rax movq %rax, %rdx .loc 1 2285 0 xorl %eax, %eax .loc 1 2053 0 cmpq %rcx, %rdx ja .L4793 With the patch: .loc 1 2053 0 testq %rdx, %rdx jle .L4724 movabsq $9007199254740991, %rdx cmpq %rdx, 32(%rax) setbe %al .L4725: .loc 1 2053 0 discriminator 4 testb %al, %al je .L4787 .loc 1 2060 0 is_stmt 1 movl 12(%rbx), %edx .loc 1 2285 0 xorl %eax, %eax .loc 1 2060 0 testb $2, %dh je .L4787 ... .L4724: .LBE1932: .loc 1 2053 0 discriminator 2 movabsq $9007199254740991, %rax .loc 1 2054 0 discriminator 2 negq %rdx .loc 1 2053 0 discriminator 2 cmpq %rax, %rdx setbe %al jmp .L4725 Show quoted text
> If that works, there are multiple things we could do with the list. > An alternative to adding -fwrapv to the command line would be to edit > all the locations shown up to avoid signed overflow, along the lines of > the minimal patch that tackles just one location. This is weaker than > actually using -fwrapv, because it only fixes the locations that are > currently affected by the dodgy optimisation. We could also attempt to > write tests to exercise the overflow behaviour of each location.
I don't think that's practical. Show quoted text
> >To include -fwrapv where needed I think a change to Configure will be > >needed, since gcc can be used on several platforms.
> > Yes, absolutely. We'd need to add it whenever we're using a gcc that > supports the flag. (The flag does exist prior to 4.9; don't know how > far back it goes.)
At least back to 4.1. Tony
From: Tony Cook <tony [...] develop-help.com>
Date: Tue, 8 Apr 2014 17:00:13 +1000
To: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>
Subject: [perl #121505] Re: Perl 5.20.0 Blockers, 2014-04-07
CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.9k
Subject altered so it shows in the ticket too. On Mon, Apr 07, 2014 at 11:25:14AM -0400, Ricardo Signes wrote: Show quoted text
> 7. Can't build perl with gcc4.9 due to UB in sv.c > https://rt.perl.org/Ticket/Display.html?id=121505 > > Discussion of this on list has made it seem that this can't be a blocker, > and that we will need to carefully update our use of compiler flags to be > sure we get this right. Please inform me if I've misread. > > Alternately: compiler-savvy people: is this a blocker for 5.20.0, 5.20.1, > or 5.22?
GCC 4.9 hasn't been released yet, but will likely be released by the time we release 5.20[1], so users will expect to be able to build and test with it. So I think it's a 5.20 issue. The behaviour we're depending on technically violates the C standard (at least C89 and C99), and GCC takes advantage of that. GCC 4.9 happens to take it further and breaks perl. In the short term for 5.20.0 I think we (and I'll make a start on it) update Configure to check if -fwrapv is supported and if so, enable it (or cflags.SH?) In the longer term I think we need to deal with the problem. In some cases, like the handling of RExC_naughty in regcomp.c, I think changing the type to an unsigned type will be enough of a change. I suspect the current behaviour here is that some regexps aren't being flagged "naughty" that should be[2]. Other cases, like the overflow at line 4661 in regcomp.c: data->last_start_max += is_inf ? SSize_t_MAX : (maxcount - 1) * (minnext + data->pos_delta); could be real problems leading to bugs down the road. (the above 2 of many detected with -fsantize=undefined) Tony [1] 4.7.0 and 4.8.0 were released in March 2012 and 2013 respectively (http://www.gnu.org/software/gcc/releases.html) and the last GCC 4.9 status report hoped to be announcing a RC "soon" (http://gcc.gnu.org/ml/gcc/2014-03/msg00190.html) [2] I don't know if that matters much, I only see one test for PREGf_NAUGHTY in regexec.c
CC: Ricardo Signes <perl.p5p [...] rjbs.manxome.org>, perl5-porters [...] perl.org
From: Andy Dougherty <doughera [...] lafayette.edu>
To: Tony Cook <tony [...] develop-help.com>
Date: Tue, 8 Apr 2014 07:42:23 -0400
Subject: Re: [perl #121505] Re: Perl 5.20.0 Blockers, 2014-04-07
Download (untitled) / with headers
text/plain 1.6k
On Tue, Apr 08, 2014 at 05:00:13PM +1000, Tony Cook wrote: Show quoted text
> Subject altered so it shows in the ticket too. > > On Mon, Apr 07, 2014 at 11:25:14AM -0400, Ricardo Signes wrote:
> > 7. Can't build perl with gcc4.9 due to UB in sv.c > > https://rt.perl.org/Ticket/Display.html?id=121505 > > > > Discussion of this on list has made it seem that this can't be a blocker, > > and that we will need to carefully update our use of compiler flags to be > > sure we get this right. Please inform me if I've misread. > > > > Alternately: compiler-savvy people: is this a blocker for 5.20.0, 5.20.1, > > or 5.22?
> > GCC 4.9 hasn't been released yet, but will likely be released by the > time we release 5.20[1], so users will expect to be able to build and > test with it. > > So I think it's a 5.20 issue. > > The behaviour we're depending on technically violates the C standard > (at least C89 and C99), and GCC takes advantage of that. GCC 4.9 > happens to take it further and breaks perl. > > In the short term for 5.20.0 I think we (and I'll make a start on it) > update Configure to check if -fwrapv is supported and if so, enable > it (or cflags.SH?)
I think it would have to be Configure. cflags.SH only affects the compilation of the perl core files. Don't we need the flag to be propagated to config.sh (and hence Config.pm) and used in building extensions as well? (I'm afraid I haven't looked at the underlying issue at all, since it looks like an issue that requires some sustained thought, and I haven't had sufficient blocks of time for that in quite a while.) -- Andy Dougherty doughera@lafayette.edu
Subject: Re: [perl #121505] Re: Perl 5.20.0 Blockers, 2014-04-07
Date: Tue, 8 Apr 2014 12:45:39 +0100
To: perl5-porters [...] perl.org
From: Zefram <zefram [...] fysh.org>
Download (untitled) / with headers
text/plain 194b
Andy Dougherty wrote: Show quoted text
> Don't we need the flag to be >propagated to config.sh (and hence Config.pm) and used in building >extensions as well?
Yes. -zefram
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 287b
On Tue Apr 08 04:46:13 2014, zefram@fysh.org wrote: Show quoted text
> Andy Dougherty wrote:
> > Don't we need the flag to be > >propagated to config.sh (and hence Config.pm) and used in building > >extensions as well?
>
Patch to Configure attached for review. Tony
Subject: 0001-perl-121505-add-fwrapv-to-ccflags-for-gcc-4.9-and-la.patch
From 1d48451145d8351349ecdfa6a13ea422a4480cd3 Mon Sep 17 00:00:00 2001 From: Tony Cook <tony@develop-help.com> Date: Wed, 9 Apr 2014 11:58:33 +1000 Subject: [PATCH] [perl #121505] add -fwrapv to ccflags for gcc 4.9 and later But skip adding it if -fwrapv is already provided, or if -fno-wrapv or -fsanitize=undefined is supplied in ccflags. --- Configure | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Configure b/Configure index 15b3da1..56ce12d 100755 --- a/Configure +++ b/Configure @@ -4643,6 +4643,22 @@ case "$gccversion" in $rm -f try try.* esac +# gcc 4.9 by default does some optimizations that break perl. +# see ticket 121505. +# +# The -fwrapv disables those optimizations (and probably others,) so +# for gcc 4.9 (and later, since the optimizations probably won't go +# away), add -fwrapv unless the user requests -fno-wrapv, which +# disables -fwrapv, or if the user requests -fsanitize=undefined, +# which turns the overflows -fwrapv ignores into runtime errors. +case "$gccversion" in +4.9.*|4.1[0-9].*|[5-9].*) + case "$ccflags" in + *-fno-wrapv*|*-fsanitize=undefined*|*-fwrapv*) ;; + *) ccflags="$ccflags -fwrapv" ;; + esac +esac + : What should the include directory be ? : Use sysroot if set, so findhdr looks in the right place. echo " " -- 1.7.10.4
From: "H.Merijn Brand" <h.m.brand [...] xs4all.nl>
Date: Wed, 9 Apr 2014 08:09:33 +0200
To: perl5-porters [...] perl.org
Subject: Re: [perl #121505] Can't build perl with gcc4.9 due to UB in sv.c
CC: perlbug-followup [...] perl.org
Download (untitled) / with headers
text/plain 1018b
On Tue, 8 Apr 2014 18:59:35 -0700, "Tony Cook via RT" <perlbug-followup@perl.org> wrote: Show quoted text
> On Tue Apr 08 04:46:13 2014, zefram@fysh.org wrote:
> > Andy Dougherty wrote:
> > > Don't we need the flag to be > > >propagated to config.sh (and hence Config.pm) and used in building > > >extensions as well?
> >
> > Patch to Configure attached for review.
Why make it a new case, and not integrate into the switch just above? And are there any plans at all to "fix" the cause for this patch in perl-core? I saw some discussions pass, but I didn't pay too much attention. I got the idea people thought it too much work to fix all issues that are related to this optimization (other arch than intel) -- H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/ using perl5.00307 .. 5.19 porting perl5 on HP-UX, AIX, and openSUSE http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/ http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
Subject: Re: [perl #121505] Can't build perl with gcc4.9 due to UB in sv.c
CC: perl5-porters [...] perl.org, perlbug-followup [...] perl.org
Date: Wed, 9 Apr 2014 16:36:14 +1000
To: "H.Merijn Brand" <h.m.brand [...] xs4all.nl>
From: Tony Cook <tony [...] develop-help.com>
Download (untitled) / with headers
text/plain 1.1k
On Wed, Apr 09, 2014 at 08:09:33AM +0200, H.Merijn Brand wrote: Show quoted text
> On Tue, 8 Apr 2014 18:59:35 -0700, "Tony Cook via RT" > <perlbug-followup@perl.org> wrote: >
> > On Tue Apr 08 04:46:13 2014, zefram@fysh.org wrote:
> > > Andy Dougherty wrote:
> > > > Don't we need the flag to be > > > >propagated to config.sh (and hence Config.pm) and used in building > > > >extensions as well?
> > >
> > > > Patch to Configure attached for review.
> > Why make it a new case, and not integrate into the switch just above?
To make it easier to remove later (or at least make it easier to to what needs to be removed.) Show quoted text
> And are there any plans at all to "fix" the cause for this patch in > perl-core? I saw some discussions pass, but I didn't pay too much > attention. I got the idea people thought it too much work to fix all > issues that are related to this optimization (other arch than intel)
I think it's too much work to fix them all and be sure the fixes haven't introduced new issues before 5.20 is due for release. I think they should be fixed for 5.22, and then the Configure change removed. Tony
Subject: Re: [perl #121505] Can't build perl with gcc4.9 due to UB in sv.c
Date: Wed, 9 Apr 2014 10:15:53 +0100
To: perl5-porters [...] perl.org
From: Zefram <zefram [...] fysh.org>
Download (untitled) / with headers
text/plain 1014b
Tony Cook via RT wrote: Show quoted text
>+# for gcc 4.9 (and later, since the optimizations probably won't go >+# away), add -fwrapv
Should also add it for older gccs, as far back as the flag is supported and working. We only notice the need on 4.9, but that doesn't mean there aren't similar problems lurking. Show quoted text
>+4.9.*|4.1[0-9].*|[5-9].*)
Misses a lot of possible version numbers. For 4.9+, you want 4.9.*|4.[1-9][0-9]*|[5-9].*|[1-9][0-9]*) A bit of googling says -fwrapv is broken prior to 4.3, so that would be a better threshold: 4.[3-9].*|4.[1-9][0-9]*|[5-9].*|[1-9][0-9]*) Some explanation: http://www.airs.com/blog/archives/120 That page also offers the options -fno-strict-overflow and -Wstrict-overflow. I think we do want -fwrapv rather than -fno-strict-overflow. We could usefully apply -Wstrict-overflow to help audit (in the 5.21 cycle) for other places where we're relying on overflow behaviour: it won't give complete results, but will at least tell us where the overflow would affect gcc. -zefram
From: "H.Merijn Brand" <h.m.brand [...] xs4all.nl>
Subject: Re: [perl #121505] Can't build perl with gcc4.9 due to UB in sv.c
To: perl5-porters [...] perl.org
Date: Wed, 9 Apr 2014 11:32:21 +0200
Download (untitled) / with headers
text/plain 1.4k
On Wed, 9 Apr 2014 10:15:53 +0100, Zefram <zefram@fysh.org> wrote: Show quoted text
> Tony Cook via RT wrote:
> >+# for gcc 4.9 (and later, since the optimizations probably won't go > >+# away), add -fwrapv
> > Should also add it for older gccs, as far back as the flag is supported > and working. We only notice the need on 4.9, but that doesn't mean > there aren't similar problems lurking. >
> >+4.9.*|4.1[0-9].*|[5-9].*)
> > Misses a lot of possible version numbers. For 4.9+, you want > > 4.9.*|4.[1-9][0-9]*|[5-9].*|[1-9][0-9]*) > > A bit of googling says -fwrapv is broken prior to 4.3, so that would be > a better threshold: > > 4.[3-9].*|4.[1-9][0-9]*|[5-9].*|[1-9][0-9]*)
I already pushed Tony's patch. Shall I amend it? Show quoted text
> Some explanation: > > http://www.airs.com/blog/archives/120 > > That page also offers the options -fno-strict-overflow and > -Wstrict-overflow. I think we do want -fwrapv rather than > -fno-strict-overflow. We could usefully apply -Wstrict-overflow to > help audit (in the 5.21 cycle) for other places where we're relying on > overflow behaviour: it won't give complete results, but will at least > tell us where the overflow would affect gcc. > > -zefram
-- H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/ using perl5.00307 .. 5.19 porting perl5 on HP-UX, AIX, and openSUSE http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/ http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
Subject: Re: [perl #121505] Can't build perl with gcc4.9 due to UB in sv.c
Date: Wed, 9 Apr 2014 10:33:23 +0100
To: perl5-porters [...] perl.org
From: Zefram <zefram [...] fysh.org>
H.Merijn Brand wrote: Show quoted text
>I already pushed Tony's patch. Shall I amend it?
+1 to amend. -zefram
From: "H.Merijn Brand" <h.m.brand [...] xs4all.nl>
Subject: Re: [perl #121505] Can't build perl with gcc4.9 due to UB in sv.c
Date: Wed, 9 Apr 2014 20:38:57 +0200
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 963b
On Wed, 9 Apr 2014 10:33:23 +0100, Zefram <zefram@fysh.org> wrote: Show quoted text
> H.Merijn Brand wrote:
> >I already pushed Tony's patch. Shall I amend it?
> > +1 to amend.
For those not following git, both the original patch and the amendment done in commit 00051dd553979bd2a1dee100c324b59ee76a49e7 Author: H.Merijn Brand <h.m.brand@xs4all.nl> Date: Wed Apr 9 12:31:23 2014 +0200 -fwrapv is broken prior to gcc-4.3 (googled and patched by Zefram) commit 869747506fd0081f6c7eed149ec6f7adbcc4d5b1 Author: H.Merijn Brand <h.m.brand@xs4all.nl> Date: Wed Apr 9 11:16:55 2014 +0200 gcc 4.9 by default does some optimizations that break perl (#121505) Patch by Tony Cook -- H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/ using perl5.00307 .. 5.19 porting perl5 on HP-UX, AIX, and openSUSE http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/ http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.3k
On Wed Apr 09 02:16:26 2014, zefram@fysh.org wrote: Show quoted text
> Tony Cook via RT wrote:
> >+# for gcc 4.9 (and later, since the optimizations probably won't go > >+# away), add -fwrapv
> > Should also add it for older gccs, as far back as the flag is supported > and working. We only notice the need on 4.9, but that doesn't mean > there aren't similar problems lurking. >
> >+4.9.*|4.1[0-9].*|[5-9].*)
> > Misses a lot of possible version numbers. For 4.9+, you want > > 4.9.*|4.[1-9][0-9]*|[5-9].*|[1-9][0-9]*) > > A bit of googling says -fwrapv is broken prior to 4.3, so that would be > a better threshold: > > 4.[3-9].*|4.[1-9][0-9]*|[5-9].*|[1-9][0-9]*) > > Some explanation: > > http://www.airs.com/blog/archives/120 > > That page also offers the options -fno-strict-overflow and > -Wstrict-overflow. I think we do want -fwrapv rather than > -fno-strict-overflow. We could usefully apply -Wstrict-overflow to > help audit (in the 5.21 cycle) for other places where we're relying on > overflow behaviour: it won't give complete results, but will at least > tell us where the overflow would affect gcc.
The problem with including 4.3 - 4.8 is -fwrapv disables optimizations, and since we've had no reports that those optimizations cause problems in gcc 4.3 through 4.8, we're slowing perl down for no particular reason. I still need to produce an update for win32/makefile.mk. Tony
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 320b
On Wed Apr 09 14:55:54 2014, tonyc wrote: Show quoted text
> I still need to produce an update for win32/makefile.mk.
Though op/numconvert.t doesn't fail, nor does any other relevant test, with 4.9.0 20140218. I don't really see a way to pick out a version in makefile.mk either, the only real solution I see is to always add it. Tony
From: Steve Hay <steve.m.hay [...] googlemail.com>
CC: "perl5-porters [...] perl.org" <perl5-porters [...] perl.org>
Subject: Re: [perl #121505] Can't build perl with gcc4.9 due to UB in sv.c
To: Tony Cook via RT <perlbug-followup [...] perl.org>
Date: Thu, 10 Apr 2014 08:02:41 +0100
Download (untitled) / with headers
text/plain 517b
On 10 April 2014 00:57, Tony Cook via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Wed Apr 09 14:55:54 2014, tonyc wrote:
>> I still need to produce an update for win32/makefile.mk.
> > Though op/numconvert.t doesn't fail, nor does any other relevant test, with 4.9.0 20140218. > > I don't really see a way to pick out a version in makefile.mk either, the only real solution I see is to always add it. >
Add a new CCTYPE, as is done for different MSVC++ versions? GCC49 or GCC490 if you're using gcc-4.9.0 or higher?
Subject: Re: [perl #121505] Can't build perl with gcc4.9 due to UB in sv.c
To: perl5-porters [...] perl.org
Date: Thu, 10 Apr 2014 10:05:00 +0100
From: Zefram <zefram [...] fysh.org>
Download (untitled) / with headers
text/plain 295b
Tony Cook via RT wrote: Show quoted text
>The problem with including 4.3 - 4.8 is -fwrapv disables optimizations,
It disables specifically the optimisations that break our code. Show quoted text
>since we've had no reports that those optimizations cause problems
The problems wouldn't necessarily have been noticed. -zefram
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.3k
On Thu Apr 10 00:03:05 2014, shay wrote: Show quoted text
> On 10 April 2014 00:57, Tony Cook via RT <perlbug-followup@perl.org> > wrote:
> > On Wed Apr 09 14:55:54 2014, tonyc wrote:
> >> I still need to produce an update for win32/makefile.mk.
> > > > Though op/numconvert.t doesn't fail, nor does any other relevant > > test, with 4.9.0 20140218. > > > > I don't really see a way to pick out a version in makefile.mk either, > > the only real solution I see is to always add it. > >
> > Add a new CCTYPE, as is done for different MSVC++ versions? GCC49 or > GCC490 if you're using gcc-4.9.0 or higher?
Adding extra CCTYPE values means touching every place that looks at CCTYPE. I tried to add GCCVERSION and then parse that using findstr, but it was unwieldly[1] and didn't work, since dmake treats the non-zero exit from findstr when it doesn't find the string as a build-time error. I ended up adding a GCCWRAPV macro that can be overridden from the dmake command-line which enables -fwrapv by default. This builds and tests ok with the gcc 4.7.3 included with the strawberry perl I have installed. It builds perl successfully with a mingw gcc-4.9 snapshot, but I suspect testing would fail due to some strangeness in the built Errno.pm (the errno codes have l (lowercase L) suffixes. For review/comment. Tony [1] findstr supports regexps - but the regexps don't support alternation
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.4k
On Sun Apr 13 20:35:51 2014, tonyc wrote: Show quoted text
> On Thu Apr 10 00:03:05 2014, shay wrote:
> > On 10 April 2014 00:57, Tony Cook via RT <perlbug-followup@perl.org> > > wrote:
> > > On Wed Apr 09 14:55:54 2014, tonyc wrote:
> > >> I still need to produce an update for win32/makefile.mk.
> > > > > > Though op/numconvert.t doesn't fail, nor does any other relevant > > > test, with 4.9.0 20140218. > > > > > > I don't really see a way to pick out a version in makefile.mk > > > either, > > > the only real solution I see is to always add it. > > >
> > > > Add a new CCTYPE, as is done for different MSVC++ versions? GCC49 or > > GCC490 if you're using gcc-4.9.0 or higher?
> > Adding extra CCTYPE values means touching every place that looks at > CCTYPE. > > I tried to add GCCVERSION and then parse that using findstr, but it > was unwieldly[1] and didn't work, since dmake treats the non-zero exit > from findstr when it doesn't find the string as a build-time error. > > I ended up adding a GCCWRAPV macro that can be overridden from the > dmake command-line which enables -fwrapv by default. > > This builds and tests ok with the gcc 4.7.3 included with the > strawberry perl I have installed. > > It builds perl successfully with a mingw gcc-4.9 snapshot, but I > suspect testing would fail due to some strangeness in the built > Errno.pm (the errno codes have l (lowercase L) suffixes. > > For review/comment. >
I don't see any new attachment or smoke-me branch for review/comment. Am I missing something?
Subject: Re: [perl #121505] Can't build perl with gcc4.9 due to UB in sv.c
CC: pp <perl5-porters [...] perl.org>
To: Father Chrysostomos via RT <perlbug-followup [...] perl.org>
Date: Fri, 18 Apr 2014 14:29:34 -0500
From: Reini Urban <rurban [...] x-ray.at>
Download (untitled) / with headers
text/plain 596b
I'm following this thread with more and more disgust. Are your really advocating keeping wrong and UB? This will turn into security issues, I'll promise you. You really don't want to fix the signed integers to unsigned when you wrap it, and you rather want to hurt all of integer arithmetic optimizations by forcing -fwrapv onto us? Seriously, fix the problems as patched by Mark Polacek who knows what he is doing, and don't cover it up with -fwrapv. Optimizers rely on proper signed vs unsigned types, and if you treat it as unsigned declare it as such, so that the optimizer knows about it.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 163b
On Fri Apr 18 08:25:02 2014, shay wrote: Show quoted text
> I don't see any new attachment or smoke-me branch for review/comment. > Am I missing something?
Oops, here it is. Tony
Subject: 0001-perl-121505-include-fwrapv-by-default-for-GCC.patch
From 738956387b9513375e745028974e58b211bd4573 Mon Sep 17 00:00:00 2001 From: Tony Cook <tony@develop-help.com> Date: Mon, 14 Apr 2014 13:28:26 +1000 Subject: [perl #121505] include -fwrapv by default for GCC Unfortunately dmake/the Win32 command-line make it difficult to parse a GCC version (even if we pre-extract it), set -fwrapv by default and make it easy to disable, eg. dmake GCCWRAPV=undef --- win32/makefile.mk | 15 ++++++++++++++- 1 files changed, 14 insertions(+), 1 deletions(-) diff --git a/win32/makefile.mk b/win32/makefile.mk index c11a7a9..ee25f11 100644 --- a/win32/makefile.mk +++ b/win32/makefile.mk @@ -141,6 +141,12 @@ USE_LARGE_FILES *= define CCTYPE *= GCC # +# If you are using GCC, by default we add the -fwrapv option. +# See https://rt.perl.org/Ticket/Display.html?id=121505 +# +GCCWRAPV *= define + +# # If you are using Intel C++ Compiler uncomment this # #__ICC *= define @@ -408,12 +414,19 @@ INST_HTML = $(INST_TOP)$(INST_VER)\html .USESHELL : +MINIBUILDOPT *= + .IF "$(CCTYPE)" == "GCC" .IF "$(GCCCROSS)" == "define" ARCHPREFIX = x86_64-w64-mingw32- .ENDIF +.IF "$(GCCWRAPV)" == "define" +BUILDOPT += -fwrapv +MINIBUILDOPT += -fwrapv +.ENDIF + CC = $(ARCHPREFIX)gcc LINK32 = $(ARCHPREFIX)g++ LIB32 = $(ARCHPREFIX)ar rc @@ -1127,7 +1140,7 @@ $(MINIDIR) : if not exist "$(MINIDIR)" mkdir "$(MINIDIR)" $(MINICORE_OBJ) : $(CORE_NOCFG_H) - $(CC) -c $(CFLAGS) -DPERL_EXTERNAL_GLOB -DPERL_IS_MINIPERL $(OBJOUT_FLAG)$@ ..\$(*B).c + $(CC) -c $(CFLAGS) $(MINIBUILDOPT) -DPERL_EXTERNAL_GLOB -DPERL_IS_MINIPERL $(OBJOUT_FLAG)$@ ..\$(*B).c $(MINIWIN32_OBJ) : $(CORE_NOCFG_H) $(CC) -c $(CFLAGS) -DPERL_IS_MINIPERL $(OBJOUT_FLAG)$@ $(*B).c -- 1.7.4.msysgit.0
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.3k
On Fri Apr 18 12:30:19 2014, rurban wrote: Show quoted text
> I'm following this thread with more and more disgust. > Are your really advocating keeping wrong and UB? This will turn into > security issues, I'll promise you. > You really don't want to fix the signed integers to unsigned when you > wrap it, and you rather want to hurt all of integer arithmetic > optimizations by forcing -fwrapv onto us?
That patch fixes only a few of several issues detected by -fsanitize=undefined. Fixing all of the integer overflow undefined behaviour in perl would either mean releasing a perl with possibly tricky changes without extensive testing, or a several month delay while we wait for the changes to settle down. Note that most of these issues are in 5.18, 5.16, 5.14 etc, we're not releasing a perl with new bugs. (well, not these new bugs) Show quoted text
> Seriously, fix the problems as patched by Mark Polacek who knows what > he is doing, and don't cover it up with -fwrapv. > Optimizers rely on proper signed vs unsigned types, and if you treat > it as unsigned declare it as such, so that the optimizer knows about > it.
The -fwrapv change is only a workaround, which you know if you're read the thread. I don't plan to close this ticket once these workarounds are in, I'll open new tickets that this ticket depends on for each of the issues -fsanitize=undefined. No-one is happy with the -fwrapv "solution". Tony
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.4k
On Mon Apr 21 16:05:34 2014, tonyc wrote: Show quoted text
> On Fri Apr 18 08:25:02 2014, shay wrote:
> > I don't see any new attachment or smoke-me branch for review/comment. > > Am I missing something?
> > Oops, here it is. >
Testing with a 32-bit snapshot (http://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win32/Personal%20Builds/Experimental_Builds/4.9.0/threads-win32/sjlj/) I see no problem to start with, but with a 64-bit snapshot (http://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/Experimental_Builds/4.9.0/threads-win32/seh/) I get failures in op/range.t and op/numconvert.t without the patch. Those failures are fixed by the patch. Why is the -fwrapv left out from the MINIWIN32_OBJ compilation? My test results suggest that we don't currently have a problem with those files (win32.c, win32sck.c, win32thread.c, fcrypt.c and win32io.c) but surely it wouldn't hurt to compile them with it anyway to protect against any future trouble? (Likewise the x2p files, although they hardly matter since they're moving out of core in due course anyway.) Also, since the failure only happens on a 64-bit build, is it worth only including -fwrapv when doing a 64-bit build? That way, users of earlier versions of gcc that don't support -fwrapv aren't bothered by this if they're only doing a 32-bit build anyway. (Or have I just been lucky in not seeing any failure in a 32-bit build?)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.7k
On Tue Apr 22 05:10:18 2014, shay wrote: Show quoted text
> On Mon Apr 21 16:05:34 2014, tonyc wrote:
> > On Fri Apr 18 08:25:02 2014, shay wrote:
> > > I don't see any new attachment or smoke-me branch for > > > review/comment. > > > Am I missing something?
> > > > Oops, here it is. > >
> > Testing with a 32-bit snapshot (http://sourceforge.net/projects/mingw- > w64/files/Toolchains%20targetting%20Win32/Personal%20Builds/Experimental_Builds/4.9.0/threads- > win32/sjlj/) I see no problem to start with, but with a 64-bit > snapshot (http://sourceforge.net/projects/mingw- > w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/Experimental_Builds/4.9.0/threads- > win32/seh/) I get failures in op/range.t and op/numconvert.t without > the patch. > > Those failures are fixed by the patch. > > Why is the -fwrapv left out from the MINIWIN32_OBJ compilation? My > test results suggest that we don't currently have a problem with those > files (win32.c, win32sck.c, win32thread.c, fcrypt.c and win32io.c) but > surely it wouldn't hurt to compile them with it anyway to protect > against any future trouble? (Likewise the x2p files, although they > hardly matter since they're moving out of core in due course anyway.) > > Also, since the failure only happens on a 64-bit build, is it worth > only including -fwrapv when doing a 64-bit build? That way, users of > earlier versions of gcc that don't support -fwrapv aren't bothered by > this if they're only doing a 32-bit build anyway. (Or have I just been > lucky in not seeing any failure in a 32-bit build?)
The following also initializes GCCWRAPV better, so that it's only set to "define" by default for 4.3.0 or higer: GCCWRAPV *= $(shell for /f "delims=. tokens=1,2,3" %i in ('gcc -dumpversion') do @if "%i"=="4" (if "%j" geq "3" echo define) else if "%i" geq "5" (echo define))
Date: Tue, 22 Apr 2014 20:37:24 +0100
To: Tony Cook via RT <perlbug-followup [...] perl.org>
CC: "perl5-porters [...] perl.org" <perl5-porters [...] perl.org>
Subject: Re: [perl #121505] Can't build perl with gcc4.9 due to UB in sv.c
From: Steve Hay <steve.m.hay [...] googlemail.com>
On 22 April 2014 18:12, Steve Hay via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Tue Apr 22 05:10:18 2014, shay wrote:
>> On Mon Apr 21 16:05:34 2014, tonyc wrote:
>> > On Fri Apr 18 08:25:02 2014, shay wrote:
>> > > I don't see any new attachment or smoke-me branch for >> > > review/comment. >> > > Am I missing something?
>> > >> > Oops, here it is. >> >
>> >> Testing with a 32-bit snapshot (http://sourceforge.net/projects/mingw- >> w64/files/Toolchains%20targetting%20Win32/Personal%20Builds/Experimental_Builds/4.9.0/threads- >> win32/sjlj/) I see no problem to start with, but with a 64-bit >> snapshot (http://sourceforge.net/projects/mingw- >> w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/Experimental_Builds/4.9.0/threads- >> win32/seh/) I get failures in op/range.t and op/numconvert.t without >> the patch. >> >> Those failures are fixed by the patch. >> >> Why is the -fwrapv left out from the MINIWIN32_OBJ compilation? My >> test results suggest that we don't currently have a problem with those >> files (win32.c, win32sck.c, win32thread.c, fcrypt.c and win32io.c) but >> surely it wouldn't hurt to compile them with it anyway to protect >> against any future trouble? (Likewise the x2p files, although they >> hardly matter since they're moving out of core in due course anyway.) >> >> Also, since the failure only happens on a 64-bit build, is it worth >> only including -fwrapv when doing a 64-bit build? That way, users of >> earlier versions of gcc that don't support -fwrapv aren't bothered by >> this if they're only doing a 32-bit build anyway. (Or have I just been >> lucky in not seeing any failure in a 32-bit build?)
> > > The following also initializes GCCWRAPV better, so that it's only set to "define" by default for 4.3.0 or higer: > > GCCWRAPV *= $(shell for /f "delims=. tokens=1,2,3" %i in ('gcc -dumpversion') do @if "%i"=="4" (if "%j" geq "3" echo define) else if "%i" geq "5" (echo define)) > >
(I forgot: that probably needs putting in a GCC-specific section to avoid trouble for anyone using dmake with VC++. And the cross-compiler's gcc isn't called gcc -- it's x86_64-w64-mingw32-gcc.)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.5k
On Tue Apr 22 12:37:46 2014, shay wrote: Show quoted text
> On 22 April 2014 18:12, Steve Hay via RT <perlbug-followup@perl.org> > wrote:
> > On Tue Apr 22 05:10:18 2014, shay wrote:
> >> On Mon Apr 21 16:05:34 2014, tonyc wrote:
> >> > On Fri Apr 18 08:25:02 2014, shay wrote:
> >> > > I don't see any new attachment or smoke-me branch for > >> > > review/comment. > >> > > Am I missing something?
> >> > > >> > Oops, here it is. > >> >
> >> > >> Testing with a 32-bit snapshot > >> (http://sourceforge.net/projects/mingw- > >> w64/files/Toolchains%20targetting%20Win32/Personal%20Builds/Experimental_Builds/4.9.0/threads- > >> win32/sjlj/) I see no problem to start with, but with a 64-bit > >> snapshot (http://sourceforge.net/projects/mingw- > >> w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/Experimental_Builds/4.9.0/threads- > >> win32/seh/) I get failures in op/range.t and op/numconvert.t without > >> the patch. > >> > >> Those failures are fixed by the patch. > >> > >> Why is the -fwrapv left out from the MINIWIN32_OBJ compilation? My > >> test results suggest that we don't currently have a problem with > >> those > >> files (win32.c, win32sck.c, win32thread.c, fcrypt.c and win32io.c) > >> but > >> surely it wouldn't hurt to compile them with it anyway to protect > >> against any future trouble? (Likewise the x2p files, although they > >> hardly matter since they're moving out of core in due course > >> anyway.) > >> > >> Also, since the failure only happens on a 64-bit build, is it worth > >> only including -fwrapv when doing a 64-bit build? That way, users of > >> earlier versions of gcc that don't support -fwrapv aren't bothered > >> by > >> this if they're only doing a 32-bit build anyway. (Or have I just > >> been > >> lucky in not seeing any failure in a 32-bit build?)
> > > > > > The following also initializes GCCWRAPV better, so that it's only set > > to "define" by default for 4.3.0 or higer: > > > > GCCWRAPV *= $(shell for /f "delims=. tokens=1,2,3" %i in ('gcc > > -dumpversion') do @if "%i"=="4" (if "%j" geq "3" echo define) else if > > "%i" geq "5" (echo define)) > > > >
> > (I forgot: that probably needs putting in a GCC-specific section to > avoid trouble for anyone using dmake with VC++. And the > cross-compiler's gcc isn't called gcc -- it's x86_64-w64-mingw32-gcc.)
Oops, this ticket slipped my (alleged) mind. I've attached an updated patch: - uses the shell macro you provided to set a default GCCWRAPV, replacing gcc with $(CC) *after* CC has been set based on the GCCCROSS macro - compiles MINIWIN32_OBJ with $(MINIBUILDOPT) Tony
Subject: 0001-perl-121505-include-fwrapv-by-default-for-GCC-4.3-an.patch
From e0c717880fdaf205d43f66cd54ba672119129d22 Mon Sep 17 00:00:00 2001 From: Tony Cook <tony@develop-help.com> Date: Mon, 5 May 2014 10:22:25 +1000 Subject: [perl #121505] include -fwrapv by default for GCC 4.3 and later --- win32/makefile.mk | 19 +++++++++++++++++-- 1 files changed, 17 insertions(+), 2 deletions(-) diff --git a/win32/makefile.mk b/win32/makefile.mk index 99193a2..e005ee5 100644 --- a/win32/makefile.mk +++ b/win32/makefile.mk @@ -141,6 +141,12 @@ USE_LARGE_FILES *= define CCTYPE *= GCC # +# If you are using GCC, 4.3 or later by default we add the -fwrapv option. +# See https://rt.perl.org/Ticket/Display.html?id=121505 +# +#GCCWRAPV *= define + +# # If you are using Intel C++ Compiler uncomment this # #__ICC *= define @@ -408,6 +414,8 @@ INST_HTML = $(INST_TOP)$(INST_VER)\html .USESHELL : +MINIBUILDOPT *= + .IF "$(CCTYPE)" == "GCC" .IF "$(GCCCROSS)" == "define" @@ -420,6 +428,13 @@ LIB32 = $(ARCHPREFIX)ar rc IMPLIB = $(ARCHPREFIX)dlltool RSC = $(ARCHPREFIX)windres +GCCWRAPV *= $(shell for /f "delims=. tokens=1,2,3" %i in ('$(CC) -dumpversion') do @if "%i"=="4" (if "%j" geq "3" echo define) else if "%i" geq "5" (echo define)) + +.IF "$(GCCWRAPV)" == "define" +BUILDOPT += -fwrapv +MINIBUILDOPT += -fwrapv +.ENDIF + i = .i o = .o a = .a @@ -1139,10 +1154,10 @@ $(MINIDIR) : if not exist "$(MINIDIR)" mkdir "$(MINIDIR)" $(MINICORE_OBJ) : $(CORE_NOCFG_H) - $(CC) -c $(CFLAGS) -DPERL_EXTERNAL_GLOB -DPERL_IS_MINIPERL $(OBJOUT_FLAG)$@ ..\$(*B).c + $(CC) -c $(CFLAGS) $(MINIBUILDOPT) -DPERL_EXTERNAL_GLOB -DPERL_IS_MINIPERL $(OBJOUT_FLAG)$@ ..\$(*B).c $(MINIWIN32_OBJ) : $(CORE_NOCFG_H) - $(CC) -c $(CFLAGS) -DPERL_IS_MINIPERL $(OBJOUT_FLAG)$@ $(*B).c + $(CC) -c $(CFLAGS) $(MINIBUILDOPT) -DPERL_IS_MINIPERL $(OBJOUT_FLAG)$@ $(*B).c # -DPERL_IMPLICIT_SYS needs C++ for perllib.c # rules wrapped in .IFs break Win9X build (we end up with unbalanced []s unless -- 1.7.4.msysgit.0
Subject: Re: [perl #121505] Can't build perl with gcc4.9 due to UB in sv.c
CC: "perl5-porters [...] perl.org" <perl5-porters [...] perl.org>
To: Tony Cook via RT <perlbug-followup [...] perl.org>
Date: Tue, 6 May 2014 18:14:28 +0100
From: Steve Hay <steve.m.hay [...] googlemail.com>
On 5 May 2014 01:27, Tony Cook via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Tue Apr 22 12:37:46 2014, shay wrote:
>> On 22 April 2014 18:12, Steve Hay via RT <perlbug-followup@perl.org> >> wrote:
>> > On Tue Apr 22 05:10:18 2014, shay wrote:
>> >> On Mon Apr 21 16:05:34 2014, tonyc wrote:
>> >> > On Fri Apr 18 08:25:02 2014, shay wrote:
>> >> > > I don't see any new attachment or smoke-me branch for >> >> > > review/comment. >> >> > > Am I missing something?
>> >> > >> >> > Oops, here it is. >> >> >
>> >> >> >> Testing with a 32-bit snapshot >> >> (http://sourceforge.net/projects/mingw- >> >> w64/files/Toolchains%20targetting%20Win32/Personal%20Builds/Experimental_Builds/4.9.0/threads- >> >> win32/sjlj/) I see no problem to start with, but with a 64-bit >> >> snapshot (http://sourceforge.net/projects/mingw- >> >> w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/Experimental_Builds/4.9.0/threads- >> >> win32/seh/) I get failures in op/range.t and op/numconvert.t without >> >> the patch. >> >> >> >> Those failures are fixed by the patch. >> >> >> >> Why is the -fwrapv left out from the MINIWIN32_OBJ compilation? My >> >> test results suggest that we don't currently have a problem with >> >> those >> >> files (win32.c, win32sck.c, win32thread.c, fcrypt.c and win32io.c) >> >> but >> >> surely it wouldn't hurt to compile them with it anyway to protect >> >> against any future trouble? (Likewise the x2p files, although they >> >> hardly matter since they're moving out of core in due course >> >> anyway.) >> >> >> >> Also, since the failure only happens on a 64-bit build, is it worth >> >> only including -fwrapv when doing a 64-bit build? That way, users of >> >> earlier versions of gcc that don't support -fwrapv aren't bothered >> >> by >> >> this if they're only doing a 32-bit build anyway. (Or have I just >> >> been >> >> lucky in not seeing any failure in a 32-bit build?)
>> > >> > >> > The following also initializes GCCWRAPV better, so that it's only set >> > to "define" by default for 4.3.0 or higer: >> > >> > GCCWRAPV *= $(shell for /f "delims=. tokens=1,2,3" %i in ('gcc >> > -dumpversion') do @if "%i"=="4" (if "%j" geq "3" echo define) else if >> > "%i" geq "5" (echo define)) >> > >> >
>> >> (I forgot: that probably needs putting in a GCC-specific section to >> avoid trouble for anyone using dmake with VC++. And the >> cross-compiler's gcc isn't called gcc -- it's x86_64-w64-mingw32-gcc.)
> > Oops, this ticket slipped my (alleged) mind. > > I've attached an updated patch: > > - uses the shell macro you provided to set a default GCCWRAPV, replacing gcc with $(CC) *after* CC has been set based on the GCCCROSS macro > > - compiles MINIWIN32_OBJ with $(MINIBUILDOPT) >
I'm not sure I'd have left the commented-out GCCWRAPV option at the top of the file. Is it really something users would want to fiddle with? Other gcc-specific options are added to BUILDOPT later on with no mention in the "Build configuration" part of the file (-fno-strict-aliasing -mms-bitfields), so I'd be inclined to just put GCCWRAPV there. Would it also make sense to only do this ".IF "$(WIN64)" == "define""?
RT-Send-CC: perl5-porters [...] perl.org
On Tue May 06 10:14:50 2014, shay wrote: Show quoted text
> I'm not sure I'd have left the commented-out GCCWRAPV option at the > top of the file. Is it really something users would want to fiddle > with? Other gcc-specific options are added to BUILDOPT later on with > no mention in the "Build configuration" part of the file > (-fno-strict-aliasing -mms-bitfields), so I'd be inclined to just put > GCCWRAPV there.
A developer might want to disable it to track down the problems -fwrapv hides. In the Configure change we look for the -fno-wrapv option and disable -fwrapv if that (or -fsanitize=undefined) is present, so you can investigate undefined value issues that -fwrapv hides with: ./Configure -Accflags=-fsanitize=undefined ... Including the commented out entry documents it as something the developer can change, or supply on the command-line, like: dmake GCCWRAPV=undef BUILDOPTEXTRA=-fsanitize=undefined Show quoted text
> Would it also make sense to only do this ".IF "$(WIN64)" == "define""?
No, these optimizations aren't limited to 64-bit builds. Tony
From: Steve Hay <steve.m.hay [...] googlemail.com>
CC: "perl5-porters [...] perl.org" <perl5-porters [...] perl.org>
Subject: Re: [perl #121505] Can't build perl with gcc4.9 due to UB in sv.c
Date: Fri, 9 May 2014 14:38:00 +0100
To: Tony Cook via RT <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 1.1k
On 8 May 2014 05:42, Tony Cook via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Tue May 06 10:14:50 2014, shay wrote:
>> I'm not sure I'd have left the commented-out GCCWRAPV option at the >> top of the file. Is it really something users would want to fiddle >> with? Other gcc-specific options are added to BUILDOPT later on with >> no mention in the "Build configuration" part of the file >> (-fno-strict-aliasing -mms-bitfields), so I'd be inclined to just put >> GCCWRAPV there.
> > A developer might want to disable it to track down the problems -fwrapv hides. > > In the Configure change we look for the -fno-wrapv option and disable -fwrapv if that (or -fsanitize=undefined) is present, so you can investigate undefined value issues that -fwrapv hides with: > > ./Configure -Accflags=-fsanitize=undefined ... > > Including the commented out entry documents it as something the developer can change, or supply on the command-line, like: > > dmake GCCWRAPV=undef BUILDOPTEXTRA=-fsanitize=undefined >
>> Would it also make sense to only do this ".IF "$(WIN64)" == "define""?
> > No, these optimizations aren't limited to 64-bit builds. >
Ok, fair enough. This patch looks fine to me then.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 289b
On Fri May 09 06:38:26 2014, shay wrote: Show quoted text
> Ok, fair enough. This patch looks fine to me then.
Applied to blead as c8180b0692cef05079a2e250b3faccaca4592b10. I'll remove this ticket as a blocker rather than resolving it, since we need to go back and look for all the UB and fix them. Tony


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org