Hi Richard,
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.
This is something I already tried to fix this way, but alas, casting
values is not enough, we might lose information (in the case where guest
is 64 bits). Some plugins need a refactoring to allocate data
dynamically, instead of hiding it under a pointer.
See this previous series:
https://patchew.org/QEMU/20240814233645.944327-1-pierrick.bouv...@linaro.org/
Finally, we discussed it was not worth the effort, and Alex simply
deactivated plugins by default for 32 bits platform, so it should not be
built for arm 32 bits. If we really have someone that needs this
usecase, we might make the effort, but for now, it does not seem worth
the hassle.
Note: we already had those warnings before, but since plugins used to be
built by an external Makefile, werror was not enabled, so functionally
it was already "broken".
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);
