Skip to contentSkip to conversation

libkvm: initialize no_pit_creation

PATCH Marcelo Tosatti 3 participants 4 messages Apr 26, 2008 – May 2, 2008 KVM

lore
Share & tools
lore

Marcelo Tosatti

author
#0 lore raw

Valgrind caught this:

==11754== Conditional jump or move depends on uninitialised value(s)

==11754==    at 0x50C9BC: kvm_create_pit (libkvm-x86.c:153)
==11754==    by 0x50CA7F: kvm_arch_create (libkvm-x86.c:178)
==11754==    by 0x50AB31: kvm_create (libkvm.c:383)
==11754==    by 0x4EE691: kvm_qemu_create_context (qemu-kvm.c:616)
==11754==    by 0x412031: main (vl.c:9653)
Signed-off-by Marcelo Tosatti <[email protected]>
libkvm/libkvm.c +1 -0
index 329f29f..adf09a5 100644
@@ -249,6 +249,7 @@ kvm_context_t kvm_init(struct kvm_callbacks *callbacks,
 	kvm->opaque = opaque;
 	kvm->dirty_pages_log_all = 0;
 	kvm->no_irqchip_creation = 0;
+	kvm->no_pit_creation = 0;
 
 	return kvm;
  out_close:

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

#1 lore raw
Quoted 10 lines · Marcelo Tosatti

Valgrind caught this:

==11754== Conditional jump or move depends on uninitialised value(s)

==11754==    at 0x50C9BC: kvm_create_pit (libkvm-x86.c:153)
==11754==    by 0x50CA7F: kvm_arch_create (libkvm-x86.c:178)
==11754==    by 0x50AB31: kvm_create (libkvm.c:383)
==11754==    by 0x4EE691: kvm_qemu_create_context (qemu-kvm.c:616)
==11754==    by 0x412031: main (vl.c:9653)

Applied, thanks. Isn't valgrind great?

Signature · 8 lines
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
#2 lore raw
Quoted 12 lines · Avi Kivity
Quoted 10 lines · Marcelo Tosatti

Valgrind caught this:

==11754== Conditional jump or move depends on uninitialised value(s)

==11754==    at 0x50C9BC: kvm_create_pit (libkvm-x86.c:153)
==11754==    by 0x50CA7F: kvm_arch_create (libkvm-x86.c:178)
==11754==    by 0x50AB31: kvm_create (libkvm.c:383)
==11754==    by 0x4EE691: kvm_qemu_create_context (qemu-kvm.c:616)
==11754==    by 0x412031: main (vl.c:9653)
Quoted 2 lines

Applied, thanks. Isn't valgrind great?

Yeah, it is. Reminds me of another warning I recently came across (offsets may vary due to other patches:

==5801== 1 errors in context 1 of 2:
==5801== Conditional jump or move depends on uninitialised value(s)

==5801==    at 0x53F4AE: kvm_register_userspace_phys_mem (libkvm.c:552)
==5801==    by 0x521ACA: kvm_cpu_register_physical_memory (qemu-kvm.c:654)
==5801==    by 0x45FC82: pc_init1 (pc.c:809)
==5801==    by 0x461313: pc_init_pci (pc.c:1149)
==5801==    by 0x43081B: main (vl.c:9845)

This silences valgrind and may even be correct (if I got the code path right):

Signed-off-by Jan Kiszka <[email protected]>
libkvm/libkvm.c +3 -2
@@ -328,9 +328,10 @@ static int kvm_create_default_phys_mem(k
 
 #ifdef KVM_CAP_USER_MEMORY
 	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_USER_MEMORY);
-	if (r > 0)
+	if (r > 0) {
+		kvm->physical_memory = NULL;
 		return 0;
-	else
+	} else
 #endif
 		r = kvm_alloc_kernel_memory(kvm, memory, vm_mem);
 	if (r < 0)

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

#3 lore raw
Quoted 23 lines · Jan Kiszka

Yeah, it is. Reminds me of another warning I recently came across (offsets may vary due to other patches:

==5801== 1 errors in context 1 of 2:
==5801== Conditional jump or move depends on uninitialised value(s)

==5801==    at 0x53F4AE: kvm_register_userspace_phys_mem (libkvm.c:552)
==5801==    by 0x521ACA: kvm_cpu_register_physical_memory (qemu-kvm.c:654)
==5801==    by 0x45FC82: pc_init1 (pc.c:809)
==5801==    by 0x461313: pc_init_pci (pc.c:1149)
==5801==    by 0x43081B: main (vl.c:9845)

This silences valgrind and may even be correct (if I got the code path right):

Signed-off-by Jan Kiszka <[email protected]>
libkvm/libkvm.c +0 -0
@@ -328,9 +328,10 @@ static int kvm_create_default_phys_mem(k
 
 #ifdef KVM_CAP_USER_MEMORY

libkvm/libkvm.c static int kvm_create_default_phys_mem(k · line 337

Quoted 10 lines
libkvm/libkvm.c +3 -2
 	r = ioctl(kvm->fd, KVM_CHECK_EXTENSION, KVM_CAP_USER_MEMORY);
-	if (r > 0)
+	if (r > 0) {
+		kvm->physical_memory = NULL;
 		return 0;
-	else
+	} else
 #endif
 		r = kvm_alloc_kernel_memory(kvm, memory, vm_mem);
 	if (r < 0)

As related earlier, physical_memory is bogus, so I removed it. Let's pretend it never existed.

Signature · 9 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone