1. Patchew
  2. QEMU
  3. View series

[PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:

Nabih Estefan posted 1 patch 6 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250429155621.2028198-1-nabihestefan@google.com
Maintainers: Akihiko Odaki <akihiko.odaki@daynix.com>, Sriram Yagnaraman <sriram.yagnaraman@ericsson.com>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
tests/qtest/libqos/igb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
Posted by Nabih Estefan 6 months, 2 weeks ago 
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,
-- 
2.49.0.901.g37484f566f-goog
Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
Posted by Peter Maydell 6 months, 2 weeks ago 
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
Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
Posted by Nabih Estefan 6 months, 2 weeks ago 
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
Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
Posted by Alex Bennée 6 months, 2 weeks ago 
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
Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
Posted by Alex Bennée 6 months, 2 weeks ago 
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
Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
Posted by Philippe Mathieu-Daudé 6 months, 2 weeks ago 
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.
>
Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
Posted by Laurent Vivier 6 months, 2 weeks ago 
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>
Re: [PATCH v2] Running with `--enable-ubsan` leads to a qtest failure:
Posted by Richard Henderson 6 months, 2 weeks ago 
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,

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.