Re: [PATCH 3/3] plugins: avoid failing plugin when CPU is inited several times

["Alex Bennée" <alex.bennee@linaro.org>]

Nikolay Igotti <igotti@gmail.com> writes:

> --- counter.c
>
> #include <assert.h>
> #include <pthread.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
>
> #include <glib.h>
>
> #include <qemu-plugin.h>
>
> QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
>
> // Files with descriptors after this one are intercepted for instruction
> counting marks.
> #define CATCH_BASE 0xcafebabe
>
> static uint64_t insn_count = 0;
> static pthread_t counting = false;
> static pthread_t counting_for = 0;
> static bool on_every_close = false;
>
> static void vcpu_insn_exec_before(unsigned int cpu_index, void *udata)
> {
>     if (counting && pthread_self() == counting_for)
>         insn_count++;
> }
>
> static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> {
>     size_t n = qemu_plugin_tb_n_insns(tb);
>     size_t i;
>
>     for (i = 0; i < n; i++) {
>         struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
>
>         // TODO: do this call only on first insn in bb.
>         qemu_plugin_register_vcpu_insn_exec_cb(
>             insn, vcpu_insn_exec_before, QEMU_PLUGIN_CB_NO_REGS, NULL);
>     }
> }
>
> static void print_insn_count(void) {
>     g_autofree gchar *out = g_strdup_printf("executed %" PRIu64 "
> instructions\n", insn_count);
>     qemu_plugin_outs(out);
> }
>
> static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
>                         int64_t num, uint64_t a1, uint64_t a2,
>                         uint64_t a3, uint64_t a4, uint64_t a5,
>                         uint64_t a6, uint64_t a7, uint64_t a8)
> {
>     // We put our listener on fd reads in range [CATCH_BASE, CATCH_BASE + 1]
>     if (num == 0) { // sys_read
>         switch (a1)
>         {
>             case CATCH_BASE + 0:
>                 counting = true;
>                 counting_for = pthread_self();
>                 insn_count = 0;
>                 break;
>             case CATCH_BASE + 1: {
>                 counting = false;
>                 counting_for = 0;
>                 if (a3 == 8) {
>                     // In case of user emulation in QEMU, addresses are 1:1
> translated, so we can tell the caller
>                     // number of executed instructions by just writing into
> the buffer argument of read.
>                     *(uint64_t*)a2 = insn_count;

Hmm this was certainly unintentional - is it the host or guest address
you are messing with here? I wouldn't count on it pointing where you
think and relying on it to pass information back to the instrumented
guest.

Anyway I have a replication - we are trying to insert the same id into
the plugin cpu index hash table twice. 

>                 }
>                 print_insn_count();
>                 break;
>             }
>             default:
>                 break;
>         }
>     }
>     if (num == 3 && on_every_close) { // sys_close
>         print_insn_count();
>     }
> }
>
> QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
>                                            const qemu_info_t *info,
>                                            int argc, char **argv)
> {
>     int i;
>     for (i = 0; i < argc; i++) {
>         if (!strcmp(argv[i], "on_every_close")) {
>             on_every_close = true;
>             counting = true;
>             counting_for = pthread_self();
>         }
>     }
>
>     qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
>     qemu_plugin_register_vcpu_syscall_cb(id, vcpu_syscall);
>     return 0;
> }
>
> --- test.c
>
> #include <stdint.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <pthread.h>
>
> #define CATCH_BASE 0xcafebabe
>
> static void start_counting() {
>     char buf;
>     int rv = read(CATCH_BASE, &buf, 1);
>     (void)rv;
> }
>
> static void end_counting() {
>     uint64_t counter = 0;
>     int rv = read(CATCH_BASE + 1, &counter, sizeof(counter));
>     (void)rv;
>     printf("We got %lld from TCG\n", counter);
> }
>
> int global = 0;
>
> typedef struct {
>     int delay;
> } ThreadArg;
>
> static void* thread_fn(void* varg)  {
>     ThreadArg* arg = varg;
>     usleep(arg->delay);
>     free(arg);
>     return NULL;
> }
>
> int main(int argc, char** argv) {
>     int i;
>     int repeat = 100;
> #define THREAD_NUM 10
>     pthread_t threads[THREAD_NUM];
>
>     if (argc > 1) {
>         repeat = atoi(argv[1]);
>     }
>
>     for (i = 0; i < THREAD_NUM; i++) {
>         ThreadArg* arg = calloc(sizeof(ThreadArg), 1);
>         arg->delay = i * 100;
>         pthread_create(threads + i, NULL, thread_fn, arg);
>     }
>
>     start_counting();
>     for (i = 0; i < repeat; i++) {
>         global += i;
>     }
>     end_counting();
>
>     for (i = 0; i < THREAD_NUM; i++) {
>         pthread_join(threads[i], NULL);
>     }
>
>     return 0;
> }
>
> On Tue, May 12, 2020 at 3:55 AM Emilio G. Cota <cota@braap.org> wrote:
>
>> On Mon, May 11, 2020 at 18:53:19 +0300, Nikolay Igotti wrote:
>> > Attached to the mail counter.c when running with attached test.c compiled
>> > to Linux standalone binary shows failing assert, unless the patch is
>> > applied.
>>
>> I didn't get the attachment. Can you paste the code at the end of your
>> reply?
>>
>> Thanks,
>>                 Emilio
>>


-- 
Alex Bennée