[m-rev.] for review: fix a bug in the string to int and uint overflow checks

[Zoltan Somogyi]

On Fri, 17 Jul 2026 17:07:47 +1000, Julien Fischer <jfischer at opturion.com> wrote: 
> We can almost certainly do better for the common bases performance-wise (and
> likely the generic case as well). I will look into doing that separately.

The obvious way to do it would be to check the base being in 2..36 not using
comparisons, but by using a switch, and having that switch also return
the precomputed value of maxint/base (for both 32 and 64 bits) as well.

> For example, with 32-bit ints consider the input string "5368709120" (in the
> base 10 case). At the last step of the conversion loop, we have N0 = 536870912
> and the final digit '0'. The true value of (10 * 536870912) + 0 is 5368709120,
> which does not fit in 32 bits. The multiplication wraps around, giving N =
> 1073741824.  Since 536870912 =< 1073741824, the overflow check succeeds, and
> the conversion incorrectly returns 1073741824 instead of failing.

Such examples would be MUCH more understandable if you added a comma
every third decimal place, to allow distinguishing three billion from thirty billion
without tedious digit counting.

> --- a/library/string.m
> +++ b/library/string.m
> @@ -6035,10 +6035,10 @@ base_positive_int_accumulator(Base) = Pred :-
> 
>  accumulate_int(Base, Char, N0, N) :-

I would rename this to either accumulate_positive_int
or accumulate_non_negative_int.

>      char.unsafe_base_digit_to_int(Base, Char, M),
> -    N = (Base * N0) + M,
> -    % Fail on overflow.
> -    % XXX depends on undefined behaviour
> -    N0 =< N.
> +    % Fail if Base * N0 + M would exceed max_int.
> +    % The division is safe since our caller sets Base to be in 2..36.
> +    N0 =< (max_int - M) `unchecked_quotient` Base,
> +    N = (Base * N0) + M.
> 
>  :- func base_negative_int_accumulator(int) = pred(char, int, int).
>  :- mode base_negative_int_accumulator(in) = out(pred(in, in, out) is semidet)
> @@ -6067,10 +6067,13 @@ base_negative_int_accumulator(Base) = Pred :-
> 
>  accumulate_negative_int(Base, Char, N0, N) :-
>      char.unsafe_base_digit_to_int(Base, Char, M),
> -    N = (Base * N0) - M,
> -    % Fail on overflow.
> -    % XXX depends on undefined behaviour
> -    N =< N0.
> +    % Fail if Base * N0 - M would be less than min_int.
> +    % We must use truncating division in the following check.
> +    % Flooring division (i.e. div) causes the test to succeed
> +    % for values of N0 for which the multiplication overflows.
> +    % The division is safe since our caller sets Base to be in 2..36.
> +    N0 >= (min_int + M) `unchecked_quotient` Base,
> +    N = (Base * N0) - M.

What guarantees do our target languages offer about truncating
vs flooring? If you know, it would help to mention them here.
 
I did not check the test cases, partly because the absence of commas
in large numbers makes it too hard :-(

However, the rest of the diff is fine.

Zoltan.