Re: [Qemu-trivial] [PATCH] tci: Fix build with no '-DNDEBUG'

[Sergey Fedorov <serge.fdrv@gmail.com>]

On 04/04/16 14:56, Stefan Weil wrote:
> Am 04.04.2016 um 13:05 schrieb Sergey Fedorov:
>> From: Sergey Fedorov <serge.fdrv@gmail.com>
>>
>> assert() always evaluates its argument so there's no need to #ifdef the
>> definitions which is only used for assert(). Actually, doing so
>> generates a compilation warning which is treated as an error in QEMU
>> build by default. Let compiler sort out and eliminate unnecessary
>> local variables.
>>
>> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
>> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
>> ---
>>  tci.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/tci.c b/tci.c
>> index 7cbb39ed4b6a..d709e008f3f9 100644
>> --- a/tci.c
>> +++ b/tci.c
>> @@ -472,10 +472,8 @@ uintptr_t tcg_qemu_tb_exec(CPUArchState *env, uint8_t *tb_ptr)
>>  
>>      for (;;) {
>>          TCGOpcode opc = tb_ptr[0];
>> -#if !defined(NDEBUG)
>>          uint8_t op_size = tb_ptr[1];
>>          uint8_t *old_code_ptr = tb_ptr;
>> -#endif
>>          tcg_target_ulong t0;
>>          tcg_target_ulong t1;
>>          tcg_target_ulong t2;
>
> This patch should not be applied.

Oops, you're right, it's a really silly patch.

>
> From the Linux man page for assert: "the macro assert() generates no code".
>
> Which variant of the assert macro evaluates its argument even when NDEBUG
> is defined? 

I'm not sure of how I got this misunderstanding :)

> On which system with which configuration did you see the
> problem?

I get the following error messages when I compile on Ubuntu 14.04.4 LTS.

$ /home/sergey/projects/qemu/configure
--target-list=x86_64-linux-user,x86_64-softmmu --enable-tcg-interpreter
$ make
...
/home/sergey/projects/qemu/tci.c: In function ‘tcg_qemu_tb_exec’:
/home/sergey/projects/qemu/tci.c:528:25: error: ‘old_code_ptr’
undeclared (first use in this function)
             assert(tb_ptr == old_code_ptr + op_size);
                         ^
/home/sergey/projects/qemu/tci.c:528:25: note: each undeclared
identifier is reported only once for each function it appears in
/home/sergey/projects/qemu/tci.c:528:40: error: ‘op_size’ undeclared
(first use in this function)
             assert(tb_ptr == old_code_ptr + op_size);
                                        ^
make[1]: *** [tci.o] Error 1

>
> There is indeed a regression in the current code. Commit
> d38ea87ac54af64ef611de434d07c12dc0399216 added an include statement
> which includes assert.h before NDEBUG is defined. This is wrong and
> needs a fix. Could you please try tci.c starting like this?
>
> /* Defining NDEBUG disables assertions (which makes the code faster). */
> #if !defined(CONFIG_DEBUG_TCG) && !defined(NDEBUG)
> # define NDEBUG
> #endif
>
> #include "qemu/osdep.h"
>

Doing so really helps. So what are the plans of how to fix the problem?

Thanks,
Sergey