On 8/15/24 10:40, Pierrick Bouvier wrote:
On 8/15/24 05:03, 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/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.bouv...@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).
After looking more closely at existing plugins, the problem is wider
than expected (and build warnings).
The GLib macro (GUINT_TO_POINTER) silently transform an int64 value to a
pointer, including on 32 bits platform. It performs a double case
(unsigned long -> pointer).
Thus, even if there is no warning, some plugins are broken because of
data (either because they pass data in callback, or they have an address
based hashtable).
It could be fixable, but the wiser solution is probably to disable
plugins on 32 bits platform, like Alex proposed.
Thomas
