Re: Bug in drivers/kvm/vmx.c inject_rmode_irq()?

Re: Bug in drivers/kvm/vmx.c inject_rmode_irq()?

[Eric Sesterhenn / Snakebyte]

* Avi Kivity (avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org) wrote:
> Eric Sesterhenn / Snakebyte wrote:
> >
> >i was testing the gcc 4.3 against the latest git kernel, and got a
> >warning in your code (using -Wstrict-overflow=1)
> >
> >drivers/kvm/vmx.c: In function 'inject_rmode_irq':
> >drivers/kvm/vmx.c:1193: warning: assuming signed overflow does not occur
> >when assuming that (X - c) > X is always false
> >
> >The problem is basically that gcc 4.3 handles integer overflows
> >different, when using -O2 and -Os, the code triggering this is the
> >following:
> >
> >if (sp > ss_limit || sp - 6 > sp) {
> >
> >I am not completely sure, but wouldnt a check for 
> >( sp > ss_limit || sp > 6 ) be enough?
> >
> hmm.  sp is declared as u16, which is unsigned.  I don't see how gcc can 
> promote it to a signed type, unless I'm misremembering C's promotion rules.
> 
> Anyway, it could just be coded as
> 
>    if (sp > ss_limit || sp < 6)
> 
> and achieve the same effect.
> 

Since 4.2 gcc might decide that overflows can never occur, and optimize
away this check, see http://gcc.gnu.org/gcc-4.2/changes.html
Lets make sure we still check this.

Signed-off-by: Eric Sesterhenn <snakebyte-Mmb7MZpHnFY@public.gmane.org>

--- linux-2.6/drivers/kvm/vmx.c.orig	2007-04-09 17:03:22.000000000 +0200
+++ linux-2.6/drivers/kvm/vmx.c	2007-04-09 17:03:50.000000000 +0200
@@ -1190,7 +1190,7 @@ static void inject_rmode_irq(struct kvm_
 	u16 sp =  vmcs_readl(GUEST_RSP);
 	u32 ss_limit = vmcs_read32(GUEST_SS_LIMIT);
 
-	if (sp > ss_limit || sp - 6 > sp) {
+	if (sp > ss_limit || sp < 6 ) {
 		vcpu_printf(vcpu, "%s: #SS, rsp 0x%lx ss 0x%lx limit 0x%x\n",
 			    __FUNCTION__,
 			    vmcs_readl(GUEST_RSP),


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: Bug in drivers/kvm/vmx.c inject_rmode_irq()?

[Avi Kivity]

Eric Sesterhenn / Snakebyte wrote:
> * Avi Kivity (avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org) wrote:
>   
>> Eric Sesterhenn / Snakebyte wrote:
>>     
>>> i was testing the gcc 4.3 against the latest git kernel, and got a
>>> warning in your code (using -Wstrict-overflow=1)
>>>
>>> drivers/kvm/vmx.c: In function 'inject_rmode_irq':
>>> drivers/kvm/vmx.c:1193: warning: assuming signed overflow does not occur
>>> when assuming that (X - c) > X is always false
>>>
>>> The problem is basically that gcc 4.3 handles integer overflows
>>> different, when using -O2 and -Os, the code triggering this is the
>>> following:
>>>
>>> if (sp > ss_limit || sp - 6 > sp) {
>>>
>>> I am not completely sure, but wouldnt a check for 
>>> ( sp > ss_limit || sp > 6 ) be enough?
>>>
>>>       
>> hmm.  sp is declared as u16, which is unsigned.  I don't see how gcc can 
>> promote it to a signed type, unless I'm misremembering C's promotion rules.
>>
>> Anyway, it could just be coded as
>>
>>    if (sp > ss_limit || sp < 6)
>>
>> and achieve the same effect.
>>
>>     
>
> Since 4.2 gcc might decide that overflows can never occur, and optimize
> away this check, see http://gcc.gnu.org/gcc-4.2/changes.html
> Lets make sure we still check this.
>
> Signed-off-by: Eric Sesterhenn <snakebyte-Mmb7MZpHnFY@public.gmane.org>
>   

Applied, thanks.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV