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

[Pierrick Bouvier]

On 8/15/24 05:03, 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/hwprofile.c: In function 'new_location':
> > > ../contrib/plugins/hwprofile.c:172:32: error: cast to pointer from integer of 
> > > different size [-Werror=int-to-pointer-cast]
> > >     172 |     g_hash_table_insert(table, (gpointer) off_or_pc, loc);
> > >         |                                ^
> > > ../contrib/plugins/hwprofile.c: In function 'vcpu_haddr':
> > > ../contrib/plugins/hwprofile.c:227:19: error: cast from pointer to integer of 
> > > different size [-Werror=pointer-to-int-cast]
> > >     227 |             off = (uint64_t) udata;
> > >         |                   ^
> > > ../contrib/plugins/hwprofile.c:232:62: error: cast to pointer from integer of 
> > > different size [-Werror=int-to-pointer-cast]
> > >     232 |                                                              
> > > (gpointer) off);
> > >         |                                                              ^
> > > ../contrib/plugins/hwprofile.c: In function 'vcpu_tb_trans':
> > > ../contrib/plugins/hwprofile.c:250:26: error: cast to pointer from integer of 
> > > different size [-Werror=int-to-pointer-cast]
> > >     250 |         gpointer udata = (gpointer) (source ? 
> > > qemu_plugin_insn_vaddr(insn) : 0);
> > >         |
> > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> > > ---
> > >    contrib/plugins/hwprofile.c | 9 +++++----
> > >    1 file changed, 5 insertions(+), 4 deletions(-)
> > > diff --git a/contrib/plugins/hwprofile.c
> > > b/contrib/plugins/hwprofile.c
> > > index 739ac0c66b5..ee94a74ad94 100644
> > > --- a/contrib/plugins/hwprofile.c
> > > +++ b/contrib/plugins/hwprofile.c
> > > @@ -165,7 +165,7 @@ static DeviceCounts *new_count(const char *name, uint64_t 
> > > base)
> > >        return count;
> > >    }
> > >    -static IOLocationCounts *new_location(GHashTable *table, uint64_t
> > > off_or_pc)
> > > +static IOLocationCounts *new_location(GHashTable *table, uintptr_t off_or_pc)
> > >    {
> > >        IOLocationCounts *loc = g_new0(IOLocationCounts, 1);
> > >        loc->off_or_pc = off_or_pc;
> > > @@ -201,7 +201,7 @@ static void vcpu_haddr(unsigned int cpu_index, 
> > > qemu_plugin_meminfo_t meminfo,
> > >            return;
> > >        } else {
> > >            const char *name = qemu_plugin_hwaddr_device_name(hwaddr);
> > > -        uint64_t off = qemu_plugin_hwaddr_phys_addr(hwaddr);
> > > +        uintptr_t off = qemu_plugin_hwaddr_phys_addr(hwaddr);
> >
> > qemu_plugin_hwaddr_phys_addr() returns an uint64_t, so this looks
> > wrong to me.
>
> It is. However it just goes to show you should be expecting to
> instrument 64 bit code with a 32 bit host because you can't do pointer
> stuffing tricks like this.
>
> Maybe we could just disable plugins on 32 bit hosts?

Only two plugins are concerned by this problem, it's worth fixing them 
correctly (i.e. not use 64 bits data directly as a pointer, but allocate 
memory and pass pointer instead).

> >   Thomas