All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@gmail.com>
To: Bug 902148 <902148@bugs.launchpad.net>
Cc: richard.sandiford@linaro.org, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [Bug 902148] Re: qemu-img V1.0 hangs on creating Image (0.15.1 runs)
Date: Tue, 20 Dec 2011 11:53:15 +0000	[thread overview]
Message-ID: <CAJSP0QVd=stbPDERX=4LGcNZWGTfz8AbpqDxXPc=hwayKhu94A@mail.gmail.com> (raw)
In-Reply-To: <CAJSP0QXtUQoMBLgPd5=W7oAtRoOKbcJcQmG_rMPzvAf4Fz1EAw@mail.gmail.com>

On Tue, Dec 20, 2011 at 10:49 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Tue, Dec 20, 2011 at 10:25 AM, Michael Niehren
> <902148@bugs.launchpad.net> wrote:
>> here the compiled binary with your patch and the --enable-debug option
>> This one works
>
> Thanks, I'm able to reproduce the problem here with your original
> binary.  Will send an update once I've figured out what the problem
> is.

This looks like an interesting bug, perhaps a bug in gcc.  I have to
preface that with what the QEMU code is doing here is pretty tricky,
so it might be a regular bug in QEMU... :)

Here is the C function that triggers the bug:

static void coroutine_trampoline(int i0, int i1)
{
    union cc_arg arg;
    CoroutineUContext *self;
    Coroutine *co;

    arg.i[0] = i0;  <-- workaround for makecontext() limitation,
    arg.i[1] = i1;      need to pass Coroutine *co argument in as
    self = arg.p;       two ints and use a union to extract it.
    co = &self->base;

    /* Initialize longjmp environment and switch back the caller */
    if (!setjmp(self->env)) {
        longjmp(*(jmp_buf *)co->entry_arg, 1);
    }  <-- I think this part is not relevant to the bug

    while (true) {  <-- here is the loop that miscompiles
        co->entry(co->entry_arg);
        qemu_coroutine_switch(co, co->caller, COROUTINE_TERMINATE);
    }
}

Here is the loop output of my compiler - this works fine:

   31bd0:       48 8b 44 24 08          mov    0x8(%rsp),%rax  <-- co->entry
   31bd5:       48 8b 78 08             mov    0x8(%rax),%rdi  <-- co->entry_arg
   31bd9:       ff 10                   callq  *(%rax)
   31bdb:       48 8b 44 24 08          mov    0x8(%rsp),%rax
   31be0:       48 8b 7c 24 10          mov    0x10(%rsp),%rdi
   31be5:       ba 02 00 00 00          mov    $0x2,%edx
   31bea:       48 8b 70 10             mov    0x10(%rax),%rsi
   31bee:       e8 2d ff ff ff          callq  31b20 <qemu_coroutine_switch>
   31bf3:       eb db                   jmp    31bd0 <coroutine_trampoline+0x40>

Here is your compiler output - this is broken:

   31c40:       4c 89 e7                mov    %r12,%rdi  <--
co->entry_args from %r12
   31c43:       ff d5                   callq  *%rbp      <--
co->entry already in %rbp
   31c45:       48 8b 3c 24             mov    (%rsp),%rdi
   31c49:       ba 02 00 00 00          mov    $0x2,%edx
   31c4e:       48 89 de                mov    %rbx,%rsi
   31c51:       e8 2a ff ff ff          callq  31b80 <qemu_coroutine_switch>
   31c56:       eb e8                   jmp    31c40 <coroutine_trampoline+0x50>

Your binary does not reload the coroutine entry function pointer or
entry_args argument fields.  As a result coroutines are not working in
your QEMU build.

It seems your compiler is disregarding the co->entry() function
pointer call and assuming co's fields will not change.  The
setjmp()/longjmp() and cc_args union make this an ugly function -
perhaps they play a part in making the compiler think it's okay to
keep stale values of co's fields around.

Richard Sandiford has suggested a way to capture what gcc is doing:

$ cd qemu/
$ rm coroutine-ucontext.o
$ make V=1 coroutine-ucontext.o

Now copy the gcc command-line that was displayed but add
-fdump-tree-all-details:

$ gcc ... -fdump-tree-all-details coroutine-ucontext.c

Collect all the gcc internals output:

$ tar czf coroutine-ucontext.tgz coroutine-ucontext.c.*

Please upload the tarball and also post your Linux distro and gcc
--version details.

Thanks,
Stefan

  reply	other threads:[~2011-12-20 11:53 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-09 13:10 [Qemu-devel] [Bug 902148] [NEW] qemu-img V1.0 hangs on creating Image (0.15.1 runs) Michael Niehren
2011-12-12 10:09 ` Stefan Hajnoczi
2011-12-19 10:02 ` [Qemu-devel] [Bug 902148] " Michael Niehren
2011-12-19 10:31   ` Stefan Hajnoczi
2011-12-19 10:52 ` Michael Niehren
2011-12-19 13:22   ` Stefan Hajnoczi
2011-12-19 17:26 ` Michael Niehren
2011-12-20  8:04   ` Stefan Hajnoczi
2011-12-20 10:17 ` Michael Niehren
2011-12-20 10:25 ` Michael Niehren
2011-12-20 10:49   ` Stefan Hajnoczi
2011-12-20 11:53     ` Stefan Hajnoczi [this message]
2011-12-20 15:25 ` Michael Niehren
2011-12-20 16:49   ` Stefan Hajnoczi
2011-12-20 21:45     ` Stefan Hajnoczi
2012-01-09 11:25     ` Kevin Wolf
2012-01-09 13:00       ` Stefan Hajnoczi
2012-01-14 15:40         ` Zhi Yong Wu
2012-01-15 11:14           ` Stefan Hajnoczi
2012-01-16  2:44             ` Zhi Yong Wu
2011-12-20 16:51 ` Stefan Hajnoczi
2011-12-20 23:59 ` Michael Niehren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJSP0QVd=stbPDERX=4LGcNZWGTfz8AbpqDxXPc=hwayKhu94A@mail.gmail.com' \
    --to=stefanha@gmail.com \
    --cc=902148@bugs.launchpad.net \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.sandiford@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.