[PATCH v2 0/2] plugins: add a drcov plugin

[PATCH v2 0/2] plugins: add a drcov plugin

[NDNF]

These patches adds the ability to generate files in drcov format.
Primary goal this scripts is to have coverage
logfiles thatwork in Lighthouse.

Changelog:

v2:
  * Added path to executable binary file.
  * base, end, entry have correct values now.
  * Added option: "filename" for output file.
  * Install an actual tracer when the TB gets executed.

Signed-off-by: Ivanov Arkady <arkadiy.ivanov@ispras.ru>

---

Ivanov Arkady (2):
      src/plugins: add helper functions for drcov
      contrib/plugins: add a drcov plugin


 contrib/plugins/Makefile     |    1 
 contrib/plugins/drcov.c      |  148 ++++++++++++++++++++++++++++++++++++++++++
 include/qemu/qemu-plugin.h   |    5 +
 plugins/api.c                |   27 ++++++++
 plugins/qemu-plugins.symbols |    4 +
 5 files changed, 185 insertions(+)
 create mode 100644 contrib/plugins/drcov.c

--
Ivanov Arkady

[PATCH v2 1/2] src/plugins: add helper functions for drcov

[NDNF]

This patch adds helper functions to the drcov plugin.
Which provide information about:
- start_code.
- end_code.
- entry.
- path to the executable binary.

Signed-off-by: Ivanov Arkady <arkadiy.ivanov@ispras.ru>
---
 include/qemu/qemu-plugin.h   |    5 +++++
 plugins/api.c                |   27 +++++++++++++++++++++++++++
 plugins/qemu-plugins.symbols |    4 ++++
 3 files changed, 36 insertions(+)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 5775e82c4e..807d932e02 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -405,4 +405,9 @@ int qemu_plugin_n_max_vcpus(void);
  */
 void qemu_plugin_outs(const char *string);
 
+QEMU_PLUGIN_EXPORT const char *qemu_plugin_path_to_binary(void);
+QEMU_PLUGIN_EXPORT uint64_t qemu_plugin_start_code(void);
+QEMU_PLUGIN_EXPORT uint64_t qemu_plugin_end_code(void);
+QEMU_PLUGIN_EXPORT uint64_t qemu_plugin_entry_code(void);
+
 #endif /* QEMU_PLUGIN_API_H */
diff --git a/plugins/api.c b/plugins/api.c
index bbdc5a4eb4..4e8a582d58 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -340,3 +340,30 @@ void qemu_plugin_outs(const char *string)
 {
     qemu_log_mask(CPU_LOG_PLUGIN, "%s", string);
 }
+
+#ifdef CONFIG_USER_ONLY
+#include "qemu.h"
+const char *qemu_plugin_path_to_binary(void)
+{
+    TaskState *ts = (TaskState *) current_cpu->opaque;
+    return ts->bprm->filename;
+}
+
+uint64_t qemu_plugin_start_code(void)
+{
+    TaskState *ts = (TaskState *) current_cpu->opaque;
+    return ts->info->start_code;
+}
+
+uint64_t qemu_plugin_end_code(void)
+{
+    TaskState *ts = (TaskState *) current_cpu->opaque;
+    return ts->info->end_code;
+}
+
+uint64_t qemu_plugin_entry_code(void)
+{
+    TaskState *ts = (TaskState *) current_cpu->opaque;
+    return ts->info->entry;
+}
+#endif
diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
index 4bdb381f48..021851fb7d 100644
--- a/plugins/qemu-plugins.symbols
+++ b/plugins/qemu-plugins.symbols
@@ -37,4 +37,8 @@
   qemu_plugin_n_vcpus;
   qemu_plugin_n_max_vcpus;
   qemu_plugin_outs;
+  qemu_plugin_path_to_binary;
+  qemu_plugin_start_code;
+  qemu_plugin_end_code;
+  qemu_plugin_entry_code;
 };

Re: [PATCH v2 1/2] src/plugins: add helper functions for drcov

[Alex Bennée]

NDNF <arkaisp2021@gmail.com> writes:

> This patch adds helper functions to the drcov plugin.
> Which provide information about:
> - start_code.
> - end_code.
> - entry.
> - path to the executable binary.
>
> Signed-off-by: Ivanov Arkady <arkadiy.ivanov@ispras.ru>
> ---
>  include/qemu/qemu-plugin.h   |    5 +++++
>  plugins/api.c                |   27 +++++++++++++++++++++++++++
>  plugins/qemu-plugins.symbols |    4 ++++
>  3 files changed, 36 insertions(+)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 5775e82c4e..807d932e02 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -405,4 +405,9 @@ int qemu_plugin_n_max_vcpus(void);
>   */
>  void qemu_plugin_outs(const char *string);
>  
> +QEMU_PLUGIN_EXPORT const char *qemu_plugin_path_to_binary(void);
> +QEMU_PLUGIN_EXPORT uint64_t qemu_plugin_start_code(void);
> +QEMU_PLUGIN_EXPORT uint64_t qemu_plugin_end_code(void);
> +QEMU_PLUGIN_EXPORT uint64_t qemu_plugin_entry_code(void);
> +

Could you please add some documentation to these functions to explain
what each one does. Using the jdoc style:

/**
 * foo() - does bar
 * @baz: the amount of bar
 ...

as this gets automatically translated into API documentation in the
developer docs.

>  #endif /* QEMU_PLUGIN_API_H */
> diff --git a/plugins/api.c b/plugins/api.c
> index bbdc5a4eb4..4e8a582d58 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -340,3 +340,30 @@ void qemu_plugin_outs(const char *string)
>  {
>      qemu_log_mask(CPU_LOG_PLUGIN, "%s", string);
>  }
> +
> +#ifdef CONFIG_USER_ONLY
> +#include "qemu.h"
> +const char *qemu_plugin_path_to_binary(void)
> +{
> +    TaskState *ts = (TaskState *) current_cpu->opaque;
> +    return ts->bprm->filename;
> +}
> +
> +uint64_t qemu_plugin_start_code(void)
> +{
> +    TaskState *ts = (TaskState *) current_cpu->opaque;
> +    return ts->info->start_code;
> +}
> +
> +uint64_t qemu_plugin_end_code(void)
> +{
> +    TaskState *ts = (TaskState *) current_cpu->opaque;
> +    return ts->info->end_code;
> +}
> +
> +uint64_t qemu_plugin_entry_code(void)
> +{
> +    TaskState *ts = (TaskState *) current_cpu->opaque;
> +    return ts->info->entry;
> +}
> +#endif

You need some stub functions here for system emulation mode although you
might be able to return something useful for the binary path?

> diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
> index 4bdb381f48..021851fb7d 100644
> --- a/plugins/qemu-plugins.symbols
> +++ b/plugins/qemu-plugins.symbols
> @@ -37,4 +37,8 @@
>    qemu_plugin_n_vcpus;
>    qemu_plugin_n_max_vcpus;
>    qemu_plugin_outs;
> +  qemu_plugin_path_to_binary;
> +  qemu_plugin_start_code;
> +  qemu_plugin_end_code;
> +  qemu_plugin_entry_code;

Please maintain the sorted list as it makes it easier to find missing
symbols ;-)

-- 
Alex Bennée

[PATCH v2 2/2] contrib/plugins: add a drcov plugin

[NDNF]

This patch adds the ability to generate files in drcov format.
Primary goal this script is to have coverage
logfiles thatwork in Lighthouse.

Signed-off-by: Ivanov Arkady <arkadiy.ivanov@ispras.ru>
---
 contrib/plugins/Makefile |    1 
 contrib/plugins/drcov.c  |  148 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 149 insertions(+)
 create mode 100644 contrib/plugins/drcov.c

diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
index 7801b08b0d..0a681efeec 100644
--- a/contrib/plugins/Makefile
+++ b/contrib/plugins/Makefile
@@ -17,6 +17,7 @@ NAMES += hotblocks
 NAMES += hotpages
 NAMES += howvec
 NAMES += lockstep
+NAMES += drcov
 
 SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
 
diff --git a/contrib/plugins/drcov.c b/contrib/plugins/drcov.c
new file mode 100644
index 0000000000..f94b52ff64
--- /dev/null
+++ b/contrib/plugins/drcov.c
@@ -0,0 +1,148 @@
+/*
+ * Copyright (C) 2021, Ivanov Arkady <arkadiy.ivanov@ispras.ru>
+ *
+ * Drcov - a DynamoRIO-based tool that collects coverage information
+ * from a binary. Primary goal this script is to have coverage log
+ * files that work in Lighthouse.
+ *
+ * License: GNU GPL, version 2 or later.
+ *   See the COPYING file in the top-level directory.
+ */
+
+#include <inttypes.h>
+#include <assert.h>
+#include <stdlib.h>
+#include <inttypes.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <glib.h>
+
+#include <qemu-plugin.h>
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+static char header[] = "DRCOV VERSION: 2\n"
+                "DRCOV FLAVOR: drcov-64\n"
+                "Module Table: version 2, count 1\n"
+                "Columns: id, base, end, entry, path\n";
+
+static FILE *fp;
+static char *file_name;
+static GMutex lock;
+
+typedef struct {
+    uint32_t start;
+    uint16_t size;
+    uint16_t mod_id;
+} bb_entry_t;
+
+static GSList *bbs;
+
+static void printf_header(void)
+{
+    g_autoptr(GString) head = g_string_new(header);
+    const char *path = qemu_plugin_path_to_binary();
+    uint64_t start_code = qemu_plugin_start_code();
+    uint64_t end_code = qemu_plugin_end_code();
+    uint64_t entry = qemu_plugin_entry_code();
+    g_string_append_printf(head, "0, 0x%lx, 0x%lx, 0x%lx, %s\n",
+                           start_code, end_code, entry, path);
+    g_string_append_printf(head, "BB Table: %d bbs\n", g_slist_length(bbs));
+    fwrite(head->str, sizeof(char), head->len, fp);
+}
+
+static void printf_char_array32(uint32_t data)
+{
+    const uint8_t *bytes = (const uint8_t *)(&data);
+    fwrite(bytes, sizeof(char), sizeof(data), fp);
+}
+
+static void printf_char_array16(uint16_t data)
+{
+    const uint8_t *bytes = (const uint8_t *)(&data);
+    fwrite(bytes, sizeof(char), sizeof(data), fp);
+}
+
+
+static void printf_el(gpointer data, gpointer user_data)
+{
+    g_mutex_lock(&lock);
+
+    bb_entry_t *bb = (bb_entry_t *)data;
+    printf_char_array32(bb->start);
+    printf_char_array16(bb->size);
+    printf_char_array16(bb->mod_id);
+
+    g_mutex_unlock(&lock);
+}
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+    /* Print function */
+    printf_header();
+    g_slist_foreach(bbs, printf_el, NULL);
+
+    /* Clear */
+    g_slist_free_full(bbs, &g_free);
+    g_free(file_name);
+    fclose(fp);
+}
+
+static void plugin_init(void)
+{
+    fp = fopen(file_name, "wb");
+}
+
+static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
+{
+    bb_entry_t *bb = g_malloc0(sizeof(bb_entry_t));
+    memcpy(bb, udata, sizeof(bb_entry_t));
+
+    g_mutex_lock(&lock);
+    bbs = g_slist_append(bbs, bb);
+    g_mutex_unlock(&lock);
+}
+
+static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
+{
+    bb_entry_t *bb = g_new0(bb_entry_t, 1);
+    uint64_t pc = qemu_plugin_tb_vaddr(tb);
+
+    size_t n = qemu_plugin_tb_n_insns(tb);
+    for (int i = 0; i < n; i++) {
+        bb->size += qemu_plugin_insn_size(qemu_plugin_tb_get_insn(tb, i));
+    }
+
+    bb->start = pc;
+    bb->mod_id = 0;
+
+    qemu_plugin_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec,
+                                         QEMU_PLUGIN_CB_NO_REGS,
+                                         (void *)bb);
+
+}
+
+QEMU_PLUGIN_EXPORT
+int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
+                        int argc, char **argv)
+{
+
+    if (!argc) {
+        file_name = "file.drcov.trace";
+    } else {
+        if (g_str_has_prefix(argv[0], "filename=")) {
+            size_t size = strlen(argv[0]) - 9;
+            file_name = g_malloc0(size + 1);
+            strncpy(file_name, argv[0] + 9, size);
+            file_name[size] = '\0';
+        }
+    }
+
+    plugin_init();
+
+    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
+    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
+
+    return 0;
+}

Re: [PATCH v2 2/2] contrib/plugins: add a drcov plugin

[Alex Bennée]

NDNF <arkaisp2021@gmail.com> writes:

> This patch adds the ability to generate files in drcov format.
> Primary goal this script is to have coverage
> logfiles thatwork in Lighthouse.
>
> Signed-off-by: Ivanov Arkady <arkadiy.ivanov@ispras.ru>
> ---
>  contrib/plugins/Makefile |    1 
>  contrib/plugins/drcov.c  |  148 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 149 insertions(+)
>  create mode 100644 contrib/plugins/drcov.c
>
> diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
> index 7801b08b0d..0a681efeec 100644
> --- a/contrib/plugins/Makefile
> +++ b/contrib/plugins/Makefile
> @@ -17,6 +17,7 @@ NAMES += hotblocks
>  NAMES += hotpages
>  NAMES += howvec
>  NAMES += lockstep
> +NAMES += drcov
>  
>  SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
>  
> diff --git a/contrib/plugins/drcov.c b/contrib/plugins/drcov.c
> new file mode 100644
> index 0000000000..f94b52ff64
> --- /dev/null
> +++ b/contrib/plugins/drcov.c
> @@ -0,0 +1,148 @@
> +/*
> + * Copyright (C) 2021, Ivanov Arkady <arkadiy.ivanov@ispras.ru>
> + *
> + * Drcov - a DynamoRIO-based tool that collects coverage information
> + * from a binary. Primary goal this script is to have coverage log
> + * files that work in Lighthouse.
> + *
> + * License: GNU GPL, version 2 or later.
> + *   See the COPYING file in the top-level directory.
> + */
> +
> +#include <inttypes.h>
> +#include <assert.h>
> +#include <stdlib.h>
> +#include <inttypes.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <glib.h>
> +
> +#include <qemu-plugin.h>
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +
> +static char header[] = "DRCOV VERSION: 2\n"
> +                "DRCOV FLAVOR: drcov-64\n"
> +                "Module Table: version 2, count 1\n"
> +                "Columns: id, base, end, entry, path\n";
> +
> +static FILE *fp;
> +static char *file_name;
> +static GMutex lock;
> +
> +typedef struct {
> +    uint32_t start;
> +    uint16_t size;
> +    uint16_t mod_id;
> +} bb_entry_t;
> +
> +static GSList *bbs;
> +
> +static void printf_header(void)
> +{
> +    g_autoptr(GString) head = g_string_new(header);
> +    const char *path = qemu_plugin_path_to_binary();
> +    uint64_t start_code = qemu_plugin_start_code();
> +    uint64_t end_code = qemu_plugin_end_code();
> +    uint64_t entry = qemu_plugin_entry_code();
> +    g_string_append_printf(head, "0, 0x%lx, 0x%lx, 0x%lx, %s\n",
> +                           start_code, end_code, entry, path);
> +    g_string_append_printf(head, "BB Table: %d bbs\n", g_slist_length(bbs));
> +    fwrite(head->str, sizeof(char), head->len, fp);
> +}
> +
> +static void printf_char_array32(uint32_t data)
> +{
> +    const uint8_t *bytes = (const uint8_t *)(&data);
> +    fwrite(bytes, sizeof(char), sizeof(data), fp);
> +}
> +
> +static void printf_char_array16(uint16_t data)
> +{
> +    const uint8_t *bytes = (const uint8_t *)(&data);
> +    fwrite(bytes, sizeof(char), sizeof(data), fp);
> +}
> +
> +
> +static void printf_el(gpointer data, gpointer user_data)
> +{
> +    g_mutex_lock(&lock);
> +
> +    bb_entry_t *bb = (bb_entry_t *)data;
> +    printf_char_array32(bb->start);
> +    printf_char_array16(bb->size);
> +    printf_char_array16(bb->mod_id);
> +
> +    g_mutex_unlock(&lock);
> +}
> +
> +static void plugin_exit(qemu_plugin_id_t id, void *p)
> +{
> +    /* Print function */
> +    printf_header();
> +    g_slist_foreach(bbs, printf_el, NULL);
> +
> +    /* Clear */
> +    g_slist_free_full(bbs, &g_free);
> +    g_free(file_name);
> +    fclose(fp);
> +}
> +
> +static void plugin_init(void)
> +{
> +    fp = fopen(file_name, "wb");
> +}
> +
> +static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
> +{
> +    bb_entry_t *bb = g_malloc0(sizeof(bb_entry_t));
> +    memcpy(bb, udata, sizeof(bb_entry_t));
> +
> +    g_mutex_lock(&lock);
> +    bbs = g_slist_append(bbs, bb);
> +    g_mutex_unlock(&lock);

Hmm given the data you are copying is static and never going to change
you could just save the pointer to it in a log. Instead of maintaining a
bbs g_slist you could use g_ptr_array_add(bbs, udata) (with appropriate
locking) and then just iterate over the log at the end. As the same
entry will have multiple references you probably need another ptr array
just to track the creation bellow which you can use to clean-up on exit.
That should still be a net saving on two mallocs and s_list.

> +}
> +
> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> +{
> +    bb_entry_t *bb = g_new0(bb_entry_t, 1);
> +    uint64_t pc = qemu_plugin_tb_vaddr(tb);
> +
> +    size_t n = qemu_plugin_tb_n_insns(tb);
> +    for (int i = 0; i < n; i++) {
> +        bb->size += qemu_plugin_insn_size(qemu_plugin_tb_get_insn(tb, i));
> +    }
> +
> +    bb->start = pc;
> +    bb->mod_id = 0;
> +
> +    qemu_plugin_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec,
> +                                         QEMU_PLUGIN_CB_NO_REGS,
> +                                         (void *)bb);
> +
> +}
> +
> +QEMU_PLUGIN_EXPORT
> +int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
> +                        int argc, char **argv)
> +{
> +
> +    if (!argc) {
> +        file_name = "file.drcov.trace";
> +    } else {
> +        if (g_str_has_prefix(argv[0], "filename=")) {
> +            size_t size = strlen(argv[0]) - 9;
> +            file_name = g_malloc0(size + 1);
> +            strncpy(file_name, argv[0] + 9, size);
> +            file_name[size] = '\0';

I guess it's probably overkill for a single arg but if you were looping
of argv something like:

        g_autofree char **tokens = g_strsplit(argv[i], "=", 2);
        if (g_strcmp0(tokens[0], "filename") == 0) {
            file_name = g_strdup(tokens[1]);
        }

would be a more glib-like way to deal with it.

> +        }
> +    }
> +
> +    plugin_init();
> +
> +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
> +
> +    return 0;
> +}


-- 
Alex Bennée