On 7/12/22 13:31, Xiaojuan Yang wrote:
> Fix coverity errors:
> 1. In find_first_bit function, the 'size' argument need
> 'unsigned long' type, so we add the suffix 'UL' on 'size'
> argument when use the function.
> 2. In expression 1ULL << irq, left shifting by more than
> 63 bits has undefined behavior. And out-of-bounds access
> error occured when 'irq' >= 64. So we add a condition to
> avoid this.
>
> Fix coverity CID: 1489761 1489764 1489765
>
> Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> ---
> hw/intc/loongarch_pch_pic.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/hw/intc/loongarch_pch_pic.c b/hw/intc/loongarch_pch_pic.c
> index 3c9814a3b4..779a087a03 100644
> --- a/hw/intc/loongarch_pch_pic.c
> +++ b/hw/intc/loongarch_pch_pic.c
> @@ -21,16 +21,20 @@ static void pch_pic_update_irq(LoongArchPCHPIC *s, uint64_t mask, int level)
> if (level) {
> val = mask & s->intirr & ~s->int_mask;
> if (val) {
> - irq = find_first_bit(&val, 64);
> - s->intisr |= 0x1ULL << irq;
> - qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 1);
> + irq = find_first_bit(&val, 64UL);
> + if (irq < 64) {
> + s->intisr |= 0x1ULL << irq;
> + qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 1);
> + }
> }
> } else {
> val = mask & s->intisr;
> if (val) {
> - irq = find_first_bit(&val, 64);
> - s->intisr &= ~(0x1ULL << irq);
> - qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 0);
> + irq = find_first_bit(&val, 64UL);
> + if (irq < 64) {
> + s->intisr &= ~(0x1ULL << irq);
> + qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 0);
> + }
This needs to be rewritten. I mentioned this multiple times during review, but this
instance seems to have slipped through anyway.
You absolutely cannot use find_first_bit etc on raw 'unsigned long' -- bitmap.h functions
may only be used on DECLARE_BITMAP objects.
That said, there is no point in using any of the bitmap.h functions here. You should
simply use uint64_t val, and ctz64() instead of find_first_bit().
r~