[Qemu-devel] [PULL 0/4] coverity: Improve and extend model

[Qemu-devel] [PULL 0/4] coverity: Improve and extend model

[Markus Armbruster]

The following changes since commit ec6f25e788ef57ce1e9f734984ef8885172fd9e2:

  Merge remote-tracking branch 'remotes/rth/tags/pull-tg-s390-20150203' into staging (2015-02-03 21:37:16 +0000)

are available in the git repository at:


  git://repo.or.cz/qemu/armbru.git tags/pull-cov-model-2015-02-05

for you to fetch changes up to 8c413e7902ef0c19ced516f575db989ccc3785f8:

  MAINTAINERS: Add myself as Coverity model maintainer (2015-02-05 17:16:14 +0100)

----------------------------------------------------------------
coverity: Improve and extend model

----------------------------------------------------------------
Markus Armbruster (4):
      coverity: Improve model for GLib memory allocation
      coverity: Model GLib string allocation partially
      coverity: Model g_free() isn't necessarily free()
      MAINTAINERS: Add myself as Coverity model maintainer

 MAINTAINERS              |   5 ++
 scripts/coverity-model.c | 228 +++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 198 insertions(+), 35 deletions(-)

-- 
1.9.3

[Qemu-devel] [PULL 1/4] coverity: Improve model for GLib memory allocation

[Markus Armbruster]

In current versions of GLib, g_new() may expand into g_malloc_n().
When it does, Coverity can't see the memory allocation, because we
don't model g_malloc_n().  Similarly for g_new0(), g_renew(),
g_try_new(), g_try_new0(), g_try_renew().

Model g_malloc_n(), g_malloc0_n(), g_realloc_n().  Model
g_try_malloc_n(), g_try_malloc0_n(), g_try_realloc_n() by adding
indeterminate out of memory conditions on top.

To avoid undue duplication, replace the existing models for g_malloc()
& friends by trivial wrappers around g_malloc_n() & friends.

In a local scan, this flags four additional RESOURCE_LEAKs and one
NULL_RETURNS.

The NULL_RETURNS is a false positive: Coverity can now see that
g_try_malloc(l1_sz * sizeof(uint64_t)) in
qcow2_check_metadata_overlap() may return NULL, but is too stupid to
recognize that a loop executing l1_sz times won't be entered then.

Three out of the four RESOURCE_LEAKs appear genuine.  The false
positive is in ppce500_prep_device_tree(): the pointer dies, but a
pointer to a struct member escapes, and we get the pointer back for
freeing with container_of().  Too funky for Coverity.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-model.c | 139 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 104 insertions(+), 35 deletions(-)

diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
index 4c99a85..8d0839e 100644
--- a/scripts/coverity-model.c
+++ b/scripts/coverity-model.c
@@ -90,7 +90,8 @@ static int get_keysym(const name2keysym_t *table,
     }
 }
 
-/* glib memory allocation functions.
+/*
+ * GLib memory allocation functions.
  *
  * Note that we ignore the fact that g_malloc of 0 bytes returns NULL,
  * and g_realloc of 0 bytes frees the pointer.
@@ -107,60 +108,128 @@ static int get_keysym(const name2keysym_t *table,
  * we'll get a buffer overflow reported anyway.
  */
 
-void *malloc(size_t);
-void *calloc(size_t, size_t);
-void *realloc(void *, size_t);
-void free(void *);
+/*
+ * Allocation primitives, cannot return NULL
+ * See also Coverity's library/generic/libc/all/all.c
+ */
 
-void *
-g_malloc(size_t n_bytes)
+void *g_malloc_n(size_t nmemb, size_t size)
 {
-    void *mem;
-    __coverity_negative_sink__(n_bytes);
-    mem = malloc(n_bytes == 0 ? 1 : n_bytes);
-    if (!mem) __coverity_panic__();
-    return mem;
+    size_t sz;
+    void *ptr;
+
+    __coverity_negative_sink__(nmemb);
+    __coverity_negative_sink__(size);
+    sz = nmemb * size;
+    ptr = __coverity_alloc__(size);
+    __coverity_mark_as_uninitialized_buffer__(ptr);
+    __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+    return ptr;
+}
+
+void *g_malloc0_n(size_t nmemb, size_t size)
+{
+    size_t sz;
+    void *ptr;
+
+    __coverity_negative_sink__(nmemb);
+    __coverity_negative_sink__(size);
+    sz = nmemb * size;
+    ptr = __coverity_alloc__(size);
+    __coverity_writeall0__(ptr);
+    __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+    return ptr;
+}
+
+void *g_realloc_n(void *ptr, size_t nmemb, size_t size)
+{
+    size_t sz;
+
+    __coverity_negative_sink__(nmemb);
+    __coverity_negative_sink__(size);
+    sz = nmemb * size;
+    __coverity_escape__(ptr);
+    ptr = __coverity_alloc__(size);
+    /*
+     * Memory beyond the old size isn't actually initialized.  Can't
+     * model that.  See Coverity's realloc() model
+     */
+    __coverity_writeall__(ptr);
+    __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+    return ptr;
+}
+
+void g_free(void *ptr)
+{
+    __coverity_free__(ptr);
+    __coverity_mark_as_afm_freed__(ptr, AFM_free);
+}
+
+/*
+ * Derive the g_try_FOO_n() from the g_FOO_n() by adding indeterminate
+ * out of memory conditions
+ */
+
+void *g_try_malloc_n(size_t nmemb, size_t size)
+{
+    int nomem;
+
+    if (nomem) {
+        return NULL;
+    }
+    return g_malloc_n(nmemb, size);
 }
 
-void *
-g_malloc0(size_t n_bytes)
+void *g_try_malloc0_n(size_t nmemb, size_t size)
+{
+    int nomem;
+
+    if (nomem) {
+        return NULL;
+    }
+    return g_malloc0_n(nmemb, size);
+}
+
+void *g_try_realloc_n(void *ptr, size_t nmemb, size_t size)
+{
+    int nomem;
+
+    if (nomem) {
+        return NULL;
+    }
+    return g_realloc_n(ptr, nmemb, size);
+}
+
+/* Trivially derive the g_FOO() from the g_FOO_n() */
+
+void *g_malloc(size_t size)
 {
-    void *mem;
-    __coverity_negative_sink__(n_bytes);
-    mem = calloc(1, n_bytes == 0 ? 1 : n_bytes);
-    if (!mem) __coverity_panic__();
-    return mem;
+    return g_malloc_n(1, size);
 }
 
-void g_free(void *mem)
+void *g_malloc0(size_t size)
 {
-    free(mem);
+    return g_malloc0_n(1, size);
 }
 
-void *g_realloc(void * mem, size_t n_bytes)
+void *g_realloc(void *ptr, size_t size)
 {
-    __coverity_negative_sink__(n_bytes);
-    mem = realloc(mem, n_bytes == 0 ? 1 : n_bytes);
-    if (!mem) __coverity_panic__();
-    return mem;
+    return g_realloc_n(ptr, 1, size);
 }
 
-void *g_try_malloc(size_t n_bytes)
+void *g_try_malloc(size_t size)
 {
-    __coverity_negative_sink__(n_bytes);
-    return malloc(n_bytes == 0 ? 1 : n_bytes);
+    return g_try_malloc_n(1, size);
 }
 
-void *g_try_malloc0(size_t n_bytes)
+void *g_try_malloc0(size_t size)
 {
-    __coverity_negative_sink__(n_bytes);
-    return calloc(1, n_bytes == 0 ? 1 : n_bytes);
+    return g_try_malloc0_n(1, size);
 }
 
-void *g_try_realloc(void *mem, size_t n_bytes)
+void *g_try_realloc(void *ptr, size_t size)
 {
-    __coverity_negative_sink__(n_bytes);
-    return realloc(mem, n_bytes == 0 ? 1 : n_bytes);
+    return g_try_realloc_n(ptr, 1, size);
 }
 
 /* Other glib functions */
-- 
1.9.3

[Qemu-devel] [PULL 2/4] coverity: Model GLib string allocation partially

[Markus Armbruster]

Without a model, Coverity can't know that the result of g_strdup()
needs to be fed to g_free().

One way to get such a model is to scan GLib, build a derived model
file with cov-collect-models, and use that when scanning QEMU.
Unfortunately, the Coverity Scan service we use doesn't support that.

Thus, we're stuck with the other way: write a user model.  Doing that
for all of GLib is hardly practical.  I'm doing it for the "String
Utility Functions" we actually use that return dynamically allocated
strings.

In a local scan, this flags 20 additional RESOURCE_LEAKs.  The ones I
checked look genuine.

It also loses a NULL_RETURNS about ppce500_init() using
qemu_find_file() without error checking.  I don't understand why.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-model.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
index 8d0839e..230bc30 100644
--- a/scripts/coverity-model.c
+++ b/scripts/coverity-model.c
@@ -40,6 +40,8 @@ typedef unsigned long long uint64_t;
 typedef long long int64_t;
 typedef _Bool bool;
 
+typedef struct va_list_str *va_list;
+
 /* exec.c */
 
 typedef struct AddressSpace AddressSpace;
@@ -232,6 +234,93 @@ void *g_try_realloc(void *ptr, size_t size)
     return g_try_realloc_n(ptr, 1, size);
 }
 
+/*
+ * GLib string allocation functions
+ */
+
+char *g_strdup(const char *s)
+{
+    char *dup;
+    size_t i;
+
+    if (!s) {
+        return NULL;
+    }
+
+    __coverity_string_null_sink__(s);
+    __coverity_string_size_sink__(s);
+    dup = __coverity_alloc_nosize__();
+    __coverity_mark_as_afm_allocated__(dup, AFM_free);
+    for (i = 0; (dup[i] = s[i]); i++) ;
+    return dup;
+}
+
+char *g_strndup(const char *s, size_t n)
+{
+    char *dup;
+    size_t i;
+
+    __coverity_negative_sink__(n);
+
+    if (!s) {
+        return NULL;
+    }
+
+    dup = g_malloc(n + 1);
+    for (i = 0; i < n && (dup[i] = s[i]); i++) ;
+    dup[i] = 0;
+    return dup;
+}
+
+char *g_strdup_printf(const char *format, ...)
+{
+    char ch, *s;
+    size_t len;
+
+    __coverity_string_null_sink__(format);
+    __coverity_string_size_sink__(format);
+
+    ch = *format;
+
+    s = __coverity_alloc_nosize__();
+    __coverity_writeall__(s);
+    __coverity_mark_as_afm_allocated__(s, AFM_free);
+    return s;
+}
+
+char *g_strdup_vprintf(const char *format, va_list ap)
+{
+    char ch, *s;
+    size_t len;
+
+    __coverity_string_null_sink__(format);
+    __coverity_string_size_sink__(format);
+
+    ch = *format;
+    ch = *(char *)ap;
+
+    s = __coverity_alloc_nosize__();
+    __coverity_writeall__(s);
+    __coverity_mark_as_afm_allocated__(s, AFM_free);
+
+    return len;
+}
+
+char *g_strconcat(const char *s, ...)
+{
+    char *s;
+
+    /*
+     * Can't model: last argument must be null, the others
+     * null-terminated strings
+     */
+
+    s = __coverity_alloc_nosize__();
+    __coverity_writeall__(s);
+    __coverity_mark_as_afm_allocated__(s, AFM_free);
+    return s;
+}
+
 /* Other glib functions */
 
 typedef struct _GIOChannel GIOChannel;
-- 
1.9.3

Re: [Qemu-devel] [PULL 2/4] coverity: Model GLib string allocation partially

[Paolo Bonzini]

On 05/02/2015 17:24, Markus Armbruster wrote:
> +
> +char *g_strdup(const char *s)
> +{
> +    char *dup;
> +    size_t i;
> +
> +    if (!s) {
> +        return NULL;
> +    }
> +
> +    __coverity_string_null_sink__(s);
> +    __coverity_string_size_sink__(s);

What's __coverity_string_size_sink__?  It is likely responsible for this
in libcacard:

Unbounded source buffer (STRING_SIZE)
string_size: Passing string argv[argc - 2] of unknown size to g_strdup,
which expects a string of a particular size

I guess it's okay to mark this as intentional?

> 
> +char *g_strndup(const char *s, size_t n)
> +{
> +    char *dup;
> +    size_t i;
> +
> +    __coverity_negative_sink__(n);
> +
> +    if (!s) {
> +        return NULL;
> +    }
> +
> +    dup = g_malloc(n + 1);


This should be g_malloc0 I think.

Paolo

Re: [Qemu-devel] [PULL 2/4] coverity: Model GLib string allocation partially

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 05/02/2015 17:24, Markus Armbruster wrote:
>> +
>> +char *g_strdup(const char *s)
>> +{
>> +    char *dup;
>> +    size_t i;
>> +
>> +    if (!s) {
>> +        return NULL;
>> +    }
>> +
>> +    __coverity_string_null_sink__(s);
>> +    __coverity_string_size_sink__(s);
>
> What's __coverity_string_size_sink__?  It is likely responsible for this
> in libcacard:
>
> Unbounded source buffer (STRING_SIZE)
> string_size: Passing string argv[argc - 2] of unknown size to g_strdup,
> which expects a string of a particular size

Yup, it is.  The reference manual explains it "indicates to the
STRING_SIZE checker that a function is a string size sink and must be
protected from arbitrarily large strings."

Of course, treating argv[] strings as unbounded in size is debatable.
The OS generally imposes some limit.  Linux's limit is rather generous:
32 pages, if I read execve(2) correctly.

That aside, why on earth are we copying these strings?  The only use of
the copies is passing them to getaddrinfo() via connect_to_qemu().

> I guess it's okay to mark this as intentional?

I guess it's okay to delete the copying.

>> +char *g_strndup(const char *s, size_t n)
>> +{
>> +    char *dup;
>> +    size_t i;
>> +
>> +    __coverity_negative_sink__(n);
>> +
>> +    if (!s) {
>> +        return NULL;
>> +    }
>> +
>> +    dup = g_malloc(n + 1);
        for (i = 0; i < n && (dup[i] = s[i]); i++) ;
        dup[i] = 0;
        return dup;
    }
>
>
> This should be g_malloc0 I think.

g_malloc0() is more faithful to what the function does, but I decided to
use g_malloc() anyway, because I recommend not to rely on the padding
behavior[*].

I think g_strndup() is fine when you want to duplicate a substring.  No
padding then.  I figure this is the common use case.

When you want to duplicate a string, but limit the size of the
duplicate, then it doesn't do the right thing when the string is shorter
than the limit.  Use g_strdup_printf() instead.

What it does is right for something else: allocating a buffer with a
specific size and completely initializing it from a string.  I can't
remember having had a use for that myself.


[*] Looks like the designers of g_strndup() couldn't resist the
temptation to "improve" upon strndup().  Fine, but call it something
else then.

[Qemu-devel] [PULL 3/4] coverity: Model g_free() isn't necessarily free()

[Markus Armbruster]

Memory allocated with GLib needs to be freed with GLib.  Freeing it
with free() instead of g_free() is a common error.  Harmless when
g_free() is a trivial wrapper around free(), which is commonly the
case.  But model the difference anyway.

In a local scan, this flags four ALLOC_FREE_MISMATCH.  Requires
--enable ALLOC_FREE_MISMATCH, because the checker is still preview.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/coverity-model.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
index 230bc30..58356af 100644
--- a/scripts/coverity-model.c
+++ b/scripts/coverity-model.c
@@ -125,7 +125,7 @@ void *g_malloc_n(size_t nmemb, size_t size)
     sz = nmemb * size;
     ptr = __coverity_alloc__(size);
     __coverity_mark_as_uninitialized_buffer__(ptr);
-    __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+    __coverity_mark_as_afm_allocated__(ptr, "g_free");
     return ptr;
 }
 
@@ -139,7 +139,7 @@ void *g_malloc0_n(size_t nmemb, size_t size)
     sz = nmemb * size;
     ptr = __coverity_alloc__(size);
     __coverity_writeall0__(ptr);
-    __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+    __coverity_mark_as_afm_allocated__(ptr, "g_free");
     return ptr;
 }
 
@@ -157,14 +157,14 @@ void *g_realloc_n(void *ptr, size_t nmemb, size_t size)
      * model that.  See Coverity's realloc() model
      */
     __coverity_writeall__(ptr);
-    __coverity_mark_as_afm_allocated__(ptr, AFM_free);
+    __coverity_mark_as_afm_allocated__(ptr, "g_free");
     return ptr;
 }
 
 void g_free(void *ptr)
 {
     __coverity_free__(ptr);
-    __coverity_mark_as_afm_freed__(ptr, AFM_free);
+    __coverity_mark_as_afm_freed__(ptr, "g_free");
 }
 
 /*
@@ -250,7 +250,7 @@ char *g_strdup(const char *s)
     __coverity_string_null_sink__(s);
     __coverity_string_size_sink__(s);
     dup = __coverity_alloc_nosize__();
-    __coverity_mark_as_afm_allocated__(dup, AFM_free);
+    __coverity_mark_as_afm_allocated__(dup, "g_free");
     for (i = 0; (dup[i] = s[i]); i++) ;
     return dup;
 }
@@ -284,7 +284,7 @@ char *g_strdup_printf(const char *format, ...)
 
     s = __coverity_alloc_nosize__();
     __coverity_writeall__(s);
-    __coverity_mark_as_afm_allocated__(s, AFM_free);
+    __coverity_mark_as_afm_allocated__(s, "g_free");
     return s;
 }
 
@@ -301,7 +301,7 @@ char *g_strdup_vprintf(const char *format, va_list ap)
 
     s = __coverity_alloc_nosize__();
     __coverity_writeall__(s);
-    __coverity_mark_as_afm_allocated__(s, AFM_free);
+    __coverity_mark_as_afm_allocated__(s, "g_free");
 
     return len;
 }
@@ -317,7 +317,7 @@ char *g_strconcat(const char *s, ...)
 
     s = __coverity_alloc_nosize__();
     __coverity_writeall__(s);
-    __coverity_mark_as_afm_allocated__(s, AFM_free);
+    __coverity_mark_as_afm_allocated__(s, "g_free");
     return s;
 }
 
-- 
1.9.3

[Qemu-devel] [PULL 4/4] MAINTAINERS: Add myself as Coverity model maintainer

[Markus Armbruster]

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 MAINTAINERS | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fd335a4..b68cb7e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -783,6 +783,11 @@ M: Samuel Thibault <samuel.thibault@ens-lyon.org>
 S: Maintained
 F: backends/baum.c
 
+Coverity model
+M: Markus Armbruster <armbru@redhat.com>
+S: Supported
+F: scripts/coverity-model.c
+
 CPU
 M: Andreas Färber <afaerber@suse.de>
 S: Supported
-- 
1.9.3

Re: [Qemu-devel] [PULL 0/4] coverity: Improve and extend model

[Peter Maydell]

On 5 February 2015 at 16:24, Markus Armbruster <armbru@redhat.com> wrote:
> The following changes since commit ec6f25e788ef57ce1e9f734984ef8885172fd9e2:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-tg-s390-20150203' into staging (2015-02-03 21:37:16 +0000)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-cov-model-2015-02-05
>
> for you to fetch changes up to 8c413e7902ef0c19ced516f575db989ccc3785f8:
>
>   MAINTAINERS: Add myself as Coverity model maintainer (2015-02-05 17:16:14 +0100)
>
> ----------------------------------------------------------------
> coverity: Improve and extend model
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL 0/4] coverity: Improve and extend model

[Paolo Bonzini]

On 05/02/2015 18:11, Peter Maydell wrote:
> On 5 February 2015 at 16:24, Markus Armbruster <armbru@redhat.com> wrote:
>> The following changes since commit ec6f25e788ef57ce1e9f734984ef8885172fd9e2:
>>
>>   Merge remote-tracking branch 'remotes/rth/tags/pull-tg-s390-20150203' into staging (2015-02-03 21:37:16 +0000)
>>
>> are available in the git repository at:
>>
>>
>>   git://repo.or.cz/qemu/armbru.git tags/pull-cov-model-2015-02-05
>>
>> for you to fetch changes up to 8c413e7902ef0c19ced516f575db989ccc3785f8:
>>
>>   MAINTAINERS: Add myself as Coverity model maintainer (2015-02-05 17:16:14 +0100)
>>
>> ----------------------------------------------------------------
>> coverity: Improve and extend model
>>
>> ----------------------------------------------------------------
> 
> Applied, thanks.
> 
> -- PMM
> 
> 

It seems like Coverity Scan doesn't like the new model.  Possibly the
fault of the third patch.  Will check (for now I'm still running scans
with the old model).

Paolo

Re: [Qemu-devel] [PULL 0/4] coverity: Improve and extend model

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 05/02/2015 18:11, Peter Maydell wrote:
>> On 5 February 2015 at 16:24, Markus Armbruster <armbru@redhat.com> wrote:
>>> The following changes since commit ec6f25e788ef57ce1e9f734984ef8885172fd9e2:
>>>
>>>   Merge remote-tracking branch
>>> remotes/rth/tags/pull-tg-s390-20150203' into staging (2015-02-03
>>> 21:37:16 +0000)
>>>
>>> are available in the git repository at:
>>>
>>>
>>>   git://repo.or.cz/qemu/armbru.git tags/pull-cov-model-2015-02-05
>>>
>>> for you to fetch changes up to 8c413e7902ef0c19ced516f575db989ccc3785f8:
>>>
>>>   MAINTAINERS: Add myself as Coverity model maintainer (2015-02-05
>>> 17:16:14 +0100)
>>>
>>> ----------------------------------------------------------------
>>> coverity: Improve and extend model
>>>
>>> ----------------------------------------------------------------
>> 
>> Applied, thanks.
>> 
>> -- PMM
>> 
>> 
>
> It seems like Coverity Scan doesn't like the new model.  Possibly the
> fault of the third patch.

Works for me with a local 7.0.3 installation.  Which I just realized is
outdated.  I'll recheck with 7.6.0.

>                            Will check (for now I'm still running scans
> with the old model).

Appreciated.

Re: [Qemu-devel] [PULL 0/4] coverity: Improve and extend model

[Paolo Bonzini]

\>> It seems like Coverity Scan doesn't like the new model.  Possibly the
>> fault of the third patch.
> 
> Works for me with a local 7.0.3 installation.  Which I just realized is
> outdated.  I'll recheck with 7.6.0.
> 
>>                            Will check (for now I'm still running scans
>> with the old model).
> 
> Appreciated.

Yes, the last patch seems to be the culprit.  It failed immediately with
the __coverity_mark_as_afm_freed__(ptr, "g_free") model; it's been
running for a while without it even though it's not done yet.

Perhaps we can do something like this:

/* Some helpful comment here.  */
#if 1
#define AFM_g_free              AFM_free
#else
#define AFM_g_free              "g_free"
#endif

Paolo

Re: [Qemu-devel] [PULL 0/4] coverity: Improve and extend model

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> \>> It seems like Coverity Scan doesn't like the new model.  Possibly the
>>> fault of the third patch.
>> 
>> Works for me with a local 7.0.3 installation.  Which I just realized is
>> outdated.  I'll recheck with 7.6.0.
>> 
>>>                            Will check (for now I'm still running scans
>>> with the old model).
>> 
>> Appreciated.
>
> Yes, the last patch seems to be the culprit.  It failed immediately with
> the __coverity_mark_as_afm_freed__(ptr, "g_free") model;

Any diagnostics?

Works for me locally with 7.6.0, and matches the reference manual's
examples for the ALLOC_FREE_MISMATCH checker.

>                                                          it's been
> running for a while without it even though it's not done yet.
>
> Perhaps we can do something like this:
>
> /* Some helpful comment here.  */
> #if 1
> #define AFM_g_free              AFM_free
> #else
> #define AFM_g_free              "g_free"
> #endif

Yes.  If you want me to write the helpful comment, I need to know what
exactly goes wrong.

The alternative is to revert the patch for now.

Aside: would be nice if Coverity pre-defined a symbol when
ALLOC_FREE_MISMATCH is enabled, it would let me enable support for the
checker exactly when it's wanted.

Re: [Qemu-devel] [PULL 0/4] coverity: Improve and extend model

[Paolo Bonzini]

On 12/02/2015 10:11, Markus Armbruster wrote:
> > Yes, the last patch seems to be the culprit.  It failed immediately with
> > the __coverity_mark_as_afm_freed__(ptr, "g_free") model;
> 
> Any diagnostics?

None.  It just says roughly "the build had a temporary problem and will
be rescheduled".

> The alternative is to revert the patch for now.

No problem, the model file is anyway just documentation since you have
to upload it manually.

I'll ask the admins.

Paolo