scripts/coverity-scan/model.c | 235 ++++++++++++++++------------------ 1 file changed, 110 insertions(+), 125 deletions(-)
Recently, an update to the Coverity tools caused it to introduce hundreds of new complaints about using g_free() to free memory areas allocated by GLib functions. The solution adopted here (patch 2) is to just make g_free a synonym of free, removing the custom g_free marker from __coverity_mark_as_afm_allocated__ and __coverity_mark_as_afm_freed__. This unfortunately goes against the GLib documentation, which suggests that g_malloc() should be matched with g_free() and plain malloc() with free(); since GLib 2.46 however g_malloc() is hardcoded to always use the system malloc implementation, and g_free is just "free" plus a tracepoint. Therefore, this should not cause any problem in practice. There are still problems, in that Coverity believes that the result of g_malloc/g_malloc0 can return NULL, which is not true. What caused the issue is anybody's guess; possibly a new version of Coverity changed the semantics of __coverity_alloc__, but I also had to inline the model of g_malloc_n in g_malloc (and likewise for the other five functions) though it seems like Coverity. This is implemented in patches 5-6. On top of these changes, this includes a few more changes to the model file: - patch 1 include a few more simplified memory read/write models, so that Coverity has a model for all functions in the pci_dma_* and dma_memory_* family. This fixes a few incorrect out of bounds false positive, where Coverity does not realize that only up to LEN bytes are read/written by those functions - patch 3 removes the model for various allocation functions, which is unnecessary now that we need not (or cannot) detect their being paired with g_free - patch 4 is a small cleanup that makes the inlined allocation functions smaller. This series is a sort of FYI; since the only way to debug the model file is to upload it to scan.coverity.com, these changes are all already live. The last will be as of the next build, but was effective last Thursday and worked (I tried disabling it on Friday in something like a bisection, but it failed and I have now reverted to Thursday's model). Thanks, Paolo Paolo Bonzini (6): coverity-model: update address_space_read/write models coverity-model: make g_free a synonym of free coverity-model: remove model for more allocation functions coverity-model: clean up the models for array allocation functions coverity-model: constrain g_malloc/g_malloc0/g_realloc as never returning NULL coverity-model: write models fully for non-array allocation functions scripts/coverity-scan/model.c | 235 ++++++++++++++++------------------ 1 file changed, 110 insertions(+), 125 deletions(-) -- 2.31.1
On Sat, 31 Jul 2021 at 07:29, Paolo Bonzini <pbonzini@redhat.com> wrote: > > Recently, an update to the Coverity tools caused it to introduce hundreds > of new complaints about using g_free() to free memory areas allocated > by GLib functions. The solution adopted here (patch 2) is to just > make g_free a synonym of free, removing the custom g_free marker from > __coverity_mark_as_afm_allocated__ and __coverity_mark_as_afm_freed__. > This unfortunately goes against the GLib documentation, which suggests > that g_malloc() should be matched with g_free() and plain malloc() with > free(); since GLib 2.46 however g_malloc() is hardcoded to always use the > system malloc implementation, and g_free is just "free" plus a tracepoint. > Therefore, this should not cause any problem in practice. > > There are still problems, in that Coverity believes that the result of > g_malloc/g_malloc0 can return NULL, which is not true. What caused the > issue is anybody's guess; possibly a new version of Coverity changed > the semantics of __coverity_alloc__, but I also had to inline the model > of g_malloc_n in g_malloc (and likewise for the other five functions) > though it seems like Coverity. This is implemented in patches 5-6. > > On top of these changes, this includes a few more changes to the model > file: > > - patch 1 include a few more simplified memory read/write models, so > that Coverity has a model for all functions in the pci_dma_* and > dma_memory_* family. This fixes a few incorrect out of bounds > false positive, where Coverity does not realize that only up to > LEN bytes are read/written by those functions > > - patch 3 removes the model for various allocation functions, which > is unnecessary now that we need not (or cannot) detect their > being paired with g_free > > - patch 4 is a small cleanup that makes the inlined allocation > functions smaller. > > This series is a sort of FYI; since the only way to debug the model file > is to upload it to scan.coverity.com, these changes are all already live. > The last will be as of the next build, but was effective last Thursday > and worked (I tried disabling it on Friday in something like a bisection, > but it failed and I have now reverted to Thursday's model). Thanks for digging through all this mess. I take it that the Coverity results are now stable and people can now start looking through them and triaging again ? -- PMM
On 02/08/21 14:46, Peter Maydell wrote: >> This series is a sort of FYI; since the only way to debug the model file >> is to upload it to scan.coverity.com, these changes are all already live. >> The last will be as of the next build, but was effective last Thursday >> and worked (I tried disabling it on Friday in something like a bisection, >> but it failed and I have now reverted to Thursday's model). > Thanks for digging through all this mess. I take it that the > Coverity results are now stable and people can now start looking > through them and triaging again ? Yes, these patches are the final result of the "investigation". Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: [...] > This series is a sort of FYI; since the only way to debug the model file > is to upload it to scan.coverity.com, these changes are all already live. When I mess with Coverity, I test with my locally installed version first. Version skew and lack of the web interface can make that less than useful sometimes. This is not criticism of your work flow. > The last will be as of the next build, but was effective last Thursday > and worked (I tried disabling it on Friday in something like a bisection, > but it failed and I have now reverted to Thursday's model).
© 2016 - 2024 Red Hat, Inc.