From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57937) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bwcMZ-0006Oh-0g for qemu-devel@nongnu.org; Tue, 18 Oct 2016 17:55:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bwcMV-0000ca-QV for qemu-devel@nongnu.org; Tue, 18 Oct 2016 17:55:59 -0400 Received: from mail-wm0-x243.google.com ([2a00:1450:400c:c09::243]:33129) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1bwcMV-0000br-Iv for qemu-devel@nongnu.org; Tue, 18 Oct 2016 17:55:55 -0400 Received: by mail-wm0-x243.google.com with SMTP id f193so1308827wmg.0 for ; Tue, 18 Oct 2016 14:55:55 -0700 (PDT) Sender: Paolo Bonzini References: <20161018145620.20658-1-bobby.prani@gmail.com> <20161018145620.20658-2-bobby.prani@gmail.com> From: Paolo Bonzini Message-ID: Date: Tue, 18 Oct 2016 23:55:49 +0200 MIME-Version: 1.0 In-Reply-To: <20161018145620.20658-2-bobby.prani@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] translate-all: Use proper type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pranith Kumar , Peter Crosthwaite , Richard Henderson , "open list:Overall" On 18/10/2016 16:56, Pranith Kumar wrote: > gcc does not warn about the wrong type since it is a void pointer > which can be cast to any type. > > Signed-off-by: Pranith Kumar > --- > translate-all.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/translate-all.c b/translate-all.c > index 8ca393c..c77470a 100644 > --- a/translate-all.c > +++ b/translate-all.c > @@ -412,7 +412,7 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, int alloc) > > /* Level 2..N-1. */ > for (i = V_L1_SHIFT / V_L2_BITS - 1; i > 0; i--) { > - void **p = atomic_rcu_read(lp); > + void *p = atomic_rcu_read(lp); Wrong; you can see below that p is initialized with p = g_new0(void *, V_L2_SIZE); so it must be a pointer to "void *". You are introducing exactly the bug that is mentioned in the commit message, and it would have screwed up this statement: lp = p + ((index >> (i * V_L2_BITS)) & (V_L2_SIZE - 1)); because it would then omit the multiplication of the RHS by sizeof(void *). How did you test the patch? Coverity would have caught this, but please be more careful. Thanks, Paolo > if (p == NULL) { > if (!alloc) { >