From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53429) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1an4SU-0001mg-6y for qemu-devel@nongnu.org; Mon, 04 Apr 2016 09:22:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1an4SQ-0003WP-9o for qemu-devel@nongnu.org; Mon, 04 Apr 2016 09:22:22 -0400 References: <1459767918-796-1-git-send-email-sergey.fedorov@linaro.org> <5702567E.6060701@weilnetz.de> From: Sergey Fedorov Message-ID: <57026A7E.1000506@gmail.com> Date: Mon, 4 Apr 2016 16:22:06 +0300 MIME-Version: 1.0 In-Reply-To: <5702567E.6060701@weilnetz.de> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] tci: Fix build with no '-DNDEBUG' List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil , Sergey Fedorov , qemu-devel@nongnu.org Cc: QEMU Trivial , Peter Maydell On 04/04/16 14:56, Stefan Weil wrote: > Am 04.04.2016 um 13:05 schrieb Sergey Fedorov: >> From: Sergey Fedorov >> >> 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 >> Signed-off-by: Sergey Fedorov >> --- >> 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