[Qemu-devel] [PATCH 0/6] vhost-user-test and leak fixes

[Qemu-devel] [PATCH 0/6] vhost-user-test and leak fixes

[Marc-André Lureau]

Hi,

The following patches fix a regression introduced in commit
218bb57dd79d that prevent ASAN from being detected & used.  There is
also a works around for a GCC ASAN optimization bug. A few test leaks
are fixed, and a few patches reenable vhost-user memfd test fixing the
recent race bug that was identified in the previous version.

Marc-André Lureau (6):
  build-sys: fix -fsanitize=address check
  lockable: workaround GCC link issue with ASAN
  vhost-user-test: add back memfd check
  vhost-user-test: do not hang if chardev creation failed
  ahci-test: fix opts leak of skip tests
  sdhci-test: fix leaks

 include/qemu/lockable.h |  2 +-
 tests/ahci-test.c       |  1 +
 tests/sdhci-test.c      |  2 ++
 tests/vhost-user-test.c | 94 +++++++++++++++++++++++++++++++++++--------------
 configure               | 22 ++++++------
 5 files changed, 83 insertions(+), 38 deletions(-)

-- 
2.16.1.73.g5832b7e9f2

[Qemu-devel] [PATCH 1/6] build-sys: fix -fsanitize=address check

[Marc-André Lureau]

Since 218bb57dd79d6843e0592c30a82ea8c1fddc74a5, the -fsanitize=address
check fails with:
config-temp/qemu-conf.c:3:20: error: integer overflow in expression [-Werror=overflow]
   return INT32_MIN / -1;

Interestingly, UBSAN check doesn't produce a compile time warning.
Use a test that doesn't have compile time warnings, and make it
specific to UBSAN check.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 configure | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/configure b/configure
index 913e14839d..cc610823e1 100755
--- a/configure
+++ b/configure
@@ -5306,25 +5306,27 @@ fi
 ##########################################
 # checks for sanitizers
 
-# we could use a simple skeleton for flags checks, but this also
-# detect the static linking issue of ubsan, see also:
-# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84285
-cat > $TMPC << EOF
-#include <stdint.h>
-int main(void) {
-  return INT32_MIN / -1;
-}
-EOF
-
 have_asan=no
 have_ubsan=no
 have_asan_iface_h=no
 have_asan_iface_fiber=no
 
 if test "$sanitizers" = "yes" ; then
+  write_c_skeleton
   if compile_prog "$CPU_CFLAGS -Werror -fsanitize=address" ""; then
       have_asan=yes
   fi
+
+  # we could use a simple skeleton for flags checks, but this also
+  # detect the static linking issue of ubsan, see also:
+  # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84285
+  cat > $TMPC << EOF
+#include <stdlib.h>
+int main(void) {
+    void *tmp = malloc(10);
+    return *(int *)(tmp + 2);
+}
+EOF
   if compile_prog "$CPU_CFLAGS -Werror -fsanitize=undefined" ""; then
       have_ubsan=yes
   fi
-- 
2.16.1.73.g5832b7e9f2

Re: [Qemu-devel] [PATCH 1/6] build-sys: fix -fsanitize=address check

[Emilio G. Cota]

On Thu, Feb 15, 2018 at 22:25:47 +0100, Marc-André Lureau wrote:
> Since 218bb57dd79d6843e0592c30a82ea8c1fddc74a5, the -fsanitize=address
> check fails with:
> config-temp/qemu-conf.c:3:20: error: integer overflow in expression [-Werror=overflow]
>    return INT32_MIN / -1;
> 
> Interestingly, UBSAN check doesn't produce a compile time warning.
> Use a test that doesn't have compile time warnings, and make it
> specific to UBSAN check.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Emilio G. Cota <cota@braap.org>

		E.

[Qemu-devel] [PATCH 2/6] lockable: workaround GCC link issue with ASAN

[Marc-André Lureau]

Current GCC has an optimization bug when compiling with ASAN.

See also GCC bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84307

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/lockable.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
index b6ed6c89ec..84ea794bcf 100644
--- a/include/qemu/lockable.h
+++ b/include/qemu/lockable.h
@@ -28,7 +28,7 @@ struct QemuLockable {
  * to QEMU_MAKE_LOCKABLE.  For optimized builds, we can rely on dead-code elimination
  * from the compiler, and give the errors already at link time.
  */
-#ifdef __OPTIMIZE__
+#if defined(__OPTIMIZE__) && !defined(__SANITIZE_ADDRESS__)
 void unknown_lock_type(void *);
 #else
 static inline void unknown_lock_type(void *unused)
-- 
2.16.1.73.g5832b7e9f2

[Qemu-devel] [PATCH 3/6] vhost-user-test: add back memfd check

[Marc-André Lureau]

This revert commit fb68096da3d35e64c88cd610c1fa42766c58e92a, and
modify test_read_guest_mem() to use different chardev names, when
using memfd (_test_server_free(), where the chardev is removed, runs
in idle).

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/vhost-user-test.c | 93 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 66 insertions(+), 27 deletions(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index a217353e2c..67e5f7f858 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -18,6 +18,7 @@
 #include "qemu/range.h"
 #include "qemu/sockets.h"
 #include "chardev/char-fe.h"
+#include "qemu/memfd.h"
 #include "sysemu/sysemu.h"
 #include "libqos/libqos.h"
 #include "libqos/pci-pc.h"
@@ -40,23 +41,14 @@
 #define HAVE_MONOTONIC_TIME
 #endif
 
-#define QEMU_CMD_MEM    " -m %d -object memory-backend-file,id=mem,size=%dM,"\
+#define QEMU_CMD_MEM    " -m %d -object memory-backend-file,id=mem,size=%dM," \
                         "mem-path=%s,share=on -numa node,memdev=mem"
+#define QEMU_CMD_MEMFD  " -m %d -object memory-backend-memfd,id=mem,size=%dM," \
+                        " -numa node,memdev=mem"
 #define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s%s"
 #define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=%s,vhostforce"
 #define QEMU_CMD_NET    " -device virtio-net-pci,netdev=net0"
 
-#define QEMU_CMD        QEMU_CMD_MEM QEMU_CMD_CHR \
-                        QEMU_CMD_NETDEV QEMU_CMD_NET
-
-#define GET_QEMU_CMD(s)                                         \
-    g_strdup_printf(QEMU_CMD, 512, 512, (root), (s)->chr_name,  \
-                    (s)->socket_path, "", (s)->chr_name)
-
-#define GET_QEMU_CMDE(s, mem, chr_opts, extra, ...)                     \
-    g_strdup_printf(QEMU_CMD extra, (mem), (mem), (root), (s)->chr_name, \
-                    (s)->socket_path, (chr_opts), (s)->chr_name, ##__VA_ARGS__)
-
 #define HUGETLBFS_MAGIC       0x958458f6
 
 /*********** FROM hw/virtio/vhost-user.c *************************************/
@@ -175,6 +167,33 @@ static void test_server_listen(TestServer *server);
 static const char *tmpfs;
 static const char *root;
 
+enum test_memfd {
+    TEST_MEMFD_AUTO,
+    TEST_MEMFD_YES,
+    TEST_MEMFD_NO,
+};
+
+static char *get_qemu_cmd(TestServer *s,
+                          int mem, enum test_memfd memfd, const char *mem_path,
+                          const char *chr_opts, const char *extra)
+{
+    if (memfd == TEST_MEMFD_AUTO && qemu_memfd_check()) {
+        memfd = TEST_MEMFD_YES;
+    }
+
+    if (memfd == TEST_MEMFD_YES) {
+        return g_strdup_printf(QEMU_CMD_MEMFD QEMU_CMD_CHR
+                               QEMU_CMD_NETDEV QEMU_CMD_NET "%s", mem, mem,
+                               s->chr_name, s->socket_path,
+                               chr_opts, s->chr_name, extra);
+    } else {
+        return g_strdup_printf(QEMU_CMD_MEM QEMU_CMD_CHR
+                               QEMU_CMD_NETDEV QEMU_CMD_NET "%s", mem, mem,
+                               mem_path, s->chr_name, s->socket_path,
+                               chr_opts, s->chr_name, extra);
+    }
+}
+
 static void init_virtio_dev(TestServer *s, uint32_t features_mask)
 {
     uint32_t features;
@@ -640,16 +659,18 @@ GSourceFuncs test_migrate_source_funcs = {
     .check = test_migrate_source_check,
 };
 
-static void test_read_guest_mem(void)
+static void test_read_guest_mem(const void *arg)
 {
+    enum test_memfd memfd = GPOINTER_TO_INT(arg);
     TestServer *server = NULL;
     char *qemu_cmd = NULL;
     QTestState *s = NULL;
 
-    server = test_server_new("test");
+    server = test_server_new(memfd == TEST_MEMFD_YES ?
+                             "read-guest-memfd" : "read-guest-mem");
     test_server_listen(server);
 
-    qemu_cmd = GET_QEMU_CMD(server);
+    qemu_cmd = get_qemu_cmd(server, 512, memfd, root, "", "");
 
     s = qtest_start(qemu_cmd);
     g_free(qemu_cmd);
@@ -671,7 +692,7 @@ static void test_migrate(void)
     char *uri = g_strdup_printf("%s%s", "unix:", dest->mig_path);
     QTestState *global = global_qtest, *from, *to;
     GSource *source;
-    gchar *cmd;
+    gchar *cmd, *tmp;
     QDict *rsp;
     guint8 *log;
     guint64 size;
@@ -679,7 +700,7 @@ static void test_migrate(void)
     test_server_listen(s);
     test_server_listen(dest);
 
-    cmd = GET_QEMU_CMDE(s, 2, "", "");
+    cmd = get_qemu_cmd(s, 2, TEST_MEMFD_AUTO, root, "", "");
     from = qtest_start(cmd);
     g_free(cmd);
 
@@ -688,7 +709,9 @@ static void test_migrate(void)
     size = get_log_size(s);
     g_assert_cmpint(size, ==, (2 * 1024 * 1024) / (VHOST_LOG_PAGE * 8));
 
-    cmd = GET_QEMU_CMDE(dest, 2, "", " -incoming %s", uri);
+    tmp = g_strdup_printf(" -incoming %s", uri);
+    cmd = get_qemu_cmd(dest, 2, TEST_MEMFD_AUTO, root, "", tmp);
+    g_free(tmp);
     to = qtest_init(cmd);
     g_free(cmd);
 
@@ -801,7 +824,7 @@ static void test_reconnect_subprocess(void)
     char *cmd;
 
     g_thread_new("connect", connect_thread, s);
-    cmd = GET_QEMU_CMDE(s, 2, ",server", "");
+    cmd = get_qemu_cmd(s, 2, TEST_MEMFD_AUTO, root, ",server", "");
     qtest_start(cmd);
     g_free(cmd);
 
@@ -839,7 +862,7 @@ static void test_connect_fail_subprocess(void)
 
     s->test_fail = true;
     g_thread_new("connect", connect_thread, s);
-    cmd = GET_QEMU_CMDE(s, 2, ",server", "");
+    cmd = get_qemu_cmd(s, 2, TEST_MEMFD_AUTO, root, ",server", "");
     qtest_start(cmd);
     g_free(cmd);
 
@@ -869,7 +892,7 @@ static void test_flags_mismatch_subprocess(void)
 
     s->test_flags = TEST_FLAGS_DISCONNECT;
     g_thread_new("connect", connect_thread, s);
-    cmd = GET_QEMU_CMDE(s, 2, ",server", "");
+    cmd = get_qemu_cmd(s, 2, TEST_MEMFD_AUTO, root, ",server", "");
     qtest_start(cmd);
     g_free(cmd);
 
@@ -904,11 +927,21 @@ static void test_multiqueue(void)
     s->queues = 2;
     test_server_listen(s);
 
-    cmd = g_strdup_printf(QEMU_CMD_MEM QEMU_CMD_CHR QEMU_CMD_NETDEV ",queues=%d "
-                          "-device virtio-net-pci,netdev=net0,mq=on,vectors=%d",
-                          512, 512, root, s->chr_name,
-                          s->socket_path, "", s->chr_name,
-                          s->queues, s->queues * 2 + 2);
+    if (qemu_memfd_check()) {
+        cmd = g_strdup_printf(
+            QEMU_CMD_MEMFD QEMU_CMD_CHR QEMU_CMD_NETDEV ",queues=%d "
+            "-device virtio-net-pci,netdev=net0,mq=on,vectors=%d",
+            512, 512, s->chr_name,
+            s->socket_path, "", s->chr_name,
+            s->queues, s->queues * 2 + 2);
+    } else {
+        cmd = g_strdup_printf(
+            QEMU_CMD_MEM QEMU_CMD_CHR QEMU_CMD_NETDEV ",queues=%d "
+            "-device virtio-net-pci,netdev=net0,mq=on,vectors=%d",
+            512, 512, root, s->chr_name,
+            s->socket_path, "", s->chr_name,
+            s->queues, s->queues * 2 + 2);
+    }
     qtest_start(cmd);
     g_free(cmd);
 
@@ -954,7 +987,13 @@ int main(int argc, char **argv)
     /* run the main loop thread so the chardev may operate */
     thread = g_thread_new(NULL, thread_function, loop);
 
-    qtest_add_func("/vhost-user/read-guest-mem", test_read_guest_mem);
+    if (qemu_memfd_check()) {
+        qtest_add_data_func("/vhost-user/read-guest-mem/memfd",
+                            GINT_TO_POINTER(TEST_MEMFD_YES),
+                            test_read_guest_mem);
+    }
+    qtest_add_data_func("/vhost-user/read-guest-mem/memfile",
+                        GINT_TO_POINTER(TEST_MEMFD_NO), test_read_guest_mem);
     qtest_add_func("/vhost-user/migrate", test_migrate);
     qtest_add_func("/vhost-user/multiqueue", test_multiqueue);
 
-- 
2.16.1.73.g5832b7e9f2

Re: [Qemu-devel] [PATCH 3/6] vhost-user-test: add back memfd check

[Maxime Coquelin]

On 02/15/2018 10:25 PM, Marc-André Lureau wrote:
> This revert commit fb68096da3d35e64c88cd610c1fa42766c58e92a, and
> modify test_read_guest_mem() to use different chardev names, when
> using memfd (_test_server_free(), where the chardev is removed, runs
> in idle).
> 
> Signed-off-by: Marc-André Lureau<marcandre.lureau@redhat.com>
> ---
>   tests/vhost-user-test.c | 93 +++++++++++++++++++++++++++++++++++--------------
>   1 file changed, 66 insertions(+), 27 deletions(-)

Looks good to me:
Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

Re: [Qemu-devel] [PATCH 3/6] vhost-user-test: add back memfd check

[Marc-André Lureau]

Hi Paolo,

Could you pick the remaining fixes acked by Maxime?

thanks

On Fri, Feb 23, 2018 at 3:23 PM, Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
>
> On 02/15/2018 10:25 PM, Marc-André Lureau wrote:
>>
>> This revert commit fb68096da3d35e64c88cd610c1fa42766c58e92a, and
>> modify test_read_guest_mem() to use different chardev names, when
>> using memfd (_test_server_free(), where the chardev is removed, runs
>> in idle).
>>
>> Signed-off-by: Marc-André Lureau<marcandre.lureau@redhat.com>
>> ---
>>   tests/vhost-user-test.c | 93
>> +++++++++++++++++++++++++++++++++++--------------
>>   1 file changed, 66 insertions(+), 27 deletions(-)
>
>
> Looks good to me:
> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 3/6] vhost-user-test: add back memfd check

[Paolo Bonzini]

On 16/03/2018 15:17, Marc-André Lureau wrote:
> Hi Paolo,
> 
> Could you pick the remaining fixes acked by Maxime?

Ok, queued them now.

Paolo

> thanks
> 
> On Fri, Feb 23, 2018 at 3:23 PM, Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>>
>> On 02/15/2018 10:25 PM, Marc-André Lureau wrote:
>>>
>>> This revert commit fb68096da3d35e64c88cd610c1fa42766c58e92a, and
>>> modify test_read_guest_mem() to use different chardev names, when
>>> using memfd (_test_server_free(), where the chardev is removed, runs
>>> in idle).
>>>
>>> Signed-off-by: Marc-André Lureau<marcandre.lureau@redhat.com>
>>> ---
>>>   tests/vhost-user-test.c | 93
>>> +++++++++++++++++++++++++++++++++++--------------
>>>   1 file changed, 66 insertions(+), 27 deletions(-)
>>
>>
>> Looks good to me:
>> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>
> 

[Qemu-devel] [PATCH 4/6] vhost-user-test: do not hang if chardev creation failed

[Marc-André Lureau]

Before the chardev name fix, the following error may happen: "attempt
to add duplicate property 'chr-test' to object (type 'container')",
due to races.

Sadly, error_vprintf() uses g_test_message(), so you have to use
read the cryptic --debug-log to see it. Later, it would make sense to
use g_critical() instead, and catch errors with
g_test_expect_message() (in glib 2.34).

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/vhost-user-test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 67e5f7f858..efd28411d3 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -513,6 +513,7 @@ static void test_server_create_chr(TestServer *server, const gchar *opt)
     chr = qemu_chr_new(server->chr_name, chr_path);
     g_free(chr_path);
 
+    g_assert_nonnull(chr);
     qemu_chr_fe_init(&server->chr, chr, &error_abort);
     qemu_chr_fe_set_handlers(&server->chr, chr_can_read, chr_read,
                              chr_event, NULL, server, NULL, true);
-- 
2.16.1.73.g5832b7e9f2

Re: [Qemu-devel] [PATCH 4/6] vhost-user-test: do not hang if chardev creation failed

[Maxime Coquelin]

On 02/15/2018 10:25 PM, Marc-André Lureau wrote:
> Before the chardev name fix, the following error may happen: "attempt
> to add duplicate property 'chr-test' to object (type 'container')",
> due to races.
> 
> Sadly, error_vprintf() uses g_test_message(), so you have to use
> read the cryptic --debug-log to see it. Later, it would make sense to
> use g_critical() instead, and catch errors with
> g_test_expect_message() (in glib 2.34).
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   tests/vhost-user-test.c | 1 +
>   1 file changed, 1 insertion(+)
Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

[Qemu-devel] [PATCH 5/6] ahci-test: fix opts leak of skip tests

[Marc-André Lureau]

Fixes the following ASAN report:

Direct leak of 128 byte(s) in 8 object(s) allocated from:
    #0 0x7fefce311850 in malloc (/lib64/libasan.so.4+0xde850)
    #1 0x7fefcdd5ef0c in g_malloc ../glib/gmem.c:94
    #2 0x559b976faff0 in create_ahci_io_test /home/elmarco/src/qemu/tests/ahci-test.c:1810

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/ahci-test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 7aa5af428c..1bd3cc7ca8 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1822,6 +1822,7 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
     if ((addr == ADDR_MODE_LBA48) && (offset == OFFSET_HIGH) &&
         (mb_to_sectors(test_image_size_mb) <= 0xFFFFFFF)) {
         g_test_message("%s: skipped; test image too small", name);
+        g_free(opts);
         g_free(name);
         return;
     }
-- 
2.16.1.73.g5832b7e9f2

Re: [Qemu-devel] [PATCH 5/6] ahci-test: fix opts leak of skip tests

[John Snow]

On 02/15/2018 04:25 PM, Marc-André Lureau wrote:
> Fixes the following ASAN report:
> 
> Direct leak of 128 byte(s) in 8 object(s) allocated from:
>     #0 0x7fefce311850 in malloc (/lib64/libasan.so.4+0xde850)
>     #1 0x7fefcdd5ef0c in g_malloc ../glib/gmem.c:94
>     #2 0x559b976faff0 in create_ahci_io_test /home/elmarco/src/qemu/tests/ahci-test.c:1810
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/ahci-test.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index 7aa5af428c..1bd3cc7ca8 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -1822,6 +1822,7 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
>      if ((addr == ADDR_MODE_LBA48) && (offset == OFFSET_HIGH) &&
>          (mb_to_sectors(test_image_size_mb) <= 0xFFFFFFF)) {
>          g_test_message("%s: skipped; test image too small", name);
> +        g_free(opts);
>          g_free(name);
>          return;
>      }
> 

Whupps.

Thanks.

Reviewed-by: John Snow <jsnow@redhat.com>

And, feel free to stage this in whomever's branch, it won't conflict
with anything:

Acked-by: John Snow <jsnow@redhat.com>

[Qemu-devel] [PATCH 6/6] sdhci-test: fix leaks

[Marc-André Lureau]

Fix the following ASAN reports:

==20125==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f0faea03a38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38)
    #1 0x7f0fae450f75 in g_malloc0 ../glib/gmem.c:124
    #2 0x562fffd526fc in machine_start /home/elmarco/src/qemu/tests/sdhci-test.c:180

Indirect leak of 152 byte(s) in 1 object(s) allocated from:
    #0 0x7f0faea03850 in malloc (/lib64/libasan.so.4+0xde850)
    #1 0x7f0fae450f0c in g_malloc ../glib/gmem.c:94
    #2 0x562fffd5d21d in qpci_init_pc /home/elmarco/src/qemu/tests/libqos/pci-pc.c:122

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/sdhci-test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 493023fd0c..8a7099398c 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -209,8 +209,10 @@ static QSDHCI *machine_start(const struct sdhci_t *test)
 
 static void machine_stop(QSDHCI *s)
 {
+    qpci_free_pc(s->pci.bus);
     g_free(s->pci.dev);
     qtest_quit(global_qtest);
+    g_free(s);
 }
 
 static void test_machine(const void *data)
-- 
2.16.1.73.g5832b7e9f2

Re: [Qemu-devel] [PATCH 6/6] sdhci-test: fix leaks

[Philippe Mathieu-Daudé]

On 02/15/2018 06:25 PM, Marc-André Lureau wrote:
> Fix the following ASAN reports:
> 
> ==20125==ERROR: LeakSanitizer: detected memory leaks
> 
> Direct leak of 24 byte(s) in 1 object(s) allocated from:
>     #0 0x7f0faea03a38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38)
>     #1 0x7f0fae450f75 in g_malloc0 ../glib/gmem.c:124
>     #2 0x562fffd526fc in machine_start /home/elmarco/src/qemu/tests/sdhci-test.c:180
> 
> Indirect leak of 152 byte(s) in 1 object(s) allocated from:
>     #0 0x7f0faea03850 in malloc (/lib64/libasan.so.4+0xde850)
>     #1 0x7f0fae450f0c in g_malloc ../glib/gmem.c:94
>     #2 0x562fffd5d21d in qpci_init_pc /home/elmarco/src/qemu/tests/libqos/pci-pc.c:122

Oops my bad, thanks!

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  tests/sdhci-test.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
> index 493023fd0c..8a7099398c 100644
> --- a/tests/sdhci-test.c
> +++ b/tests/sdhci-test.c
> @@ -209,8 +209,10 @@ static QSDHCI *machine_start(const struct sdhci_t *test)
>  
>  static void machine_stop(QSDHCI *s)
>  {
> +    qpci_free_pc(s->pci.bus);
>      g_free(s->pci.dev);
>      qtest_quit(global_qtest);
> +    g_free(s);
>  }
>  
>  static void test_machine(const void *data)
> 

Re: [Qemu-devel] [PATCH 0/6] vhost-user-test and leak fixes

[Paolo Bonzini]

On 15/02/2018 22:25, Marc-André Lureau wrote:
> Hi,
> 
> The following patches fix a regression introduced in commit
> 218bb57dd79d that prevent ASAN from being detected & used.  There is
> also a works around for a GCC ASAN optimization bug. A few test leaks
> are fixed, and a few patches reenable vhost-user memfd test fixing the
> recent race bug that was identified in the previous version.
> 
> Marc-André Lureau (6):
>   build-sys: fix -fsanitize=address check
>   lockable: workaround GCC link issue with ASAN
>   vhost-user-test: add back memfd check
>   vhost-user-test: do not hang if chardev creation failed
>   ahci-test: fix opts leak of skip tests
>   sdhci-test: fix leaks
> 
>  include/qemu/lockable.h |  2 +-
>  tests/ahci-test.c       |  1 +
>  tests/sdhci-test.c      |  2 ++
>  tests/vhost-user-test.c | 94 +++++++++++++++++++++++++++++++++++--------------
>  configure               | 22 ++++++------
>  5 files changed, 83 insertions(+), 38 deletions(-)
> 

Queued 1-2-5-6, not really confident enough in what's going on with
vhost-user-test. :)

Paolo

Re: [Qemu-devel] [PATCH 0/6] vhost-user-test and leak fixes

[Marc-André Lureau]

Hi

On Fri, Feb 23, 2018 at 12:37 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 15/02/2018 22:25, Marc-André Lureau wrote:
>> Hi,
>>
>> The following patches fix a regression introduced in commit
>> 218bb57dd79d that prevent ASAN from being detected & used.  There is
>> also a works around for a GCC ASAN optimization bug. A few test leaks
>> are fixed, and a few patches reenable vhost-user memfd test fixing the
>> recent race bug that was identified in the previous version.
>>
>> Marc-André Lureau (6):
>>   build-sys: fix -fsanitize=address check
>>   lockable: workaround GCC link issue with ASAN
>>   vhost-user-test: add back memfd check
>>   vhost-user-test: do not hang if chardev creation failed
>>   ahci-test: fix opts leak of skip tests
>>   sdhci-test: fix leaks
>>
>>  include/qemu/lockable.h |  2 +-
>>  tests/ahci-test.c       |  1 +
>>  tests/sdhci-test.c      |  2 ++
>>  tests/vhost-user-test.c | 94 +++++++++++++++++++++++++++++++++++--------------
>>  configure               | 22 ++++++------
>>  5 files changed, 83 insertions(+), 38 deletions(-)
>>
>
> Queued 1-2-5-6, not really confident enough in what's going on with
> vhost-user-test. :)

It hangs when chardev creation fails. Which is apparent when running
with --debug-log. That's also why I added "vhost-user-test: do not
hang if chardev creation failed". The problem comes from reusing
socket location without waiting for the idle cleanup. Using different
socket path solves it (vhost-user-test: add back memfd check)

Maxime, could you help review those patches?

thanks

-- 
Marc-André Lureau