Re: [Qemu-devel] [PATCH v6 01/10] qom: add helpers for UserCreatable object types

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3096 bytes --]

On 04/27/2016 03:58 AM, Daniel P. Berrange wrote:
> On Wed, Apr 27, 2016 at 11:26:23AM +0200, Markus Armbruster wrote:
>> This commit regresses error message quality from
>>
>>     $ qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
>>     qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found
>>
>> to just
>>
>>     qemu-system-x86_64: Property '.foo' not found
> 
> I'm not seeing that behaviour myself in current git master, nor
> immediately before or after 90998d58964cd17f8b0b03800b0a4508f8b543da
> is applied. I always just get
> 
>  $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -display none -object secret,id=sec0,data=letmein,format=raw,foo=bar
>  qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found
> 
> So it all appears to be working correctly. How reliably reproducable
> is it for you ?  I'm testing on Fedora 23 x86_64 host and can't
> see the failure despite many invokations.

I'm reproducing it on my F23 machine, where 90998d58 indeed flips the
behavior I'm seeing. Maybe it's a factor of which malloc engine is in
use, or level of compiler optimization?

My config.status states:
exec '/home/eblake/qemu/configure' '--enable-kvm' '--enable-system'
'--disable-user' '--target-list=x86_64-softmmu,ppc64-softmmu'
'--enable-debug'



> 
>> Clue: cur_loc points to garbage.
>>
>>     (gdb) p cur_loc
>>     $1 = (Location *) 0x7fffffffdc10
>>     (gdb) p *cur_loc
>>     $2 = {kind = (unknown: 4294958128), num = 32767, 
>>       ptr = 0x555555b804a2 <error_report_err+44>, prev = 0x5555565d2770 <std_loc>}
>>
>> Looks like cur_loc is dangling.  Happens when you forget to loc_pop() a
>> Location before it dies.  This one is on the stack.
>>
>> *Might* be release critical.
> 
> This patch doesn't even touch any code which calls loc_push/loc_pop
> so I'm kind of surprised if this patch breaks it.  Given that it looks
> like stack corruption though, I wonder if this commit has just exposed
> an already latent non-deterministic bug for you ? IOW root cause could
> be an earlier patch ?

Could it be a latent bug in qemu_opts_foreach()? Your patch changes a
call from qemu_opts_foreach(object_create) to
qemu_opts_foreach(user_creatable_add_opts_foreach), where the new
callback may expose different behavior to the stack and thus expose the
latent problem.


>>> @@ -4417,8 +4360,9 @@ int main(int argc, char **argv, char **envp)
>>>      }
>>>  
>>>      if (qemu_opts_foreach(qemu_find_opts("object"),
>>> -                          object_create,
>>> -                          object_create_delayed, NULL)) {
>>> +                          user_creatable_add_opts_foreach,
>>> +                          object_create_delayed, &err)) {
>>> +        error_report_err(err);
>>>          exit(1);
>>>      }
> 
> Regards,
> Daniel
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]