Re: [Qemu-devel] [PATCH v2 04/11] translate: downgrade IRQ BQL asserts to tcg_debug_assert

[Paolo Bonzini]

On 03/03/2017 11:08, Peter Maydell wrote:
> On 2 March 2017 at 19:53, Alex Bennée <address@hidden> wrote:
>> While on MTTCG hosts it is very important that updates to
>> cpu->interrupt_request are protected by the BQL not all guests have
>> been converted to the correct locking yet. As a result we are seeing
>> breaking on non-MTTCG enabled guests in production builds.
>>
>> The locking in the guests needs to be fixed but while running single
>> threaded they will continue to work. By moving the asserts to
>> tcg_debug_asserts() they will still be useful during conversion
>> work (much like the existing assert_memory_lock/assert_tb_lock
>> asserts).
>>
>> Signed-off-by: Alex Bennée <address@hidden>
>> ---
>>  translate-all.c    | 2 +-
>>  translate-common.c | 3 ++-
>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/translate-all.c b/translate-all.c
>> index 9bac061c9b..7ee273410d 100644
>> --- a/translate-all.c
>> +++ b/translate-all.c
>> @@ -1928,7 +1928,7 @@ void dump_opcount_info(FILE *f, fprintf_function 
>> cpu_fprintf)
>>
>>  void cpu_interrupt(CPUState *cpu, int mask)
>>  {
>> -    g_assert(qemu_mutex_iothread_locked());
>> +    tcg_debug_assert(qemu_mutex_iothread_locked());
> 
> If CONFIG_DEBUG_TCG isn't enabled then tcg_debug_assert()
> turns into "if (!(X)) { __builtin_unreachable(); }", which
> means that instead of asserting we now run straight
> into compiler undefined behaviour, don't we?
> If what we want is "don't actually check this condition in
> the non-tcg-debug config" then we should do something
> that means we don't actually check the condition...

I think we should fix it for good and leave this as a hard assertion.

Paolo