On 3/28/23 1:45 PM, Junio C Hamano wrote:

Patrick Steinhardt [off-list ref] writes:

quoted

In 80c928d947 (commit-graph: simplify compute_generation_numbers(),
2023-03-20), the code to compute generation numbers was simplified to
use the same infrastructure as is used to compute topological levels.
This refactoring introduced a bug where the generation numbers are
truncated when they exceed UINT32_MAX because we explicitly cast the
computed generation number to `uint32_t`. This is not required though:
both the computed value and the field of `struct commit_graph_data` are
of the same type `timestamp_t` already, so casting to `uint32_t` will
cause truncation.

This cast can cause us to miscompute generation data overflows:

    1. Given a commit with no parents and committer date
       `UINT32_MAX + 1`.

    2. We compute its generation number as `UINT32_MAX + 1`, but
       truncate it to `1`.

    3. We calculate the generation offset via `$generation - $date`,
       which is thus `1 - (UINT32_MAX + 1)`. The computation underflows
       and we thus end up with an offset that is bigger than the maximum
       allowed offset.

As a result, we'd be writing generation data overflow information into
the commit-graph that is bogus and ultimately not even required.

Fix this bug by removing the needless cast.

Signed-off-by: Patrick Steinhardt <redacted>
---

This commit applies on top of cbfe360b14 (commit-reach: add
tips_reachable_from_bases(), 2023-03-20), which has recently been merged
to next.

The patch is clearly explained and the change looks quite
straight-forward.  Derrick, Ack?

Yes, looks good. What a silly mistake, but thanks for going
the extra mile to introduce a test that will prevent it in
the future.

Thanks,
-Stolee