The MIPS GIC does a check for a guest error in the write path for the
SH_MAP*_VP registers which triggers a Coverity complaint because it
assigns -1 to a uint64_t. The code doesn't misbehave because the -1
case will be caught by the following OFFSET_CHECK(), but the code
could be improved:
 * there is no need to special case to avoid passing 0 to ctz64(),
   because (unlike the compiler builtins) QEMU defines that this
   has a specific behaviour, returning 64
 * the OFFSET_CHECK() macro will go to the "bad_offset" label and
   print an error implying that the guest wrote to an invalid
   register offset. This is misleading about the actual problem,
   which is that the guest wrote a bogus value to a valid register
   offset

Make the error check print a better log message, and avoid the
special casing on ctz64(); in passing, this should also make
Coverity happier.

CID: 1547545
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/mips_gic.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/intc/mips_gic.c b/hw/intc/mips_gic.c
index ad9363a4c7..d337a76603 100644
--- a/hw/intc/mips_gic.c
+++ b/hw/intc/mips_gic.c
@@ -326,9 +326,13 @@ static void gic_write(void *opaque, hwaddr addr, uint64_t data, unsigned size)
         /* up to 32 bytes per a pin */
         irq_src = (addr - GIC_SH_MAP0_VP_OFS) / 32;
         OFFSET_CHECK(irq_src < gic->num_irq);
-        data = data ? ctz64(data) : -1;
-        OFFSET_CHECK(data < gic->num_vps);
-        gic->irq_state[irq_src].map_vp = data;
+        if (ctz64(data) >= gic->num_vps) {
+            qemu_log_mask(LOG_GUEST_ERROR, "Bad data value 0x%" PRIx64
+                          " at MAP VP register offset 0x%" PRIx64 "\n",
+                          data, addr);
+            break;
+        }
+        gic->irq_state[irq_src].map_vp = ctz64(data);
         break;
     case VP_LOCAL_SECTION_OFS ... (VP_LOCAL_SECTION_OFS + GIC_VL_BRK_GROUP):
         gic_write_vp(gic, vp_index, addr - VP_LOCAL_SECTION_OFS, data, size);
-- 
2.43.0

On 12/5/26 13:15, Peter Maydell wrote:
> The MIPS GIC does a check for a guest error in the write path for the
> SH_MAP*_VP registers which triggers a Coverity complaint because it
> assigns -1 to a uint64_t. The code doesn't misbehave because the -1
> case will be caught by the following OFFSET_CHECK(), but the code
> could be improved:
>   * there is no need to special case to avoid passing 0 to ctz64(),
>     because (unlike the compiler builtins) QEMU defines that this
>     has a specific behaviour, returning 64
>   * the OFFSET_CHECK() macro will go to the "bad_offset" label and
>     print an error implying that the guest wrote to an invalid
>     register offset. This is misleading about the actual problem,
>     which is that the guest wrote a bogus value to a valid register
>     offset
> 
> Make the error check print a better log message, and avoid the
> special casing on ctz64(); in passing, this should also make
> Coverity happier.
> 
> CID: 1547545
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/intc/mips_gic.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)

Patch queued, thanks.

On 12/5/26 13:15, Peter Maydell wrote:
> The MIPS GIC does a check for a guest error in the write path for the
> SH_MAP*_VP registers which triggers a Coverity complaint because it
> assigns -1 to a uint64_t. The code doesn't misbehave because the -1
> case will be caught by the following OFFSET_CHECK(), but the code
> could be improved:
>   * there is no need to special case to avoid passing 0 to ctz64(),
>     because (unlike the compiler builtins) QEMU defines that this
>     has a specific behaviour, returning 64
>   * the OFFSET_CHECK() macro will go to the "bad_offset" label and
>     print an error implying that the guest wrote to an invalid
>     register offset. This is misleading about the actual problem,
>     which is that the guest wrote a bogus value to a valid register
>     offset
> 
> Make the error check print a better log message, and avoid the
> special casing on ctz64(); in passing, this should also make
> Coverity happier.
> 
> CID: 1547545
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/intc/mips_gic.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)

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