On Thu, Dec 09, 2021 at 08:19:27PM +0100, Ævar Arnfjörð Bjarmason wrote:
> Change the "int" type used by compute_assignment() to "intmax_t". On
> 64 bit systems this changes the overflow "die" added in the preceding
> commit (which before that was a segfault) to something that merely
> takes a very long time and a lot of memory to run.
>
> On my relatively beefy system this completes:
>
> git -P range-diff --creation-factor=50 origin/master...git-for-windows/main
>
> In around 300 seconds, with a reported max RSS of just under 18GB, but
> it does give you correct results for all ~50k commitsin that range.
So here you do what I think is the "real" fix. And at this point I
suspect that the overflow checks here:
> -static inline int cost_index(int *cost, int a, int b, int c)
> +static inline intmax_t cost_index(intmax_t *cost, intmax_t a, intmax_t b, intmax_t c)
> {
> - int r;
> + intmax_t r;
>
> if (INT_MULTIPLY_WRAPV(a, c, &r))
> - die(_("integer overflow in cost[%d + %d * %d] multiplication"), b, a, c);
> + die(_("integer overflow in cost[%"PRIuMAX" + %"PRIuMAX" * %"PRIuMAX"] multiplication"), b, a, c);
> if (INT_ADD_WRAPV(b, r, &r))
> - die(_("integer overflow in cost[%d + ((%d * %d) = %d)] addition"), b, a, c, r);
> + die(_("integer overflow in cost[%"PRIuMAX" + ((%"PRIuMAX" * %"PRIuMAX") = %"PRIuMAX")] addition"), b, a, c, r);
>
> return r;
cannot be triggered. We are indexing an array that is already limited to
size_t. So using that type should be sufficient (or else we have
problems even outside of overflow).
And for that reason I'd probably pick size_t (or ssize_t; you don't need
it in the hunk above, but presumably some of the other code cares about
signedness. And as I said earlier, it's effectively the same from a
protection standpoint).
-Peff
