[Qemu-devel] [PATCH] trace: add sanity check

Anthony Xu posted 1 patch 6 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1494369432-15418-1-git-send-email-anthony.xu@intel.com
Test checkpatch passed
Test docker passed
Test s390x passed
qom/cpu.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] trace: add sanity check
Posted by Anthony Xu 6 years, 11 months ago
If trace backend is set to TRACE_NOP, trace_get_vcpu_event_count
returns 0, cause bitmap_new call abort.

Signed-off-by: Anthony Xu <anthony.xu@intel.com>
---
 qom/cpu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qom/cpu.c b/qom/cpu.c
index f02e9c0..f9111a0 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -382,6 +382,7 @@ static void cpu_common_unrealizefn(DeviceState *dev, Error **errp)
 
 static void cpu_common_initfn(Object *obj)
 {
+    uint32_t count;
     CPUState *cpu = CPU(obj);
     CPUClass *cc = CPU_GET_CLASS(obj);
 
@@ -396,7 +397,10 @@ static void cpu_common_initfn(Object *obj)
     QTAILQ_INIT(&cpu->breakpoints);
     QTAILQ_INIT(&cpu->watchpoints);
 
-    cpu->trace_dstate = bitmap_new(trace_get_vcpu_event_count());
+    count = trace_get_vcpu_event_count();
+    if (count) {
+        cpu->trace_dstate = bitmap_new(count);
+    }
 
     cpu_exec_initfn(cpu);
 }
-- 
1.8.3.1


Re: [Qemu-devel] [PATCH] trace: add sanity check
Posted by Stefan Hajnoczi 6 years, 11 months ago
On Tue, May 09, 2017 at 03:37:12PM -0700, Anthony Xu wrote:
> If trace backend is set to TRACE_NOP, trace_get_vcpu_event_count
> returns 0, cause bitmap_new call abort.
> 
> Signed-off-by: Anthony Xu <anthony.xu@intel.com>
> ---
>  qom/cpu.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Please post steps for reproducing the abort.  I cannot reproduce this
with qemu-system-x86_64.

> diff --git a/qom/cpu.c b/qom/cpu.c
> index f02e9c0..f9111a0 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -382,6 +382,7 @@ static void cpu_common_unrealizefn(DeviceState *dev, Error **errp)
>  
>  static void cpu_common_initfn(Object *obj)
>  {
> +    uint32_t count;
>      CPUState *cpu = CPU(obj);
>      CPUClass *cc = CPU_GET_CLASS(obj);
>  
> @@ -396,7 +397,10 @@ static void cpu_common_initfn(Object *obj)
>      QTAILQ_INIT(&cpu->breakpoints);
>      QTAILQ_INIT(&cpu->watchpoints);
>  
> -    cpu->trace_dstate = bitmap_new(trace_get_vcpu_event_count());
> +    count = trace_get_vcpu_event_count();
> +    if (count) {
> +        cpu->trace_dstate = bitmap_new(count);
> +    }
>  
>      cpu_exec_initfn(cpu);
>  }
> -- 
> 1.8.3.1
> 
> 
Re: [Qemu-devel] [PATCH] trace: add sanity check
Posted by Xu, Anthony 6 years, 11 months ago
> Please post steps for reproducing the abort.  I cannot reproduce this
> with qemu-system-x86_64.

The steps to reproduce the issue,

./configure --enable-trace-backend=nop --target-list=x86_64-softmmu

gdb -args ./x86_64-softmmu/qemu-system-x86_64 -bios /home/root/guest/seabios.bin -smp 1 -machine q35,accel=kvm
   -m 1G -drive format=raw,file=/home/root/images/centos7.2.img,if=ide,index=0 -nographic  -nodefaults
  -serial stdio -monitor pty


(gdb) bt
#0  0x00007ffff04e25f7 in raise () from /lib64/libc.so.6
#1  0x00007ffff04e3ce8 in abort () from /lib64/libc.so.6
#2  0x00005555559de905 in bitmap_new (nbits=<optimized out>)
    at /home/root/git/qemu2.git/include/qemu/bitmap.h:96
#3  cpu_common_initfn (obj=0x555556621d30) at qom/cpu.c:399
#4  0x0000555555a11869 in object_init_with_type (obj=0x555556621d30, ti=0x55555656bbb0) at qom/object.c:341
#5  0x0000555555a11869 in object_init_with_type (obj=0x555556621d30, ti=0x55555656bd30) at qom/object.c:341
#6  0x0000555555a11efc in object_initialize_with_type (data=data@entry=0x555556621d30, size=76560,
    type=type@entry=0x55555656bd30) at qom/object.c:376
#7  0x0000555555a12061 in object_new_with_type (type=0x55555656bd30) at qom/object.c:484
#8  0x0000555555a121c5 in object_new (typename=typename@entry=0x555556550340 "qemu64-x86_64-cpu")
    at qom/object.c:494
#9  0x00005555557f6e3d in pc_new_cpu (typename=typename@entry=0x555556550340 "qemu64-x86_64-cpu", apic_id=0,
    errp=errp@entry=0x5555565391b0 <error_fatal>) at /home/root/git/qemu2.git/hw/i386/pc.c:1101
#10 0x00005555557fa33e in pc_cpus_init (pcms=pcms@entry=0x5555565f9690)
    at /home/root/git/qemu2.git/hw/i386/pc.c:1184
#11 0x00005555557fe0f6 in pc_q35_init (machine=0x5555565f9690) at /home/root/git/qemu2.git/hw/i386/pc_q35.c:121
#12 0x000055555574fbad in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4562


Anthony


> 
> > diff --git a/qom/cpu.c b/qom/cpu.c
> > index f02e9c0..f9111a0 100644
> > --- a/qom/cpu.c
> > +++ b/qom/cpu.c
> > @@ -382,6 +382,7 @@ static void cpu_common_unrealizefn(DeviceState
> *dev, Error **errp)
> >
> >  static void cpu_common_initfn(Object *obj)
> >  {
> > +    uint32_t count;
> >      CPUState *cpu = CPU(obj);
> >      CPUClass *cc = CPU_GET_CLASS(obj);
> >
> > @@ -396,7 +397,10 @@ static void cpu_common_initfn(Object *obj)
> >      QTAILQ_INIT(&cpu->breakpoints);
> >      QTAILQ_INIT(&cpu->watchpoints);
> >
> > -    cpu->trace_dstate = bitmap_new(trace_get_vcpu_event_count());
> > +    count = trace_get_vcpu_event_count();
> > +    if (count) {
> > +        cpu->trace_dstate = bitmap_new(count);
> > +    }
> >
> >      cpu_exec_initfn(cpu);
> >  }
> > --
> > 1.8.3.1
> >
> >

Re: [Qemu-devel] [PATCH] trace: add sanity check
Posted by Stefan Hajnoczi 6 years, 11 months ago
On Tue, May 09, 2017 at 03:37:12PM -0700, Anthony Xu wrote:
> If trace backend is set to TRACE_NOP, trace_get_vcpu_event_count
> returns 0, cause bitmap_new call abort.
> 
> Signed-off-by: Anthony Xu <anthony.xu@intel.com>
> ---
>  qom/cpu.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Thanks, applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan