Am 13.06.19 um 13:49 schrieb Johannes Schindelin via GitGitGadget:
> From: Johannes Schindelin <johannes.schindelin@xxxxxx>
>
> The `labs()` function operates, as the initial `l` suggests, on `long`
> parameters. However, in `config.c` we tried to use it on values of type
> `intmax_t`.
>
> This problem was found by GCC v9.x.
>
> To fix it, let's just "unroll" the function (i.e. negate the value if it
> is negative).
There's also imaxabs(3).
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
> config.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/config.c b/config.c
> index 296a6d9cc4..01c6e9df23 100644
> --- a/config.c
> +++ b/config.c
> @@ -869,9 +869,9 @@ static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
> errno = EINVAL;
> return 0;
> }
> - uval = labs(val);
> + uval = val < 0 ? -val : val;
> uval *= factor;
> - if (uval > max || labs(val) > uval) {
> + if (uval > max || (val < 0 ? -val : val) > uval) {
> errno = ERANGE;
> return 0;
> }
So this check uses unsigned arithmetic to find out if the multiplication
overflows, right? Let's say value is "4G", then val will be 4 and
factor will be 2^30. Multiplying the two yields 2^32. On a 32-bit
system this will wrap around to 0, so that's what we get for uval there.
The range check will then pass unless max is negative, which is wrong.
This behavior was not introduced by your patch, of course.
We could fix it by using the macro unsigned_mult_overflows():
uval = imaxabs(val);
if (unsigned_mult_overflows(uval, factor) ||
uval * factor > max) {
errno = ERANGE;
return 0;
}
René
