[0/4] ppc: valgrind "uninitialized values" fixes

Message

Daniel Henrique Barboza March 30, 2022, 9:04 p.m. UTC

Hi,

These are a handful of trivial fixes to make Valgrind happier when
profiling the boot of a pSeries guest. All the patches are handling a
similar case where we have something similar to this:

---
uint64_t val;

(...)

kvm_vcpu_ioctl(...., &val);
---

Valgrind does not consider that 'val' was initialized and then it keeps
complaining about every future use of 'val', or anything that got
assigned as 'val', as being an uninitialized value/data. The fix is
simple and without any side effects:: just initialize 'val'.

After this series, together with the memory leak fix sent in [1], we
don't see any more ppc/spapr related Valgrind memcheck complaints when
booting a pSeries guest.

No functional changes were made.

[1] https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg06804.html

Daniel Henrique Barboza (4):
  target/ppc: initialize 'reg_val' in kvm_get_one_spr()
  target/ppc: init 'lpcr' in kvmppc_enable_cap_large_decr()
  target/ppc: init 'sregs' in kvmppc_put_books_sregs()
  target/ppc: init 'rmmu_info' in kvm_get_radix_page_info()

 target/ppc/kvm.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Philippe Mathieu-Daudé March 30, 2022, 9:46 p.m. UTC | #1

On 30/3/22 23:04, Daniel Henrique Barboza wrote:
> Hi,
> 
> These are a handful of trivial fixes to make Valgrind happier when
> profiling the boot of a pSeries guest. All the patches are handling a
> similar case where we have something similar to this:
> 
> ---
> uint64_t val;
> 
> (...)
> 
> kvm_vcpu_ioctl(...., &val);

Which is a false positive.

> ---
> 
> Valgrind does not consider that 'val' was initialized and then it keeps
> complaining about every future use of 'val', or anything that got
> assigned as 'val', as being an uninitialized value/data. The fix is
> simple and without any side effects:: just initialize 'val'.

I gave a try to some definition but the result is rather ugly =)

-- >8 --
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index d9359859d4..85429930fb 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -194,4 +194,10 @@ extern void QEMU_NORETURN QEMU_ERROR("code path is 
reachable")
  #define QEMU_DISABLE_CFI
  #endif

+#ifdef QEMU_STATIC_ANALYSIS
+#define QEMU_UNNECESSARY_INIT(values...) = values
+#else
+#define QEMU_UNNECESSARY_INIT(values...)
+#endif
+
  #endif /* COMPILER_H */
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index dc93b99189..97aec29694 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -546,7 +546,7 @@ static void kvm_get_one_spr(CPUState *cs, uint64_t 
id, int spr)
      union {
          uint32_t u32;
          uint64_t u64;
-    } val;
+    } val QEMU_UNNECESSARY_INIT({ });
      struct kvm_one_reg reg = {
          .id = id,
          .addr = (uintptr_t) &val,
---

Being able to use valgrind is more valuable that these unnecessary
init, so:
Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>