* Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
2025-04-29 15:56 [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure: Nabih Estefan
@ 2025-04-30 2:48 ` Richard Henderson
2025-04-30 9:18 ` Laurent Vivier
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Richard Henderson @ 2025-04-30 2:48 UTC (permalink / raw)
To: qemu-devel
On 4/29/25 08:56, Nabih Estefan wrote:
> v2: used ldl_le_p and lduw_l_p instead of memcpy as per upstream
> suggestion.
>
> ```
> ../tests/qtest/libqos/igb.c:106:5: runtime error: load of misaligned address 0x562040be8e33 for type 'uint32_t', which requires 4 byte alignment
> ```
> Instead of straight casting the uint8_t array, we use memcpy to assure
> alignment is correct against uint32_t and uint16_t.
>
> Signed-off-by: Nabih Estefan <nabihestefan@google.com>
> ---
> tests/qtest/libqos/igb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
>
> diff --git a/tests/qtest/libqos/igb.c b/tests/qtest/libqos/igb.c
> index f40c4ec4cd..2e0bb58617 100644
> --- a/tests/qtest/libqos/igb.c
> +++ b/tests/qtest/libqos/igb.c
> @@ -104,10 +104,10 @@ static void igb_pci_start_hw(QOSGraphObject *obj)
> e1000e_macreg_write(&d->e1000e, E1000_RDT(0), 0);
> e1000e_macreg_write(&d->e1000e, E1000_RDH(0), 0);
> e1000e_macreg_write(&d->e1000e, E1000_RA,
> - le32_to_cpu(*(uint32_t *)address));
> + ldl_le_p((uint32_t *)address));
> e1000e_macreg_write(&d->e1000e, E1000_RA + 4,
> E1000_RAH_AV | E1000_RAH_POOL_1 |
> - le16_to_cpu(*(uint16_t *)(address + 4)));
> + lduw_le_p((uint16_t *)(address + 4)));
>
> /* Set supported receive descriptor mode */
> e1000e_macreg_write(&d->e1000e,
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
2025-04-29 15:56 [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure: Nabih Estefan
2025-04-30 2:48 ` Richard Henderson
@ 2025-04-30 9:18 ` Laurent Vivier
2025-04-30 10:49 ` Alex Bennée
2025-04-30 12:03 ` Peter Maydell
3 siblings, 0 replies; 8+ messages in thread
From: Laurent Vivier @ 2025-04-30 9:18 UTC (permalink / raw)
To: Nabih Estefan, qemu-devel
Cc: pbonzini, farosas, sriram.yagnaraman, akihiko.odaki, QEMU Trivial
CC Trivial
On 29/04/2025 17:56, Nabih Estefan wrote:
> v2: used ldl_le_p and lduw_l_p instead of memcpy as per upstream
> suggestion.
>
> ```
> ../tests/qtest/libqos/igb.c:106:5: runtime error: load of misaligned address 0x562040be8e33 for type 'uint32_t', which requires 4 byte alignment
> ```
> Instead of straight casting the uint8_t array, we use memcpy to assure
> alignment is correct against uint32_t and uint16_t.
>
> Signed-off-by: Nabih Estefan <nabihestefan@google.com>
> ---
> tests/qtest/libqos/igb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/qtest/libqos/igb.c b/tests/qtest/libqos/igb.c
> index f40c4ec4cd..2e0bb58617 100644
> --- a/tests/qtest/libqos/igb.c
> +++ b/tests/qtest/libqos/igb.c
> @@ -104,10 +104,10 @@ static void igb_pci_start_hw(QOSGraphObject *obj)
> e1000e_macreg_write(&d->e1000e, E1000_RDT(0), 0);
> e1000e_macreg_write(&d->e1000e, E1000_RDH(0), 0);
> e1000e_macreg_write(&d->e1000e, E1000_RA,
> - le32_to_cpu(*(uint32_t *)address));
> + ldl_le_p((uint32_t *)address));
> e1000e_macreg_write(&d->e1000e, E1000_RA + 4,
> E1000_RAH_AV | E1000_RAH_POOL_1 |
> - le16_to_cpu(*(uint16_t *)(address + 4)));
> + lduw_le_p((uint16_t *)(address + 4)));
>
> /* Set supported receive descriptor mode */
> e1000e_macreg_write(&d->e1000e,
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Tested-by: Laurent Vivier <lvivier@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
2025-04-29 15:56 [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure: Nabih Estefan
2025-04-30 2:48 ` Richard Henderson
2025-04-30 9:18 ` Laurent Vivier
@ 2025-04-30 10:49 ` Alex Bennée
2025-04-30 12:31 ` Philippe Mathieu-Daudé
2025-04-30 12:03 ` Peter Maydell
3 siblings, 1 reply; 8+ messages in thread
From: Alex Bennée @ 2025-04-30 10:49 UTC (permalink / raw)
To: Nabih Estefan
Cc: qemu-devel, pbonzini, lvivier, farosas, sriram.yagnaraman,
akihiko.odaki
Nabih Estefan <nabihestefan@google.com> writes:
> v2: used ldl_le_p and lduw_l_p instead of memcpy as per upstream
> suggestion.
>
> ```
> ../tests/qtest/libqos/igb.c:106:5: runtime error: load of misaligned address 0x562040be8e33 for type 'uint32_t', which requires 4 byte alignment
> ```
> Instead of straight casting the uint8_t array, we use memcpy to assure
> alignment is correct against uint32_t and uint16_t.
Queued to testing/next, thanks.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
2025-04-30 10:49 ` Alex Bennée
@ 2025-04-30 12:31 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-04-30 12:31 UTC (permalink / raw)
To: Alex Bennée, Nabih Estefan
Cc: qemu-devel, pbonzini, lvivier, farosas, sriram.yagnaraman,
akihiko.odaki
Subject is missing. What about "tests/qtest/igb: Avoid misaligned access"?
On 30/4/25 12:49, Alex Bennée wrote:
> Nabih Estefan <nabihestefan@google.com> writes:
>
>> v2: used ldl_le_p and lduw_l_p instead of memcpy as per upstream
>> suggestion.
This comment is not useful in git history.
>>
>> ```
>> ../tests/qtest/libqos/igb.c:106:5: runtime error: load of misaligned address 0x562040be8e33 for type 'uint32_t', which requires 4 byte alignment
>> ```
>> Instead of straight casting the uint8_t array, we use memcpy to assure
>> alignment is correct against uint32_t and uint16_t.
>
> Queued to testing/next, thanks.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
2025-04-29 15:56 [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure: Nabih Estefan
` (2 preceding siblings ...)
2025-04-30 10:49 ` Alex Bennée
@ 2025-04-30 12:03 ` Peter Maydell
2025-04-30 16:48 ` Nabih Estefan
3 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2025-04-30 12:03 UTC (permalink / raw)
To: Nabih Estefan
Cc: qemu-devel, pbonzini, lvivier, farosas, sriram.yagnaraman,
akihiko.odaki
On Tue, 29 Apr 2025 at 16:56, Nabih Estefan <nabihestefan@google.com> wrote:
>
> v2: used ldl_le_p and lduw_l_p instead of memcpy as per upstream
> suggestion.
>
> ```
> ../tests/qtest/libqos/igb.c:106:5: runtime error: load of misaligned address 0x562040be8e33 for type 'uint32_t', which requires 4 byte alignment
> ```
> Instead of straight casting the uint8_t array, we use memcpy to assure
> alignment is correct against uint32_t and uint16_t.
>
> Signed-off-by: Nabih Estefan <nabihestefan@google.com>
> ---
> tests/qtest/libqos/igb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/qtest/libqos/igb.c b/tests/qtest/libqos/igb.c
> index f40c4ec4cd..2e0bb58617 100644
> --- a/tests/qtest/libqos/igb.c
> +++ b/tests/qtest/libqos/igb.c
> @@ -104,10 +104,10 @@ static void igb_pci_start_hw(QOSGraphObject *obj)
> e1000e_macreg_write(&d->e1000e, E1000_RDT(0), 0);
> e1000e_macreg_write(&d->e1000e, E1000_RDH(0), 0);
> e1000e_macreg_write(&d->e1000e, E1000_RA,
> - le32_to_cpu(*(uint32_t *)address));
> + ldl_le_p((uint32_t *)address));
> e1000e_macreg_write(&d->e1000e, E1000_RA + 4,
> E1000_RAH_AV | E1000_RAH_POOL_1 |
> - le16_to_cpu(*(uint16_t *)(address + 4)));
> + lduw_le_p((uint16_t *)(address + 4)));
ldl_le_p() etc take a 'void *' -- the casts here should not be
necessary.
thanks
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
2025-04-30 12:03 ` Peter Maydell
@ 2025-04-30 16:48 ` Nabih Estefan
2025-04-30 17:06 ` Alex Bennée
0 siblings, 1 reply; 8+ messages in thread
From: Nabih Estefan @ 2025-04-30 16:48 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, pbonzini, lvivier, farosas, sriram.yagnaraman,
akihiko.odaki
On Wed, Apr 30, 2025 at 5:03 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 29 Apr 2025 at 16:56, Nabih Estefan <nabihestefan@google.com> wrote:
> >
> > v2: used ldl_le_p and lduw_l_p instead of memcpy as per upstream
> > suggestion.
> >
> > ```
> > ../tests/qtest/libqos/igb.c:106:5: runtime error: load of misaligned address 0x562040be8e33 for type 'uint32_t', which requires 4 byte alignment
> > ```
> > Instead of straight casting the uint8_t array, we use memcpy to assure
> > alignment is correct against uint32_t and uint16_t.
> >
> > Signed-off-by: Nabih Estefan <nabihestefan@google.com>
> > ---
> > tests/qtest/libqos/igb.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/qtest/libqos/igb.c b/tests/qtest/libqos/igb.c
> > index f40c4ec4cd..2e0bb58617 100644
> > --- a/tests/qtest/libqos/igb.c
> > +++ b/tests/qtest/libqos/igb.c
> > @@ -104,10 +104,10 @@ static void igb_pci_start_hw(QOSGraphObject *obj)
> > e1000e_macreg_write(&d->e1000e, E1000_RDT(0), 0);
> > e1000e_macreg_write(&d->e1000e, E1000_RDH(0), 0);
> > e1000e_macreg_write(&d->e1000e, E1000_RA,
> > - le32_to_cpu(*(uint32_t *)address));
> > + ldl_le_p((uint32_t *)address));
> > e1000e_macreg_write(&d->e1000e, E1000_RA + 4,
> > E1000_RAH_AV | E1000_RAH_POOL_1 |
> > - le16_to_cpu(*(uint16_t *)(address + 4)));
> > + lduw_le_p((uint16_t *)(address + 4)));
>
> ldl_le_p() etc take a 'void *' -- the casts here should not be
> necessary.
Should I send a new patch to fix this if it's already been queued to
testing/next?
Or can it be fixed directly in that branch?
Thanks,
Nabih
>
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
2025-04-30 16:48 ` Nabih Estefan
@ 2025-04-30 17:06 ` Alex Bennée
0 siblings, 0 replies; 8+ messages in thread
From: Alex Bennée @ 2025-04-30 17:06 UTC (permalink / raw)
To: Nabih Estefan
Cc: Peter Maydell, qemu-devel, pbonzini, lvivier, farosas,
sriram.yagnaraman, akihiko.odaki
Nabih Estefan <nabihestefan@google.com> writes:
> On Wed, Apr 30, 2025 at 5:03 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Tue, 29 Apr 2025 at 16:56, Nabih Estefan <nabihestefan@google.com> wrote:
>> >
>> > v2: used ldl_le_p and lduw_l_p instead of memcpy as per upstream
>> > suggestion.
>> >
>> > ```
>> > ../tests/qtest/libqos/igb.c:106:5: runtime error: load of misaligned address 0x562040be8e33 for type 'uint32_t', which requires 4 byte alignment
>> > ```
>> > Instead of straight casting the uint8_t array, we use memcpy to assure
>> > alignment is correct against uint32_t and uint16_t.
>> >
>> > Signed-off-by: Nabih Estefan <nabihestefan@google.com>
>> > ---
>> > tests/qtest/libqos/igb.c | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/tests/qtest/libqos/igb.c b/tests/qtest/libqos/igb.c
>> > index f40c4ec4cd..2e0bb58617 100644
>> > --- a/tests/qtest/libqos/igb.c
>> > +++ b/tests/qtest/libqos/igb.c
>> > @@ -104,10 +104,10 @@ static void igb_pci_start_hw(QOSGraphObject *obj)
>> > e1000e_macreg_write(&d->e1000e, E1000_RDT(0), 0);
>> > e1000e_macreg_write(&d->e1000e, E1000_RDH(0), 0);
>> > e1000e_macreg_write(&d->e1000e, E1000_RA,
>> > - le32_to_cpu(*(uint32_t *)address));
>> > + ldl_le_p((uint32_t *)address));
>> > e1000e_macreg_write(&d->e1000e, E1000_RA + 4,
>> > E1000_RAH_AV | E1000_RAH_POOL_1 |
>> > - le16_to_cpu(*(uint16_t *)(address + 4)));
>> > + lduw_le_p((uint16_t *)(address + 4)));
>>
>> ldl_le_p() etc take a 'void *' -- the casts here should not be
>> necessary.
>
> Should I send a new patch to fix this if it's already been queued to
> testing/next?
> Or can it be fixed directly in that branch?
I'll fix it up, I've taken notes when I re-base.
>
> Thanks,
> Nabih
>
>>
>> thanks
>> -- PMM
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 8+ messages in thread