tests/tcg/plugins/mem.c | 71 +++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 41 deletions(-)
In commit eb3f69cac62670 we removed the dependency of this
mem plugin on the QEMU headers, but in doing that we introduced
undefined behaviour when the plugin accesses unaligned memory.
This shows up if you build with the gcc or clang address
sanitizer and run 'make check-tcg', in numerous warnings like:
../../tests/tcg/plugins/mem.c:167:27: runtime error: load of misaligned address 0x7f1f300354b1 for type 'uint16_t' (aka 'unsigned short'), which requires 2 byte alignment
0x7f1f300354b1: note: pointer points here
00 00 00 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 14 15 16 17 18 19 1a 1b 1c
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../tests/tcg/plugins/mem.c:167:27
Fix this by rearranging the data reads and writes to use
memcpy() instead.
Fixes: eb3f69cac62670 ("tests/tcg/plugins/mem.c: remove dependency on qemu headers")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
tests/tcg/plugins/mem.c | 71 +++++++++++++++++------------------------
1 file changed, 30 insertions(+), 41 deletions(-)
diff --git a/tests/tcg/plugins/mem.c b/tests/tcg/plugins/mem.c
index 7d64e7018f..f3992abc8f 100644
--- a/tests/tcg/plugins/mem.c
+++ b/tests/tcg/plugins/mem.c
@@ -123,6 +123,9 @@ static void update_region_info(uint64_t region, uint64_t offset,
bool is_store = qemu_plugin_mem_is_store(meminfo);
RegionInfo *ri;
bool unseen_data = false;
+ void *val_ptr;
+ unsigned int val_size;
+ qemu_plugin_mem_value swapped_value;
g_assert(offset + size <= region_size);
@@ -144,61 +147,46 @@ static void update_region_info(uint64_t region, uint64_t offset,
}
void *ri_data = &ri->data[offset];
+
+ swapped_value.type = value.type;
switch (value.type) {
case QEMU_PLUGIN_MEM_VALUE_U8:
- {
- uint8_t val = value.data.u8;
- uint8_t *p = ri_data;
- if (is_store) {
- *p = val;
- } else {
- unseen_data = *p != val;
- }
+ swapped_value.data.u8 = value.data.u8;
+ val_ptr = &swapped_value.data.u8;
+ val_size = 1;
break;
- }
case QEMU_PLUGIN_MEM_VALUE_U16:
- {
- uint16_t val = be ? GUINT16_FROM_BE(value.data.u16) :
- GUINT16_FROM_LE(value.data.u16);
- uint16_t *p = ri_data;
- if (is_store) {
- *p = val;
- } else {
- unseen_data = *p != val;
- }
+ swapped_value.data.u16 = be ? GUINT16_FROM_BE(value.data.u16) :
+ GUINT16_FROM_LE(value.data.u16);
+ val_ptr = &swapped_value.data.u16;
+ val_size = 2;
break;
- }
case QEMU_PLUGIN_MEM_VALUE_U32:
- {
- uint32_t val = be ? GUINT32_FROM_BE(value.data.u32) :
- GUINT32_FROM_LE(value.data.u32);
- uint32_t *p = ri_data;
- if (is_store) {
- *p = val;
- } else {
- unseen_data = *p != val;
- }
+ swapped_value.data.u32 = be ? GUINT32_FROM_BE(value.data.u32) :
+ GUINT32_FROM_LE(value.data.u32);
+ val_ptr = &swapped_value.data.u32;
+ val_size = 4;
break;
- }
case QEMU_PLUGIN_MEM_VALUE_U64:
- {
- uint64_t val = be ? GUINT64_FROM_BE(value.data.u64) :
- GUINT64_FROM_LE(value.data.u64);
- uint64_t *p = ri_data;
- if (is_store) {
- *p = val;
- } else {
- unseen_data = *p != val;
- }
+ swapped_value.data.u64 = be ? GUINT64_FROM_BE(value.data.u64) :
+ GUINT64_FROM_LE(value.data.u64);
+ val_ptr = &swapped_value.data.u64;
+ val_size = 8;
break;
- }
case QEMU_PLUGIN_MEM_VALUE_U128:
- /* non in test so skip */
- break;
+ /* none in test so skip */
+ goto done;
default:
g_assert_not_reached();
}
+ /* ri_data may not be aligned, so we use memcpy/memcmp */
+ if (is_store) {
+ memcpy(ri_data, val_ptr, val_size);
+ } else {
+ unseen_data = memcmp(ri_data, val_ptr, val_size) != 0;
+ }
+
/*
* This is expected for regions initialised by QEMU (.text etc) but we
* expect to see all data read and written to the test_data region
@@ -213,6 +201,7 @@ static void update_region_info(uint64_t region, uint64_t offset,
ri->seen_all = false;
}
+done:
g_mutex_unlock(&lock);
}
--
2.43.0Peter Maydell <peter.maydell@linaro.org> writes:
> In commit eb3f69cac62670 we removed the dependency of this
> mem plugin on the QEMU headers, but in doing that we introduced
> undefined behaviour when the plugin accesses unaligned memory.
> This shows up if you build with the gcc or clang address
ubsan
> sanitizer and run 'make check-tcg', in numerous warnings like:
>
> ../../tests/tcg/plugins/mem.c:167:27: runtime error: load of misaligned address 0x7f1f300354b1 for type 'uint16_t' (aka 'unsigned short'), which requires 2 byte alignment
> 0x7f1f300354b1: note: pointer points here
> 00 00 00 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 14 15 16 17 18 19 1a 1b 1c
> ^
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../tests/tcg/plugins/mem.c:167:27
>
> Fix this by rearranging the data reads and writes to use
> memcpy() instead.
>
> Fixes: eb3f69cac62670 ("tests/tcg/plugins/mem.c: remove dependency on qemu headers")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
with the extra clarity on the sanitiser:
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> tests/tcg/plugins/mem.c | 71 +++++++++++++++++------------------------
> 1 file changed, 30 insertions(+), 41 deletions(-)
>
> diff --git a/tests/tcg/plugins/mem.c b/tests/tcg/plugins/mem.c
> index 7d64e7018f..f3992abc8f 100644
> --- a/tests/tcg/plugins/mem.c
> +++ b/tests/tcg/plugins/mem.c
> @@ -123,6 +123,9 @@ static void update_region_info(uint64_t region, uint64_t offset,
> bool is_store = qemu_plugin_mem_is_store(meminfo);
> RegionInfo *ri;
> bool unseen_data = false;
> + void *val_ptr;
> + unsigned int val_size;
> + qemu_plugin_mem_value swapped_value;
>
> g_assert(offset + size <= region_size);
>
> @@ -144,61 +147,46 @@ static void update_region_info(uint64_t region, uint64_t offset,
> }
>
> void *ri_data = &ri->data[offset];
> +
> + swapped_value.type = value.type;
> switch (value.type) {
> case QEMU_PLUGIN_MEM_VALUE_U8:
> - {
> - uint8_t val = value.data.u8;
> - uint8_t *p = ri_data;
> - if (is_store) {
> - *p = val;
> - } else {
> - unseen_data = *p != val;
> - }
> + swapped_value.data.u8 = value.data.u8;
> + val_ptr = &swapped_value.data.u8;
> + val_size = 1;
> break;
> - }
> case QEMU_PLUGIN_MEM_VALUE_U16:
> - {
> - uint16_t val = be ? GUINT16_FROM_BE(value.data.u16) :
> - GUINT16_FROM_LE(value.data.u16);
> - uint16_t *p = ri_data;
> - if (is_store) {
> - *p = val;
> - } else {
> - unseen_data = *p != val;
> - }
> + swapped_value.data.u16 = be ? GUINT16_FROM_BE(value.data.u16) :
> + GUINT16_FROM_LE(value.data.u16);
> + val_ptr = &swapped_value.data.u16;
> + val_size = 2;
> break;
> - }
> case QEMU_PLUGIN_MEM_VALUE_U32:
> - {
> - uint32_t val = be ? GUINT32_FROM_BE(value.data.u32) :
> - GUINT32_FROM_LE(value.data.u32);
> - uint32_t *p = ri_data;
> - if (is_store) {
> - *p = val;
> - } else {
> - unseen_data = *p != val;
> - }
> + swapped_value.data.u32 = be ? GUINT32_FROM_BE(value.data.u32) :
> + GUINT32_FROM_LE(value.data.u32);
> + val_ptr = &swapped_value.data.u32;
> + val_size = 4;
> break;
> - }
> case QEMU_PLUGIN_MEM_VALUE_U64:
> - {
> - uint64_t val = be ? GUINT64_FROM_BE(value.data.u64) :
> - GUINT64_FROM_LE(value.data.u64);
> - uint64_t *p = ri_data;
> - if (is_store) {
> - *p = val;
> - } else {
> - unseen_data = *p != val;
> - }
> + swapped_value.data.u64 = be ? GUINT64_FROM_BE(value.data.u64) :
> + GUINT64_FROM_LE(value.data.u64);
> + val_ptr = &swapped_value.data.u64;
> + val_size = 8;
> break;
> - }
> case QEMU_PLUGIN_MEM_VALUE_U128:
> - /* non in test so skip */
> - break;
> + /* none in test so skip */
> + goto done;
> default:
> g_assert_not_reached();
> }
>
> + /* ri_data may not be aligned, so we use memcpy/memcmp */
> + if (is_store) {
> + memcpy(ri_data, val_ptr, val_size);
> + } else {
> + unseen_data = memcmp(ri_data, val_ptr, val_size) != 0;
> + }
> +
> /*
> * This is expected for regions initialised by QEMU (.text etc) but we
> * expect to see all data read and written to the test_data region
> @@ -213,6 +201,7 @@ static void update_region_info(uint64_t region, uint64_t offset,
> ri->seen_all = false;
> }
>
> +done:
> g_mutex_unlock(&lock);
> }
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
On Thu, 5 Mar 2026 at 12:01, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In commit eb3f69cac62670 we removed the dependency of this
> mem plugin on the QEMU headers, but in doing that we introduced
> undefined behaviour when the plugin accesses unaligned memory.
> This shows up if you build with the gcc or clang address
> sanitizer and run 'make check-tcg', in numerous warnings like:
This should read "with the gcc or clang undefined behaviour
sanitizer", which then lines up with what the error message
below reports itself as:
>
> ../../tests/tcg/plugins/mem.c:167:27: runtime error: load of misaligned address 0x7f1f300354b1 for type 'uint16_t' (aka 'unsigned short'), which requires 2 byte alignment
> 0x7f1f300354b1: note: pointer points here
> 00 00 00 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 14 15 16 17 18 19 1a 1b 1c
> ^
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../tests/tcg/plugins/mem.c:167:27
Specifically, I see this with
'../../configure' '--cc=clang' '--cxx=clang++' '--enable-ubsan'
Here's a specific failure;
timeout -s KILL --foreground 120
/home/pm215/qemu/build/clang/qemu-system-aarch64 -monitor none
-display none -chardev file,path=run-plugin-memory-with-libmem.s
o.out,id=output -plugin ../plugins/libmem.so,region-summary=true -d
plugin -D memory-with-libmem.so.pout -M virt -cpu max -display none
-semihosting-config enabl
e=on,target=native,chardev=output -kernel memory
../../tests/tcg/plugins/mem.c:167:27: runtime error: load of
misaligned address 0x754e340357c1 for type 'uint16_t' (aka 'unsigned
short'), which requires 2 byte
alignment
0x754e340357c1: note: pointer points here
00 00 00 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12
13 14 15 16 17 18 19 1a 1b 1c
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
../../tests/tcg/plugins/mem.c:167:27
../../tests/tcg/plugins/mem.c:179:27: runtime error: load of
misaligned address 0x754e340357c1 for type 'uint32_t' (aka 'unsigned
int'), which requires 4 byte alignment
0x754e340357c1: note: pointer points here
00 00 00 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12
13 14 15 16 17 18 19 1a 1b 1c
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
../../tests/tcg/plugins/mem.c:179:27
../../tests/tcg/plugins/mem.c:191:27: runtime error: load of
misaligned address 0x754e340357c1 for type 'uint64_t' (aka 'unsigned
long'), which requires 8 byte alignment
0x754e340357c1: note: pointer points here
00 00 00 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12
13 14 15 16 17 18 19 1a 1b 1c
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
../../tests/tcg/plugins/mem.c:191:27
../../tests/tcg/plugins/mem.c:165:13: runtime error: store to
misaligned address 0x754e340357c1 for type 'uint16_t' (aka 'unsigned
short'), which requires 2 byte alignment
0x754e340357c1: note: pointer points here
00 00 00 00 00 00 00 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12
13 14 15 16 17 18 19 1a 1b 1c
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
../../tests/tcg/plugins/mem.c:165:13
../../tests/tcg/plugins/mem.c:177:13: runtime error: store to
misaligned address 0x754e340357c1 for type 'uint32_t' (aka 'unsigned
int'), which requires 4 byte alignment
0x754e340357c1: note: pointer points here
00 00 00 00 00 00 00 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12
13 14 15 16 17 18 19 1a 1b 1c
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
../../tests/tcg/plugins/mem.c:177:13
../../tests/tcg/plugins/mem.c:189:13: runtime error: store to
misaligned address 0x754e340357c1 for type 'uint64_t' (aka 'unsigned
long'), which requires 8 byte alignment
0x754e340357c1: note: pointer points here
00 00 00 00 00 00 00 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12
13 14 15 16 17 18 19 1a 1b 1c
^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
../../tests/tcg/plugins/mem.c:189:13
Similar with i386 and x86-64 targets.
-- PMM
On 3/5/26 6:42 AM, Peter Maydell wrote: > On Thu, 5 Mar 2026 at 12:01, Peter Maydell <peter.maydell@linaro.org> wrote: >> >> In commit eb3f69cac62670 we removed the dependency of this >> mem plugin on the QEMU headers, but in doing that we introduced >> undefined behaviour when the plugin accesses unaligned memory. >> This shows up if you build with the gcc or clang address >> sanitizer and run 'make check-tcg', in numerous warnings like: > > This should read "with the gcc or clang undefined behaviour > sanitizer", which then lines up with what the error message > below reports itself as: > >> >> ../../tests/tcg/plugins/mem.c:167:27: runtime error: load of misaligned address 0x7f1f300354b1 for type 'uint16_t' (aka 'unsigned short'), which requires 2 byte alignment >> 0x7f1f300354b1: note: pointer points here >> 00 00 00 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 14 15 16 17 18 19 1a 1b 1c >> ^ >> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../tests/tcg/plugins/mem.c:167:27 > > Specifically, I see this with > '../../configure' '--cc=clang' '--cxx=clang++' '--enable-ubsan' > > Here's a specific failure; > > timeout -s KILL --foreground 120 > /home/pm215/qemu/build/clang/qemu-system-aarch64 -monitor none > -display none -chardev file,path=run-plugin-memory-with-libmem.s > o.out,id=output -plugin ../plugins/libmem.so,region-summary=true -d > plugin -D memory-with-libmem.so.pout -M virt -cpu max -display none > -semihosting-config enabl > e=on,target=native,chardev=output -kernel memory > ../../tests/tcg/plugins/mem.c:167:27: runtime error: load of > misaligned address 0x754e340357c1 for type 'uint16_t' (aka 'unsigned > short'), which requires 2 byte > alignment > 0x754e340357c1: note: pointer points here > 00 00 00 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 > 13 14 15 16 17 18 19 1a 1b 1c > ^ > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior > ../../tests/tcg/plugins/mem.c:167:27 > ../../tests/tcg/plugins/mem.c:179:27: runtime error: load of > misaligned address 0x754e340357c1 for type 'uint32_t' (aka 'unsigned > int'), which requires 4 byte alignment > 0x754e340357c1: note: pointer points here > 00 00 00 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 > 13 14 15 16 17 18 19 1a 1b 1c > ^ > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior > ../../tests/tcg/plugins/mem.c:179:27 > ../../tests/tcg/plugins/mem.c:191:27: runtime error: load of > misaligned address 0x754e340357c1 for type 'uint64_t' (aka 'unsigned > long'), which requires 8 byte alignment > 0x754e340357c1: note: pointer points here > 00 00 00 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 > 13 14 15 16 17 18 19 1a 1b 1c > ^ > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior > ../../tests/tcg/plugins/mem.c:191:27 > ../../tests/tcg/plugins/mem.c:165:13: runtime error: store to > misaligned address 0x754e340357c1 for type 'uint16_t' (aka 'unsigned > short'), which requires 2 byte alignment > 0x754e340357c1: note: pointer points here > 00 00 00 00 00 00 00 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 > 13 14 15 16 17 18 19 1a 1b 1c > ^ > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior > ../../tests/tcg/plugins/mem.c:165:13 > ../../tests/tcg/plugins/mem.c:177:13: runtime error: store to > misaligned address 0x754e340357c1 for type 'uint32_t' (aka 'unsigned > int'), which requires 4 byte alignment > 0x754e340357c1: note: pointer points here > 00 00 00 00 00 00 00 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 > 13 14 15 16 17 18 19 1a 1b 1c > ^ > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior > ../../tests/tcg/plugins/mem.c:177:13 > ../../tests/tcg/plugins/mem.c:189:13: runtime error: store to > misaligned address 0x754e340357c1 for type 'uint64_t' (aka 'unsigned > long'), which requires 8 byte alignment > 0x754e340357c1: note: pointer points here > 00 00 00 00 00 00 00 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 > 13 14 15 16 17 18 19 1a 1b 1c > ^ > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior > ../../tests/tcg/plugins/mem.c:189:13 > > Similar with i386 and x86-64 targets. > > -- PMM Thanks for fixing it Peter. It would be nice to run this in CI by default one day. Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
© 2016 - 2026 Red Hat, Inc.