[PATCH] hw/mips/malta: Silence warning from ubsan

[PATCH] hw/mips/malta: Silence warning from ubsan

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

When compiling QEMU with --enable-ubsan there is a undefined behavior
warning when using the malta machine:

 hw/mips/malta.c:1200:32: runtime error: addition of unsigned offset
  to 0x7fb620600000 overflowed to 0x7fb6205fffff
 SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior hw/mips/malta.c:1200:32

To fix the issue, check the bios_size whether we really loaded the
firmware before trying to byte-swap the instructions here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/mips/malta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index cbdbb210568..47dd4016cfd 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1190,7 +1190,7 @@ void mips_malta_init(MachineState *machine)
          * In little endian mode the 32bit words in the bios are swapped,
          * a neat trick which allows bi-endian firmware.
          */
-        if (!TARGET_BIG_ENDIAN) {
+        if (!TARGET_BIG_ENDIAN && bios_size > 0) {
             uint32_t *end, *addr;
             const size_t swapsize = MIN(bios_size, 0x3e0000);
             addr = rom_ptr(FLASH_ADDRESS, swapsize);
-- 
2.50.1

Re: [PATCH] hw/mips/malta: Silence warning from ubsan

[Philippe Mathieu-Daudé]

Hi Thomas,

On 28/7/25 13:51, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> When compiling QEMU with --enable-ubsan there is a undefined behavior
> warning when using the malta machine:
> 
>   hw/mips/malta.c:1200:32: runtime error: addition of unsigned offset
>    to 0x7fb620600000 overflowed to 0x7fb6205fffff
>   SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior hw/mips/malta.c:1200:32
> 
> To fix the issue, check the bios_size whether we really loaded the
> firmware before trying to byte-swap the instructions here.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   hw/mips/malta.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
> index cbdbb210568..47dd4016cfd 100644
> --- a/hw/mips/malta.c
> +++ b/hw/mips/malta.c
> @@ -1190,7 +1190,7 @@ void mips_malta_init(MachineState *machine)
>            * In little endian mode the 32bit words in the bios are swapped,
>            * a neat trick which allows bi-endian firmware.
>            */
> -        if (!TARGET_BIG_ENDIAN) {
> +        if (!TARGET_BIG_ENDIAN && bios_size > 0) {
>               uint32_t *end, *addr;
>               const size_t swapsize = MIN(bios_size, 0x3e0000);
>               addr = rom_ptr(FLASH_ADDRESS, swapsize);

What about:

-- >8 --
@@ -1180,7 +1180,7 @@ void mips_malta_init(MachineState *machine)
              } else {
                  bios_size = -1;
              }
-            if ((bios_size < 0 || bios_size > BIOS_SIZE) &&
+            if ((bios_size <= 0 || bios_size > BIOS_SIZE) &&
                  machine->firmware && !qtest_enabled()) {
                  error_report("Could not load MIPS bios '%s'", 
machine->firmware);
                  exit(1);
---

?

Re: [PATCH] hw/mips/malta: Silence warning from ubsan

[Philippe Mathieu-Daudé]

On 28/7/25 15:25, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 28/7/25 13:51, Thomas Huth wrote:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> When compiling QEMU with --enable-ubsan there is a undefined behavior
>> warning when using the malta machine:
>>
>>   hw/mips/malta.c:1200:32: runtime error: addition of unsigned offset
>>    to 0x7fb620600000 overflowed to 0x7fb6205fffff
>>   SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior hw/mips/ 
>> malta.c:1200:32
>>
>> To fix the issue, check the bios_size whether we really loaded the
>> firmware before trying to byte-swap the instructions here.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   hw/mips/malta.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
>> index cbdbb210568..47dd4016cfd 100644
>> --- a/hw/mips/malta.c
>> +++ b/hw/mips/malta.c
>> @@ -1190,7 +1190,7 @@ void mips_malta_init(MachineState *machine)
>>            * In little endian mode the 32bit words in the bios are 
>> swapped,
>>            * a neat trick which allows bi-endian firmware.
>>            */
>> -        if (!TARGET_BIG_ENDIAN) {
>> +        if (!TARGET_BIG_ENDIAN && bios_size > 0) {
>>               uint32_t *end, *addr;
>>               const size_t swapsize = MIN(bios_size, 0x3e0000);
>>               addr = rom_ptr(FLASH_ADDRESS, swapsize);

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

and queued, thanks!

> 
> What about:
> 
> -- >8 --
> @@ -1180,7 +1180,7 @@ void mips_malta_init(MachineState *machine)
>               } else {
>                   bios_size = -1;
>               }
> -            if ((bios_size < 0 || bios_size > BIOS_SIZE) &&
> +            if ((bios_size <= 0 || bios_size > BIOS_SIZE) &&
>                   machine->firmware && !qtest_enabled()) {
>                   error_report("Could not load MIPS bios '%s'", machine- 
>  >firmware);
>                   exit(1);
> ---
> 
> ?