Re: [PATCH] hw/timer: fix int underflow

[Peter Maydell]

On Tue, 14 Jan 2025 at 06:41, Дмитрий Фролов <frolov@swemel.ru> wrote:
>
> Hello, Peter.
> I beg a pardon, but I guess, we have a misunderstanding here.
>
> The problem is that comparison "if (limit < 0)" will never
> be true. Thus, "true" branch is unreachable. According to
> the comment below, it was assumed that "limit" may be
> negative, and this means, that "QEMU is running too slow...".
>
> "limit" is declared as "long long" and it is initialized
> with diff of two unsigned values:
> "timeout - imx_gpt_update_count(s)".
> Unsigned subtraction will never give a signed result!
> if timeout > imx_gpt_update_count(s), the result will be > 0.
> if timeout < imx_gpt_update_count(s), the result will also
> be > 0 (underflow). Then, actually, this (positive) result
> will be implicitly casted to "long long" and assigned to
> "limit". This makes no sense!
>
> So, to my opinion, explicit cast to "long long" is necessary
> here to get the expected behavior.

I wasn't saying the existing code was necessarily correct,
or that your proposed change was necessarily wrong.
I was saying your patch didn't come with any analysis of
what the actual hardware behaviour is, which is
how you would determine whether the fix you propose
is the correct one, or if it should be some other
change instead. (Some of my response was trying to
provide some of that analysis.)

thanks
-- PMM