Re: [PATCH] t4212: handle systems with post-apocalyptic gmtime

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 28, 2014, 16:41 UTC
Message-ID: <xmqqd2h6cm26.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140327224837.GB32434@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 8 quoted lines

>> 9999999999999999 Sat Jan 25 10:46:39 316889355 -0700
>> 99999999999999999 Wed Sep 6 02:46:39 -1126091476 -0700
>> 999999999999999999 Thu Oct 24 18:46:39 1623969404 -0700
>
> Thanks. Given the value where it fails, it kind of looks like there is
> some signed 32-bit value at work (~300 million years is OK, but 10 times
> that, rather than yielding ~3 billion, gets us -1 billion). Perhaps
> tm.tm_year is 32-bit.

That is what it looks like to me, too. It makes me wonder if some other platforms may have similar breakage using 16-bit signed tm_year and how such a breakage would look like, though ;-)

Show 8 quoted lines

> So what do we want to do? I think the options are:
>
>   1. Try to guess when we have a bogus timestamp value with an arbitrary
>      cutoff like "greater than 1 million years from now" (and enforce it
>      via time_t seconds, and avoid gmtime entirely). That is made-up and
>      arbitrary, but it also is sufficiently far that it won't ever
>      matter, and sufficiently close that any gmtime should behave
>      sensibly with it.

I think that two assumptions here are that

 (1) we would be able to find a single insane value (like 3 billion
     years from now expressed in time_t seconds) the test script
     would be able to feed and expect it to fail on all platforms we
     care about, even though how exactly that value causes various
     platforms fail may be different (glibc may give us a NULL from
     gmtime, FreeBSD may leave their own sentinel in gmtime we can
     recognize, and some others may simply wrap around the years);
     and that

 (2) the only other class of failure mode we haven't considered
     bevore Charles's report is tm_year wrapping around 32-bit
     signed int.

Offhand, the three possible failure modes this thread identified sounds to me like the only plausible ones, and I think the best way forward might be to

 - teach the "is the result sane, even though we may have got a
   non-NULL from gmtime?  otherwise let's signal a failure by
   replacing it with a known sentinel value" codepath the new
   failure mode Charles's report suggests---if we feed a positive
   timestamp and gmtime gave us back a tm_year+1900 < 0, that is
   certainly an overflow; and

 - Use that 3-billion years timestamp from Charles's report in the
   test and make sure we convert it to the known sentinel value.

perhaps?

>   2. Accept that we can't guess at every broken gmtime's output, and
>      just loosen the test to make sure we don't segfault.

Of course that is a simpler option, but we may have identified all plausible failure modes, in which case we can afford to go with your original plan to validate that we not just avoid segfaulting on one of the three failure modes from gmtime, but also cover the other two failure modes and signal any of them with a sentinel. That way may allow us to identify the fourth failure mode we haven't anticipated.