All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shannon Zhao <zhaoshenglong@huawei.com>
To: Peter Maydell <peter.maydell@linaro.org>, <qemu-arm@nongnu.org>,
	<qemu-devel@nongnu.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>, patches@linaro.org
Subject: Re: [Qemu-arm] [Qemu-devel] [PATCH] arm_gicv3: Add assert()s to tell Coverity that offsets are aligned
Date: Tue, 12 Jul 2016 09:33:09 +0800	[thread overview]
Message-ID: <578448D5.4020208@huawei.com> (raw)
In-Reply-To: <1468261372-17508-1-git-send-email-peter.maydell@linaro.org>



On 2016/7/12 2:22, Peter Maydell wrote:
> Coverity complains that the GICR_IPRIORITYR case in gicv3_readl()
> can overflow an array, because it doesn't know that the offsets
> passed to that function must be word aligned. Add some assert()s
> which hopefully tell Coverity that this isn't possible.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I don't have any way to test this except getting it into master
> and seeing if Coverity still complains, but if it does then
> I'll happily just mark the error as a false positive...
> 
Since the codes are correct, maybe it could ignore the original complain
at Coverity instead of adding the assert(). But anyway I'm fine with
this patch.

>  hw/intc/arm_gicv3_redist.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
> index 2f60096..77e5cfa 100644
> --- a/hw/intc/arm_gicv3_redist.c
> +++ b/hw/intc/arm_gicv3_redist.c
> @@ -420,6 +420,8 @@ MemTxResult gicv3_redist_read(void *opaque, hwaddr offset, uint64_t *data,
>      MemTxResult r;
>      int cpuidx;
>  
> +    assert((offset & (size - 1)) == 0);
> +
>      /* This region covers all the redistributor pages; there are
>       * (for GICv3) two 64K pages per CPU. At the moment they are
>       * all contiguous (ie in this one region), though we might later
> @@ -468,6 +470,8 @@ MemTxResult gicv3_redist_write(void *opaque, hwaddr offset, uint64_t data,
>      MemTxResult r;
>      int cpuidx;
>  
> +    assert((offset & (size - 1)) == 0);
> +
>      /* This region covers all the redistributor pages; there are
>       * (for GICv3) two 64K pages per CPU. At the moment they are
>       * all contiguous (ie in this one region), though we might later
> 

-- 
Shannon


WARNING: multiple messages have this Message-ID (diff)
From: Shannon Zhao <zhaoshenglong@huawei.com>
To: Peter Maydell <peter.maydell@linaro.org>,
	qemu-arm@nongnu.org, qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>, patches@linaro.org
Subject: Re: [Qemu-devel] [PATCH] arm_gicv3: Add assert()s to tell Coverity that offsets are aligned
Date: Tue, 12 Jul 2016 09:33:09 +0800	[thread overview]
Message-ID: <578448D5.4020208@huawei.com> (raw)
In-Reply-To: <1468261372-17508-1-git-send-email-peter.maydell@linaro.org>



On 2016/7/12 2:22, Peter Maydell wrote:
> Coverity complains that the GICR_IPRIORITYR case in gicv3_readl()
> can overflow an array, because it doesn't know that the offsets
> passed to that function must be word aligned. Add some assert()s
> which hopefully tell Coverity that this isn't possible.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I don't have any way to test this except getting it into master
> and seeing if Coverity still complains, but if it does then
> I'll happily just mark the error as a false positive...
> 
Since the codes are correct, maybe it could ignore the original complain
at Coverity instead of adding the assert(). But anyway I'm fine with
this patch.

>  hw/intc/arm_gicv3_redist.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
> index 2f60096..77e5cfa 100644
> --- a/hw/intc/arm_gicv3_redist.c
> +++ b/hw/intc/arm_gicv3_redist.c
> @@ -420,6 +420,8 @@ MemTxResult gicv3_redist_read(void *opaque, hwaddr offset, uint64_t *data,
>      MemTxResult r;
>      int cpuidx;
>  
> +    assert((offset & (size - 1)) == 0);
> +
>      /* This region covers all the redistributor pages; there are
>       * (for GICv3) two 64K pages per CPU. At the moment they are
>       * all contiguous (ie in this one region), though we might later
> @@ -468,6 +470,8 @@ MemTxResult gicv3_redist_write(void *opaque, hwaddr offset, uint64_t data,
>      MemTxResult r;
>      int cpuidx;
>  
> +    assert((offset & (size - 1)) == 0);
> +
>      /* This region covers all the redistributor pages; there are
>       * (for GICv3) two 64K pages per CPU. At the moment they are
>       * all contiguous (ie in this one region), though we might later
> 

-- 
Shannon

  reply	other threads:[~2016-07-12  1:33 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-11 18:22 [Qemu-arm] [PATCH] arm_gicv3: Add assert()s to tell Coverity that offsets are aligned Peter Maydell
2016-07-11 18:22 ` [Qemu-devel] " Peter Maydell
2016-07-12  1:33 ` Shannon Zhao [this message]
2016-07-12  1:33   ` Shannon Zhao
2016-07-12  7:55   ` [Qemu-arm] " Peter Maydell
2016-07-12  7:55     ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=578448D5.4020208@huawei.com \
    --to=zhaoshenglong@huawei.com \
    --cc=patches@linaro.org \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.