Re: use of uninitialized variable involving visit_type_uint32() and friends

[Peter Maydell]

On Fri, 1 Apr 2022 at 09:08, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
>
> On Thu, Mar 31, 2022 at 7:35 PM Peter Maydell <peter.maydell@linaro.org> 
> wrote:
>>
>> Coverity warns about use of uninitialized data in what seems
>> to be a common pattern of use of visit_type_uint32() and similar
>> functions. Here's an example from target/arm/cpu64.c:
>>
>> static void cpu_max_set_sve_max_vq(Object *obj, Visitor *v, const char *name,
>>                                    void *opaque, Error **errp)
>> {
>>     ARMCPU *cpu = ARM_CPU(obj);
>>     uint32_t max_vq;
>>     if (!visit_type_uint32(v, name, &max_vq, errp)) {
>>         return;
>>     }
>>     [code that does something with max_vq here]
>> }
>>
>> This doesn't initialize max_vq, on the apparent assumption
>> that visit_type_uint32() will do so. But that function [...]
>> reads the value of *obj (the uninitialized max_vq).
>
>
> The visit_type_* functions are written to work for both getters and setters.
> For the leaves, that means potentially reading uninitialized data.  It is
> harmless but very ugly, and with respect to static analysis it was all but
> a time bomb, all the time.
>
> The best (but most intrusive) solution would be to add a parameter to all
> visit_type_* functions with the expected "direction" of the visit, which
> could be checked against v->type.

So do we have a plan for what we want to do with this issue?

In the meantime, any objections to my just marking all the Coverity
issues which are pointing out that visit_* functions use uninitialized
data as 'false positive' ? There are a ton of them, and they clog up
the issue UI and make it hard to see other actually interesting reports.

thanks
-- PMM