include/exec/memory.h | 2 ++ 1 file changed, 2 insertions(+)
Coverity scan reports multiple false-positive "defects" for the
following series of actions in virtio.c:
MemoryRegionCache indirect_desc_cache;
address_space_cache_init_empty(&indirect_desc_cache);
address_space_cache_destroy(&indirect_desc_cache);
For some reason it's unable to recognize the dependency between 'mrs.mr'
and 'fv' and insists that '!mrs.mr' check in address_space_cache_destroy
may take a 'false' branch, even though it is explicitly initialized to
NULL in the address_space_cache_init_empty():
*** CID 1522371: Memory - illegal accesses (UNINIT)
/qemu/hw/virtio/virtio.c: 1627 in virtqueue_split_pop()
1621 }
1622
1623 vq->inuse++;
1624
1625 trace_virtqueue_pop(vq, elem, elem->in_num, elem->out_num);
1626 done:
>>> CID 1522371: Memory - illegal accesses (UNINIT)
>>> Using uninitialized value "indirect_desc_cache.fv" when
>>> calling "address_space_cache_destroy".
1627 address_space_cache_destroy(&indirect_desc_cache);
1628
1629 return elem;
1630
1631 err_undo_map:
1632 virtqueue_undo_map_desc(out_num, in_num, iov);
** CID 1522370: Memory - illegal accesses (UNINIT)
Instead of trying to silence these false positive reports in 4
different places, initializing 'fv' as well, as this doesn't result
in any noticeable performance impact.
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
include/exec/memory.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index c99842d2fc..1ce80c4e82 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2770,6 +2770,8 @@ int64_t address_space_cache_init(MemoryRegionCache *cache,
static inline void address_space_cache_init_empty(MemoryRegionCache *cache)
{
cache->mrs.mr = NULL;
+ /* There is no real need to initialize fv, but it makes Coverity happy. */
+ cache->fv = NULL;
}
/**
--
2.41.0
On Mon, 9 Oct 2023 at 06:44, Ilya Maximets <i.maximets@ovn.org> wrote: > > Coverity scan reports multiple false-positive "defects" for the > following series of actions in virtio.c: > > MemoryRegionCache indirect_desc_cache; > address_space_cache_init_empty(&indirect_desc_cache); > address_space_cache_destroy(&indirect_desc_cache); > > For some reason it's unable to recognize the dependency between 'mrs.mr' > and 'fv' and insists that '!mrs.mr' check in address_space_cache_destroy > may take a 'false' branch, even though it is explicitly initialized to > NULL in the address_space_cache_init_empty(): > > *** CID 1522371: Memory - illegal accesses (UNINIT) > /qemu/hw/virtio/virtio.c: 1627 in virtqueue_split_pop() > 1621 } > 1622 > 1623 vq->inuse++; > 1624 > 1625 trace_virtqueue_pop(vq, elem, elem->in_num, elem->out_num); > 1626 done: > >>> CID 1522371: Memory - illegal accesses (UNINIT) > >>> Using uninitialized value "indirect_desc_cache.fv" when > >>> calling "address_space_cache_destroy". > 1627 address_space_cache_destroy(&indirect_desc_cache); > 1628 > 1629 return elem; > 1630 > 1631 err_undo_map: > 1632 virtqueue_undo_map_desc(out_num, in_num, iov); > > ** CID 1522370: Memory - illegal accesses (UNINIT) > > Instead of trying to silence these false positive reports in 4 > different places, initializing 'fv' as well, as this doesn't result > in any noticeable performance impact. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > include/exec/memory.h | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On 09.10.23 12:43, Ilya Maximets wrote: > Coverity scan reports multiple false-positive "defects" for the > following series of actions in virtio.c: > > MemoryRegionCache indirect_desc_cache; > address_space_cache_init_empty(&indirect_desc_cache); > address_space_cache_destroy(&indirect_desc_cache); > > For some reason it's unable to recognize the dependency between 'mrs.mr' > and 'fv' and insists that '!mrs.mr' check in address_space_cache_destroy > may take a 'false' branch, even though it is explicitly initialized to > NULL in the address_space_cache_init_empty(): > > *** CID 1522371: Memory - illegal accesses (UNINIT) > /qemu/hw/virtio/virtio.c: 1627 in virtqueue_split_pop() > 1621 } > 1622 > 1623 vq->inuse++; > 1624 > 1625 trace_virtqueue_pop(vq, elem, elem->in_num, elem->out_num); > 1626 done: > >>> CID 1522371: Memory - illegal accesses (UNINIT) > >>> Using uninitialized value "indirect_desc_cache.fv" when > >>> calling "address_space_cache_destroy". > 1627 address_space_cache_destroy(&indirect_desc_cache); Yeah, it doesn't even care about what that function actually does, just that it is called with a datastructure that is partially uninitialized. > 1628 > 1629 return elem; > 1630 > 1631 err_undo_map: > 1632 virtqueue_undo_map_desc(out_num, in_num, iov); > > ** CID 1522370: Memory - illegal accesses (UNINIT) > > Instead of trying to silence these false positive reports in 4 > different places, initializing 'fv' as well, as this doesn't result > in any noticeable performance impact. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > include/exec/memory.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index c99842d2fc..1ce80c4e82 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -2770,6 +2770,8 @@ int64_t address_space_cache_init(MemoryRegionCache *cache, > static inline void address_space_cache_init_empty(MemoryRegionCache *cache) > { > cache->mrs.mr = NULL; > + /* There is no real need to initialize fv, but it makes Coverity happy. */ > + cache->fv = NULL; > } > > /** Reviewed-by: David Hildenbrand <david@redhat.com> -- Cheers, David / dhildenb
© 2016 - 2024 Red Hat, Inc.