gcc builds: reenable -Wstrict-overflow
ClosedPublic

Authored by rlibby on Sep 9 2017, 12:34 AM.

Details

Summary

Relegate -Wno-error=strict-overflow to the zic makefile, as zic is the
only thing that needs it, and even zic appears now to be fixed upstream.

Test Plan

make buildworld buildkernel with amd64-gcc (gcc 6.3.0)
make tinderbox

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
rlibby created this revision.Sep 9 2017, 12:34 AM
ngie added a comment.Sep 9 2017, 5:03 AM

When will zic(8) be fixed?

rlibby added a comment.Sep 9 2017, 7:13 AM
In D12284#254856, @ngie wrote:

When will zic(8) be fixed?

It appears to have been in upstream: ftp://ftp.iana.org/tz/code/zic.c (see oadd).

As for when we might sync with upstream next, I have no idea. Last sync appears to have been in 2010.

emaste added a comment.Sep 9 2017, 1:32 PM

It appears to have been in upstream: ftp://ftp.iana.org/tz/code/zic.c (see oadd).

As for when we might sync with upstream next, I have no idea. Last sync appears to have been in 2010.

We could also bring in that fix independently, in advance of a full update.

rlibby added a comment.Sep 9 2017, 6:12 PM

It appears to have been in upstream: ftp://ftp.iana.org/tz/code/zic.c (see oadd).

As for when we might sync with upstream next, I have no idea. Last sync appears to have been in 2010.

We could also bring in that fix independently, in advance of a full update.

We could, sort of. We could bring in the (now-out-of-date) fix from the 2012g version, before the signature of oadd changed. I don't think we can easily bring in the up-to-date code for just this.

I'm not sure it would be good to add to our (already significant) diff against the base. But here's what it would look like:

$ git diff
diff --git a/contrib/tzcode/zic/zic.c b/contrib/tzcode/zic/zic.c
index 00043b7940e7..39558b36a890 100644
--- a/contrib/tzcode/zic/zic.c
+++ b/contrib/tzcode/zic/zic.c
@@ -2484,37 +2484,27 @@ register char * cp;
 }
 
 static long
-oadd(t1, t2)
-const long     t1;
-const long     t2;
+oadd(const long t1, const long t2)
 {
-       register long   t;
-
-       t = t1 + t2;
-       if ((t2 > 0 && t <= t1) || (t2 < 0 && t >= t1)) {
+       if (t1 < 0 ? t2 < LONG_MIN - t1 : LONG_MAX - t1 < t2) {
                error(_("time overflow"));
                exit(EXIT_FAILURE);
        }
-       return t;
+       return t1 + t2;
 }
 
 static zic_t
-tadd(t1, t2)
-const zic_t    t1;
-const long     t2;
+tadd(const zic_t t1, const long t2)
 {
-       register zic_t  t;
-
        if (t1 == max_time && t2 > 0)
                return max_time;
        if (t1 == min_time && t2 < 0)
                return min_time;
-       t = t1 + t2;
-       if ((t2 > 0 && t <= t1) || (t2 < 0 && t >= t1)) {
+       if (t1 < 0 ? t2 < min_time - t1 : max_time - t1 < t2) {
                error(_("time overflow"));
                exit(EXIT_FAILURE);
        }
-       return t;
+       return t1 + t2;
 }
 
 /*

Personally I'd rather just move the warning suppression for now as it maintains the status quo and avoids making life harder for whoever eventually syncs this. Thoughts?

rlibby edited the test plan for this revision. (Show Details)Sep 13 2017, 7:06 PM

@emaste suggested out of band to verify with a universe build. I did a make tinderbox and it succeeded.

emaste accepted this revision.Sep 13 2017, 7:58 PM
This revision is now accepted and ready to land.Sep 13 2017, 7:58 PM
This revision was automatically updated to reflect the committed changes.