In insn_check_regs() we don't explicitly check whether
qemu_plugin_read_register() failed, which confuses Coverity into
thinking that sz can be -1 in the memcmp().  In fact the assertion
that sz == reg->last->len means this can't happen, but it's clearer
to both humans and Coverity if we explicitly assert that sz > 0, as
we already do in init_vcpu_register().

Coverity: CID 1611901, 1611902
Fixes: af6e4e0a22c1 ("contrib/plugins: extend execlog to track register changes")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 contrib/plugins/execlog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index d67d0107613..8b07dd773e5 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -95,6 +95,7 @@ static void insn_check_regs(CPU *cpu)
 
         g_byte_array_set_size(reg->new, 0);
         sz = qemu_plugin_read_register(reg->handle, reg->new);
+        g_assert(sz > 0);
         g_assert(sz == reg->last->len);
 
         if (memcmp(reg->last->data, reg->new->data, sz)) {
-- 
2.43.0

Peter Maydell <peter.maydell@linaro.org> writes:

> In insn_check_regs() we don't explicitly check whether
> qemu_plugin_read_register() failed, which confuses Coverity into
> thinking that sz can be -1 in the memcmp().  In fact the assertion
> that sz == reg->last->len means this can't happen, but it's clearer
> to both humans and Coverity if we explicitly assert that sz > 0, as
> we already do in init_vcpu_register().
>
> Coverity: CID 1611901, 1611902
> Fixes: af6e4e0a22c1 ("contrib/plugins: extend execlog to track register changes")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Queued to plugins/next, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

On 7/10/25 7:45 AM, Peter Maydell wrote:
> In insn_check_regs() we don't explicitly check whether
> qemu_plugin_read_register() failed, which confuses Coverity into
> thinking that sz can be -1 in the memcmp().  In fact the assertion
> that sz == reg->last->len means this can't happen, but it's clearer
> to both humans and Coverity if we explicitly assert that sz > 0, as
> we already do in init_vcpu_register().
> 
> Coverity: CID 1611901, 1611902
> Fixes: af6e4e0a22c1 ("contrib/plugins: extend execlog to track register changes")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   contrib/plugins/execlog.c | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Hi Alex -- this patch was reviewed back in July but didn't
make it into git before 10.1 freeze; would you like to
pick it up now we've reopened for 10.2 ?

thanks
-- PMM

On Thu, 10 Jul 2025 at 16:21, Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
>
> On 7/10/25 7:45 AM, Peter Maydell wrote:
> > In insn_check_regs() we don't explicitly check whether
> > qemu_plugin_read_register() failed, which confuses Coverity into
> > thinking that sz can be -1 in the memcmp().  In fact the assertion
> > that sz == reg->last->len means this can't happen, but it's clearer
> > to both humans and Coverity if we explicitly assert that sz > 0, as
> > we already do in init_vcpu_register().
> >
> > Coverity: CID 1611901, 1611902
> > Fixes: af6e4e0a22c1 ("contrib/plugins: extend execlog to track register changes")
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   contrib/plugins/execlog.c | 1 +
> >   1 file changed, 1 insertion(+)
>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>