Re: contrib/plugins does not build on 32-bit host

[Pierrick Bouvier]

On 12/13/24 13:47, Richard Henderson wrote:
> Hi,
>
> Several of the recent contrib/plugins/ patches do not build on e.g. arm32.
> All of the issues are related to casting between pointers and uint64_t; there 
> is a Werror
> generated for casting between pointers and integers of different sizes.
>
> I suspect all of the instances will need to use separate structures to store 
> uint64_t
> within the hash tables.  The hash values themselves can use uintptr_t, as 
> "hash" by
> definition loses data.
>
> The following is *not* a suggested patch, just touches every place with an 
> error to
> highlight all of the places.
>
>
> r~
>
>
> diff --git a/contrib/plugins/cache.c b/contrib/plugins/cache.c
> index 512ef6776b..9f1b05fc35 100644
> --- a/contrib/plugins/cache.c
> +++ b/contrib/plugins/cache.c
> @@ -474,7 +474,7 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct 
> qemu_plugin_tb *tb)
>            uint64_t effective_addr;
>
>            if (sys) {
> -            effective_addr = (uint64_t) qemu_plugin_insn_haddr(insn);
> +            effective_addr = (uint64_t)(uintptr_t) 
> qemu_plugin_insn_haddr(insn);
>            } else {
>                effective_addr = (uint64_t) qemu_plugin_insn_vaddr(insn);
>            }
> diff --git a/contrib/plugins/cflow.c b/contrib/plugins/cflow.c
> index b39974d1cf..8f8ebf87cd 100644
> --- a/contrib/plugins/cflow.c
> +++ b/contrib/plugins/cflow.c
> @@ -215,10 +215,10 @@ static NodeData *fetch_node(uint64_t addr, bool 
> create_if_not_found)
>        NodeData *node = NULL;
>
>        g_mutex_lock(&node_lock);
> -    node = (NodeData *) g_hash_table_lookup(nodes, (gconstpointer) addr);
> +    node = (NodeData *) g_hash_table_lookup(nodes, (gconstpointer)(uintptr_t) 
> addr);
>        if (!node && create_if_not_found) {
>            node = create_node(addr);
> -        g_hash_table_insert(nodes, (gpointer) addr, (gpointer) node);
> +        g_hash_table_insert(nodes, (gpointer)(uintptr_t) addr, (gpointer) 
> node);
>        }
>        g_mutex_unlock(&node_lock);
>        return node;
> diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
> index 02bc5078bd..9b3d356dea 100644
> --- a/contrib/plugins/hotblocks.c
> +++ b/contrib/plugins/hotblocks.c
> @@ -111,7 +111,7 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct 
> qemu_plugin_tb *tb)
>        ExecCount *cnt;
>        uint64_t pc = qemu_plugin_tb_vaddr(tb);
>        size_t insns = qemu_plugin_tb_n_insns(tb);
> -    uint64_t hash = pc ^ insns;
> +    uintptr_t hash = pc ^ insns;
>
>        g_mutex_lock(&lock);
>        cnt = (ExecCount *) g_hash_table_lookup(hotblocks, (gconstpointer) hash);
> diff --git a/contrib/plugins/hwprofile.c b/contrib/plugins/hwprofile.c
> index 739ac0c66b..6d84ea77f2 100644
> --- a/contrib/plugins/hwprofile.c
> +++ b/contrib/plugins/hwprofile.c
> @@ -169,7 +169,7 @@ static IOLocationCounts *new_location(GHashTable *table, 
> uint64_t
> off_or_pc)
>    {
>        IOLocationCounts *loc = g_new0(IOLocationCounts, 1);
>        loc->off_or_pc = off_or_pc;
> -    g_hash_table_insert(table, (gpointer) off_or_pc, loc);
> +    g_hash_table_insert(table, (gpointer)(uintptr_t) off_or_pc, loc);
>        return loc;
>    }
>
> @@ -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);
>            bool is_write = qemu_plugin_mem_is_store(meminfo);
>            DeviceCounts *counts;
>
> @@ -224,7 +224,7 @@ static void vcpu_haddr(unsigned int cpu_index, 
> qemu_plugin_meminfo_t
> meminfo,
>
>            /* either track offsets or source of access */
>            if (source) {
> -            off = (uint64_t) udata;
> +            off = (uintptr_t) udata;
>            }
>
>            if (pattern || source) {
> @@ -247,7 +247,7 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct 
> qemu_plugin_tb *tb)
>
>        for (i = 0; i < n; i++) {
>            struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
> -        gpointer udata = (gpointer) (source ? qemu_plugin_insn_vaddr(insn) : 
> 0);
> +        gpointer udata = (gpointer)(uintptr_t) (source ? 
> qemu_plugin_insn_vaddr(insn) : 0);
>            qemu_plugin_register_vcpu_mem_cb(insn, vcpu_haddr,
>                                             QEMU_PLUGIN_CB_NO_REGS,
>                                             rw, udata);

The series fixing this was sent:
20241217010707.2557258-1-pierrick.bouvier@linaro.org/T/#t">https://lore.kernel.org/qemu-devel/20241217010707.2557258-1-pierrick.bouvier@linaro.org/T/#t

Pierrick