Re: How can I find problematic uses of error_report() with vrc?

[Paolo Bonzini <pbonzini@redhat.com>]

One fixlet:

On 5/8/23 12:41, Paolo Bonzini wrote:
> Let's just cut out function  pointers for simplicity:
> 
> (vrc) paths --limit 1 qemu_ram_resize [!ErrorPP,function_pointer]* 
> error_report
> <no output>

This should have been

   paths --limit 1 qemu_ram_resize [!ErrorPP,!function_pointer]* error_report

but the output is indeed empty; it was just a matter of cutting-and-
pasting from the wrong line.

It could also be written using DeMorgan's law as

   paths --limit 1 qemu_ram_resize [![ErrorPP|function_pointer]]* error_report

where the outer brackets are not necessary but improve readability.

Paolo

> Okay, so this one was a false positive as well.
> 
> So you can see the good and the bad here.  The tool is powerful and 
> finds what you asked.  The problem is that there's _a lot_ of hay in 
> which you have to find the needle.  For coroutines it works bettr 
> because we have already cleaned it up, you can get there but it takes 
> some sweat.
> 
> [here]
> 
> Let's try a more precise (but also more restrictive) query that only has 
> a single function that does not take Error** but calls error_report:
> 
> (vrc) paths [ErrorPP] [ErrorPP]* [!ErrorPP] error_report
> error_report <- qemu_open_old <- qmp_chardev_open_file_source
> error_report <- runstate_set <- qemu_system_wakeup_request
> error_report <- machine_consume_memdev <- machine_run_board_init
> error_report <- numa_complete_configuration <- machine_run_board_init
> error_report <- egl_rendernode_init <- egl_init
> error_report <- parse_numa_node <- set_numa_options
> 
> I checked parse_numa_node and numa_complete_configuration, and they're 
> genuine issues.
> 
> Let's add a couple labels by hand to see if it finds your example:
> 
> (vrc) label ErrorPP qmp_migrate rdma_start_outgoing_migration 
> qemu_rdma_source_init
> (vrc) paths qmp_migrate [ErrorPP]* [!ErrorPP] error_report
> error_report <- migrate_fd_connect <- rdma_start_outgoing_migration <- 
> qmp_migrate
> error_report <- qemu_rdma_cleanup <- rdma_start_outgoing_migration <- 
> qmp_migrate
> error_report <- qemu_rdma_resolve_host <- qemu_rdma_source_init <- 
> rdma_start_outgoing_migration <- qmp_migrate
> error_report <- qemu_rdma_alloc_pd_cq <- qemu_rdma_source_init <- 
> rdma_start_outgoing_migration <- qmp_migrate
> error_report <- qemu_rdma_cleanup <- qemu_rdma_source_init <- 
> rdma_start_outgoing_migration <- qmp_migrate
> error_report <- qemu_rdma_reg_control <- qemu_rdma_source_init <- 
> rdma_start_outgoing_migration <- qmp_migrate
> error_report <- qemu_rdma_connect <- rdma_start_outgoing_migration <- 
> qmp_migrate
> 
> Mission accomplished. :)
> 
> Paolo
> 
>> Here's my find-error-fns.cocci:
>>
>> @r@
>> identifier fn, errp;
>> position p;
>> @@
>>   fn@p(..., Error **errp, ...)
>>   {
>>       ...
>>   }
>> @script:python@
>> fn << r.fn;
>> p << r.p;
>> @@
>> print(f'{p[0].file}:{p[0].line}:{p[0].column}:{fn}')
>>