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

[Markus Armbruster <armbru@redhat.com>]

Peter Maydell <peter.maydell@linaro.org> writes:

> 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.

I think to object, I would have to propose an alternative with a
reasonable chance of success within a reasonable time frame.  I can't,
so I won't.

Paolo proposed improvements, and I think they're worth exploring, but
such musings fall well short of my condition for "may object".