|
From: | Daniel Henrique Barboza |
Subject: | Re: use of uninitialized variable involving visit_type_uint32() and friends |
Date: | Thu, 31 Mar 2022 19:27:43 -0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 |
On 3/31/22 14:35, Peter Maydell 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 is: bool visit_type_uint32(Visitor *v, const char *name, uint32_t *obj, Error **errp) { uint64_t value; bool ok; trace_visit_type_uint32(v, name, obj); value = *obj; ok = visit_type_uintN(v, &value, name, UINT32_MAX, "uint32_t", errp); *obj = value; return ok; } So it reads the value of *obj (the uninitialized max_vq). What's the right way to write this kind of object-property setter function? Just pre-initialize the variable to 0?
This reminds me of Valgrind ppc-related warnings I sent patches yesterday. In a code like this: (target/ppc/kvm.c) int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable) { CPUState *cs = CPU(cpu); uint64_t lpcr; kvm_get_one_reg(cs, KVM_REG_PPC_LPCR_64, &lpcr); /* Do we need to modify the LPCR? */ if (!!(lpcr & LPCR_LD) != !!enable) { Valgrind complains of "Conditional jump or move depends on uninitialised value(s)" because we're using 'lpcr' in the conditional and 'lpcr' isn't being initialized. Valgrind doesn't seem to care that kvm_get_one_reg() might be writing 'lpcr'. The fix I proposed consists of initializing the vars in these cases. My suggestion in this case is to initialize 'max_vq' as well. Apparently these static code analysis tools don't handle the "var being initialized by being passed as reference to another function" scenarios. Thanks, Daniel
thanks -- PMM
[Prev in Thread] | Current Thread | [Next in Thread] |