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

[Peter Maydell]

On 3 March 2017 at 11:05, Alex Bennée <address@hidden> wrote:
> According to the commit that added it
> (c552d6c038f7cf4058d1fd5987118ffd41e0e050) it is meant to be a hint to
> the compiler. Reading the GCC notes however seems to contradict that.
>
> FWIW I did test it in both builds and we do used tese for a bunch of
> other asserts and they haven't blown up.
>
>> 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...
>
> Hmm:
>
> 28      intptr_t qemu_real_host_page_mask;
> 29
> 30      #ifndef CONFIG_USER_ONLY
> 31      /* mask must never be zero, except for A20 change call */
> 32      static void tcg_handle_interrupt(CPUState *cpu, int mask)
> 33      {
> 34          int old_mask;
> 35          tcg_debug_assert(qemu_mutex_iothread_locked());
> 36
> 37          old_mask = cpu->interrupt_request;
> Line 34 of "/home/alex/lsrc/qemu/qemu.git/translate-common.c" is at address 
> 0x24db0a <tcg_handle_interrupt+10> but contains no code.
> Line 35 of "/home/alex/lsrc/qemu/qemu.git/translate-common.c" starts at 
> address 0x24db0a <tcg_handle_interrupt+10> and ends at 0x24db0f 
> <tcg_handle_interrupt+15>.
> Line 36 of "/home/alex/lsrc/qemu/qemu.git/translate-common.c" is at address 
> 0x24db0f <tcg_handle_interrupt+15> but contains no code.
> Line 37 of "/home/alex/lsrc/qemu/qemu.git/translate-common.c" starts at 
> address 0x24db0f <tcg_handle_interrupt+15> and ends at 0x24db15 
> <tcg_handle_interrupt+21>.
>    0x24db0a <tcg_handle_interrupt+10>:  callq  0x27a570 
> <qemu_mutex_iothread_locked>
>    0x24db0f <tcg_handle_interrupt+15>:  mov    0xa8(%rbx),%ebp
>    0x24db15 <tcg_handle_interrupt+21>:  mov    %r12d,%eax
>    0x24db18 <tcg_handle_interrupt+24>:  mov    %rbx,%rdi
>    0x24db1b <tcg_handle_interrupt+27>:  or     %ebp,%eax
>    0x24db1d <tcg_handle_interrupt+29>:  mov    %eax,0xa8(%rbx)
>    0x24db23 <tcg_handle_interrupt+35>:  callq  0x27a530 <qemu_cpu_is_self>
>
> It certainly looks as though it makes the call but ignores the result?

That is one permitted implementation of the undefined behaviour,
certainly. Not the only one, though. In particular you're telling
the compiler's optimization passes "this can never be reached"
which can result in the optimizer making significantly different
decisions (especially if it manages to inline things or it can
look "inside" the condition being asserted here, etc etc).

thanks
-- PMM