From mboxrd@z Thu Jan 1 00:00:00 1970 From: Aurelien Jarno Subject: Re: Silence compiler warning in arch/x86/kvm/emulate.c Date: Fri, 19 Feb 2016 17:45:48 +0100 Message-ID: <20160219164548.GA28630@aurel32.net> References: <142668.1440884956@turing-police.cc.vt.edu> <20160219111139.GA22041@aurel32.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii To: Valdis Kletnieks , Gleb Natapov , Paolo Bonzini , x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Aruna Hewapathirane Return-path: Received: from hall.aurel32.net ([195.154.112.97]:33496 "EHLO hall.aurel32.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751290AbcBSQqN (ORCPT ); Fri, 19 Feb 2016 11:46:13 -0500 Content-Disposition: inline In-Reply-To: <20160219111139.GA22041@aurel32.net> Sender: kvm-owner@vger.kernel.org List-ID: On 2016-02-19 12:11, Aurelien Jarno wrote: > On 2015-08-29 17:49, Valdis Kletnieks wrote: > > Compiler warning: > > > > CC [M] arch/x86/kvm/emulate.o > > arch/x86/kvm/emulate.c: In function "__do_insn_fetch_bytes": > > arch/x86/kvm/emulate.c:814:9: warning: "linear" may be used uninitialized in this function [-Wmaybe-uninitialized] > > > > GCC is smart enough to realize that the inlined __linearize may return before > > setting the value of linear, but not smart enough to realize the same > > X86EMU_CONTINUE blocks actual use of the value. However, the value of > > 'linear' can only be set to one value, so hoisting the one line of code > > upwards makes GCC happy with the code. > > > > Reported-by: Aruna Hewapathirane > > Tested-by: Aruna Hewapathirane > > Signed-off-by: Valdis Kletnieks > > > > --- a/arch/x86/kvm/emulate.c.dist 2015-08-11 14:10:05.366061993 -0400 > > +++ b/arch/x86/kvm/emulate.c 2015-08-29 13:43:13.014163958 -0400 > > @@ -650,6 +650,7 @@ static __always_inline int __linearize(s > > u16 sel; > > > > la = seg_base(ctxt, addr.seg) + addr.ea; > > + *linear = la; > > *max_size = 0; > > switch (mode) { > > case X86EMUL_MODE_PROT64: > > @@ -693,7 +694,6 @@ static __always_inline int __linearize(s > > } > > if (insn_aligned(ctxt, size) && ((la & (size - 1)) != 0)) > > return emulate_gp(ctxt, 0); > > - *linear = la; > > return X86EMUL_CONTINUE; > > bad: > > if (addr.seg == VCPU_SREG_SS) > > > > Unfortunately this patch broke GNU/Hurd when running under KVM. It fails > to boot almost immediately. I haven't debug it more, but it looks like > *linear should not always be written. Actually the same patch with a bit more context shows the issue: > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index e7a4fde..b372a75 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -647,12 +647,13 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, > bool usable; > ulong la; > u32 lim; > u16 sel; > > la = seg_base(ctxt, addr.seg) + addr.ea; > + *linear = la; The assignation is moved here... > *max_size = 0; > switch (mode) { > case X86EMUL_MODE_PROT64: > if (is_noncanonical_address(la)) > goto bad; > > @@ -690,13 +691,12 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt, > } > la &= (u32)-1; ... while the value of la might be modified in between. > break; > } > if (insn_aligned(ctxt, size) && ((la & (size - 1)) != 0)) > return emulate_gp(ctxt, 0); > - *linear = la; > return X86EMUL_CONTINUE; > bad: > if (addr.seg == VCPU_SREG_SS) > return emulate_ss(ctxt, 0); > else > return emulate_gp(ctxt, 0); One possibility would be to assign it both at the beginning of the function and at the original location should fix the bug and prevent GCC to issue a warning. Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net