Compute UNIX timestamps in Long so they do not wrap past 2038 - #27
Conversation
to-unix-timestamp multiplied the day count by DAY in 32-bit Int, so every instant past 2038-01-19 03:14:07 overflowed: 2100-01-01 came out as -192522496 and 2262-04-11 as 633351808. from-unix-timestamp divided the same way, so the wrapped value decoded to a wrong date rather than being rejected, and Int could not even express the timestamps in question. Callers that floor a negative result to zero -- llm's Retry-After deadline is one -- silently lost the wait entirely. to-unix-timestamp now returns Long and from-unix-timestamp takes one. The widening happens at the multiplication; DAY/HOUR/MINUTE/SECOND stay Int, since the time-of-day part is bounded by 86399 and only the day count can overflow. The timezone delta is already a Long, so subtracting it no longer narrows through Long.to-int. Tests cover both directions one second either side of the 32-bit range (2038-01-19 03:14:08 and 1901-12-13 20:45:51), 2100-01-01, 2262-04-11 and a round-trip past 2038. All eight fail with the Int arithmetic restored.
There was a problem hiding this comment.
Build & Tests
carp -x test/time.carp at a511342 on this armhf Pi — 318 passed, 0
failed, exit code read from the unpiped command. CI green on both legs. Fresh
angler (built from 185a9a2, so the new byte-offset-as-char-index rule is
live) is clean over time.carp and test/time.carp; carp-fmt --check clean
over both. carp -x gendocs.carp leaves the tree clean, and docs/index.html
is still byte-identical to docs/time_index.html. First review round.
Every before-value in the body is exact. I compiled the pre-PR
to-unix-timestamp/from-unix-timestamp beside the new ones in one binary
(reopening defmodule Datetime to reach EPOCH-ORDINAL and DAY):
| old | new | |
|---|---|---|
2038-01-19 03:14:07 |
2147483647 | 2147483647 |
2038-01-19 03:14:08 |
-2147483648 | 2147483648 |
2100-01-01 00:00:00 |
-192522496 | 4102444800 |
2262-04-11 00:00:00 |
633351808 | 9223286400 |
1901-12-13 20:45:51 |
+2147483647 | -2147483649 |
and decoding that wrapped 2100 value through the old from-unix-timestamp
gives 1963-11-25 17:31:44 — the "plausible-looking wrong date" the body
describes, confirmed rather than asserted. The last row is worth noting: a
past date wrapped to positive Int.MAX, which is the same bug read from the
other end and which the body does not mention.
One thing the test helpers get right that is easy to get wrong here: epoch-at
and at build their expected values with Long.from-int and arithmetic rather
than with literals. This 32-bit box folds 4102444800l to -192522496 at
compile time, so the same assertions written with a plain literal would have
been vacuous locally while passing on CI.
Findings
1. The caller sweep predates the other PR that is open right now
The body says the only caller in the org outside this repo's tests is
http/http.carp:176. That is true of every main, but llm PR #19 — open, at
head f16565a — added llm/llm.carp:264-265:
(let [delta (- (Datetime.to-unix-timestamp d)
(Datetime.to-unix-timestamp now))]
(if (and (<= delta 0) (> (Datetime.to-ordinal d) (Datetime.to-ordinal now)))
Int.MAX
(max 0 delta)))
Under this change delta becomes a Long and is then compared and returned
against Int.MAX, so seconds-until stops compiling the moment llm picks up
the new time. llm/test/llm.carp:8 has (Datetime.from-unix-timestamp 0)
and wants the 0l too.
Nothing breaks today — the chain is llm -> http-client@0.5.4 ->
http@0.4.2 -> time@0.5.3, so all three are pinned behind a release. But the
two open PRs are the same bug from opposite ends: #19 works around the wrap
locally and this fixes the root, and #19's guard becomes (correctly) dead code
once this lands. Worth one line in the body so the version bump does not
surprise whoever does it.
2. Widening the parameter to Long opens a domain that aborts
from-unix-timestamp narrows the day count straight back to Int
(time.carp:396):
days (Long.to-int (/ ts day-secs))
With the old Int parameter days was bounded to about +/-24855 and
from-ordinal could never see a non-positive ordinal. With a Long parameter
it can. Measured on this branch:
| ts | result |
|---|---|
| -62135683200 (one day before 0001-01-01) | 1-0-0 — month 0, day 0 |
| -62136288000 | 1-0--7 — day -7 |
| -62138880000 | SIGABRT, Array_unsafe_nth: Assertion 'n >= 0' failed |
The abort belongs to from-ordinal, which this PR does not touch and which
aborts the same way on main for (Datetime.from-ordinal -100) — so this is a
new door onto an old room rather than a new defect. It is also unreachable from
anything in the org. But the signature now says "any Long" and the docstring
says nothing about a range, so a sentence there (or a Maybe) would close it
honestly. Not a blocker.
Nothing else. The fix is at the root rather than at a symptom, Timezone.delta
was already Long so dropping the Long.to-int around it is a straight
simplification, and Datetime.diff carrying the same overflow is disclosed as
a follow-up rather than smuggled in.
Verdict: merge
Correct at the root, tests are honest and platform-aware, and both findings are
body/doc edits rather than code changes — but finding 1 is worth writing down
before the time bump reaches llm.
Datetime.to-unix-timestampcomputed(* days DAY)in 32-bitInt, so everyinstant past 2038-01-19 03:14:07 overflowed, and
from-unix-timestampdividedthe same way, so a wrapped value decoded to a plausible-looking wrong date
instead of being rejected.
Intcould not even express the timestamps inquestion, so a caller had no way to work around it.
Measured on
masterbefore the change:That is the same failure you hit downstream while reviewing carpentry-org/llm#19:
parse-retry-after-atsubtracts two of these and floors at zero, so anHTTP-date past 2038 yields a negative difference and the wait collapses to
0.What changed
to-unix-timestampreturnsLong;from-unix-timestamptakes one. Thewidening happens at the multiplication rather than on the constants — the
time-of-day part is bounded by 86399 and only the day count can overflow, so
DAY/HOUR/MINUTE/SECONDstayIntand the rest of the file is untouched.Subtracting the timezone offset no longer narrows through
Long.to-int, sinceTimezone.deltawas already aLong.A straight widening rather than a parallel
-longAPI: the only caller anywherein the org outside this repo's own tests and docs is
http/http.carp:176,(Datetime.from-unix-timestamp 0), which becomes0l.llmcallsto-unix-timestamptwice inparse-retry-after-atand will want aLong.to-intat the clamp when it picks this up — that is the site the bug wasfound at, and it needs the wider value to compute the right answer anyway.
Checked, not changed
time.carproutes through these two.strftimehas no%sdirective,Datetime.nowreadsstruct tmfrom the OS, and there are noInstantconversions in the library.Datetime.diffhas the sameIntoverflow in(* (- ord-a ord-b) DAY), butit needs a span over ~68 years to reach it and it feeds
Duration, whoseseconds-field isInt. Widening it is a bigger, separable change; happy todo it as a follow-up if you want it.
Tests
Eight new assertions in
test/time.carp, both directions:Int.MAX + 1and 1901-12-13 20:45:51 is exactlyInt.MIN - 1All eight fail (and only those eight) when the
Intarithmetic is put back withthe signatures left
Long, so they pin the widening itself rather than thetypes. Existing call sites in the suite gained
lsuffixes; the suite is 318passing, 0 failing, run with
-Werroras CI does.carp-fmt --checkandanglerare clean (angler built from HEAD, since its byte-offset rule postdatesmy local binary), and
docs/Datetime.htmlis regenerated — the only diff is thetwo signatures.
The expected values are assembled from parts under 2^31 rather than written as
wide
Longliterals, following the existingnanoshelper: on a 32-bit host thecompiler folds an oversized
Longliteral, which is also how I verified this onarmhf.
Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.