On 8/15/24 04:46, Alex Bennée wrote:
Thomas Huth <th...@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.bouv...@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, at least vaddr returns an explicit 64 bit value which can
hold everything (at a slight expense to 32bit emulation hosts, but
seriously stop doing that we've been in the 64bit world for some time
now).
It's an open question I had. When executing 64 bits binaries on a 32
bits host, are we emulating the full 64 bits address space, or do we
restrict to 32 bits? For user mode, I don't see how it could be possible
to have address space beyond the 32 bits range, but system mode is
probably different.
The real proper fix is to not encode directly value under udata for
callbacks, but allocate this and pass a pointer instead.
The casting for qemu_plugin_insn_vaddr is not required at all since it
already returns an uint64_t, so you can remoe that one. For the haddr
part, maybe do a double-cast:
effective_addr = (uint64_t)(uintptr_t)qemu_plugin_insn_haddr(insn)
?
Thomas
