Re: [Qemu-devel] [PATCH] accel/tcg/translate-all: expand cpu_restore_state retaddr check

[Richard Henderson <rth@twiddle.net>]

On 11/07/2017 07:53 PM, Peter Maydell wrote:
>> Then why call cpu_restore_state at all? We should be consistent as there
>> are plenty of places that do things like:
>>
>>     if (pc) {
>>         /* now we have a real cpu fault */
>>         cpu_restore_state(cs, pc);
>>     }
>>
>> I'm happy to make a 0 retaddr officially valid and actually document it
>> in exec-all.h. It's not like most callers even bother checking the
>> return code.

This is exactly the discussion that we had last time, and we did just that.

> Hmm, there's more places than I expected that do that "don't call
> if 0" check than I thought. Overall it seems better to me to officially
> allow the zero, rather than having lots of callsites that all have
> to remember to check.

... what we didn't do is go through and change all of the call sites to remove
the check for zero.

> Incidentally if retaddr is zero then
> (retaddr < (uintptr_t) tcg_init_ctx.code_gen_buffer)
> is always true and you don't need to explicitly check for zero, though
> it might be clearer to do so if we think we might change the rest
> of the condition in future.

Indeed, I was thinking

  retaddr - code_gen_buffer < code_gen_buffer_size

which works well with unsigned arithmetic.  And a large comment re zero.


r~