Re: [PATCH 2/6] contrib/plugins/cache: fix warning when compiling on 32bits host

[Richard Henderson]

On 8/15/24 21:46, Alex Bennée wrote:
> Thomas Huth <thuth@redhat.com> writes:
>
> > On 15/08/2024 01.36, Pierrick Bouvier wrote:
> > > Found on debian stable (i386).
> > > ../contrib/plugins/cache.c: In function 'vcpu_tb_trans':
> > > ../contrib/plugins/cache.c:477:30: error: cast from pointer to integer of 
> > > different size [-Werror=pointer-to-int-cast]
> > >     477 |             effective_addr = (uint64_t) qemu_plugin_insn_haddr(insn);
> > >         |
> > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> > > ---
> > >    contrib/plugins/cache.c | 6 +++---
> > >    1 file changed, 3 insertions(+), 3 deletions(-)
> > > diff --git a/contrib/plugins/cache.c b/contrib/plugins/cache.c
> > > index 512ef6776b7..82ed734d6d4 100644
> > > --- a/contrib/plugins/cache.c
> > > +++ b/contrib/plugins/cache.c
> > > @@ -471,12 +471,12 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct 
> > > qemu_plugin_tb *tb)
> > >        n_insns = qemu_plugin_tb_n_insns(tb);
> > >        for (i = 0; i < n_insns; i++) {
> > >            struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
> > > -        uint64_t effective_addr;
> > > +        uintptr_t effective_addr;
> > >              if (sys) {
> > > -            effective_addr = (uint64_t) qemu_plugin_insn_haddr(insn);
> > > +            effective_addr = (uintptr_t) qemu_plugin_insn_haddr(insn);
> > >            } else {
> > > -            effective_addr = (uint64_t) qemu_plugin_insn_vaddr(insn);
> > > +            effective_addr = (uintptr_t)
> > > qemu_plugin_insn_vaddr(insn);
> > >            }
> >
> > Is this the right fix? I assume effective_addr stores an address of
> > the guest, so if the guest is 64-bit and the host is 32-bit, you now
> > lose the upper bits of the address...?
>
> I think the problem is higher up, it was a mistake to have:
>
>    void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn);
>
> return *void.

No, not a bug.  This is a host addr, right there in the name.
Returning uint64_t would be a bug.


r~