[PATCH] commit-graph: use timestamp_t for max parent generation accumulator

[PATCH] commit-graph: use timestamp_t for max parent generation accumulator

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

compute_reachable_generation_numbers() computes each commit's
generation as

    max(c->date, max(parent.generation)) + 1

by walking its parents and accumulating their generations into a
local

    uint32_t max_gen = 0;

while info->get_generation() returns timestamp_t and
compute_generation_from_max() already takes its max_gen parameter
as timestamp_t.  For v1 (topological levels) the narrowing is
harmless because GENERATION_NUMBER_V1_MAX is less than 2^30, but
for v2 (corrected committer dates) it silently truncates any
parent generation that does not fit in 32 bits, i.e. any parent
whose committer timestamp is at or beyond 2106-02-07 UTC
(>= 2^32).

The truncated max then causes child commits to end up with a
corrected committer date that matches the parent's instead of being
at least 1 higher.  The bad value gets written into the commit-graph
and causes problems later, and can be noticed by running `git
commit-graph verify`.

Widen the accumulator to timestamp_t.

This is solely an in-memory arithmetic fix with no on-disk format
change: the on-disk format already encodes timestamp_t values and
existing readers handle them unchanged.  This merely allows the code to
compute the correct value to write to disk.

The narrowing was introduced in 80c928d947c2 (commit-graph:
simplify compute_generation_numbers(), 2023-03-20), which rewired
v2 to use the shared compute_reachable_generation_numbers()
helper; the helper's local accumulator had been declared uint32_t
in the immediately preceding 368d19b0b7fa (commit-graph: refactor
compute_topological_levels(), 2023-03-20) when only v1 was using
it, where it was harmless.

Add a new test with a future-dated parent and a present-day child;
without the above fix, `git commit-graph verify` reports the
descendant's stored generation as below parent + 1.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
    commit-graph: use timestamp_t for max parent generation accumulator
    
    We found a few repositories in the wild with commits whose authors were
    apparently on a computer in the year 2120 when they recorded their
    commits. Apparently, in a century from now, some folks are going to have
    a really weird timezone as well (-13068837), though the timezone doesn't
    factor into this patch at all.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2148%2Fnewren%2Fcommit-graph-fix-ccd-uint32-truncation-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2148/newren/commit-graph-fix-ccd-uint32-truncation-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2148

 commit-graph.c                     | 2 +-
 t/t5328-commit-graph-64bit-time.sh | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/commit-graph.c b/commit-graph.c
index 9abe62bd5a..4b7156fd76 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1669,7 +1669,7 @@ static void compute_reachable_generation_numbers(
 			struct commit *current = list->item;
 			struct commit_list *parent;
 			int all_parents_computed = 1;
-			uint32_t max_gen = 0;
+			timestamp_t max_gen = 0;
 
 			for (parent = current->parents; parent; parent = parent->next) {
 				repo_parse_commit(info->r, parent->item);
diff --git a/t/t5328-commit-graph-64bit-time.sh b/t/t5328-commit-graph-64bit-time.sh
index d8891e6a92..bc651b69de 100755
--- a/t/t5328-commit-graph-64bit-time.sh
+++ b/t/t5328-commit-graph-64bit-time.sh
@@ -74,6 +74,15 @@ test_expect_success 'single commit with generation data exceeding UINT32_MAX' '
 	git -C repo-uint32-max commit-graph verify
 '
 
+test_expect_success 'descendant of commit with date exceeding UINT32_MAX' '
+	git init repo-uint32-max-descendant &&
+	test_commit -C repo-uint32-max-descendant \
+		--date "@4294967300 +0000" future-parent &&
+	test_commit -C repo-uint32-max-descendant present-day-child &&
+	git -C repo-uint32-max-descendant commit-graph write --reachable &&
+	git -C repo-uint32-max-descendant commit-graph verify
+'
+
 test_expect_success PERL_TEST_HELPERS 'reader notices out-of-bounds generation overflow' '
 	graph=.git/objects/info/commit-graph &&
 	test_when_finished "rm -rf $graph" &&

base-commit: 600fe743028cbfb640855f659e9851522214bc0b
-- 
gitgitgadget