Re: [Qemu-devel] qemu 1.6.1
Il 26/10/2013 11:51, Stefan Weil ha scritto: Am 24.10.2013 23:47, schrieb Paolo Bonzini: Il 24/10/2013 17:37, Stefan Weil ha scritto: Yes, that works, too. It also fixes the problem with the assertion (tested with Wine). No, we cannot remove from_, because the same interface is also used for Linux and other hosts which don't have a 'current' variable. Or we would have to call qemu_coroutine_self() to get the current coroutine. Yes, I was thinking of using qemu_coroutine_self(). By the way, can you post the two assembly language outputs for just - CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, from_); + CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, current); which AIUI works and is enough to fix the bug? Paolo See disassembled code below. I removed compiler option -fstack-protector-all to simplify the assembler code and tested that the result was not affected by this removal. The C and assembler code from the test is also available at http://qemu.weilnetz.de/test/coroutine-win32/. Here is the code with annotations broken works - push %ebx sub$0x18,%espsub$0x1c,%esp mov%ebx,0x14(%esp) mov%esi,0x18(%esp) movl $0x6d62a8,(%esp) movl $0x6d62a8,(%esp) mov0x24(%esp),%ebx mov0x24(%esp),%ebx ebx = to; call ___emutls_get_address call ___emutls_get_address eax = current; mov(%eax),%esi esi = current; mov%ebx,(%eax) mov%ebx,(%eax) current = to; mov0x28(%esp),%eax mov0x28(%esp),%eax eax = action mov%eax,0x24(%ebx) mov%eax,0x24(%ebx) to-action = action mov0x20(%ebx),%eax mov0x20(%ebx),%eax eax = to-fiber mov%eax,(%esp) mov%eax,(%esp) push to-fiber call *0x835fc0 call *0x835fc0 SwitchToFiber(to-fiber) sub$0x4,%esp sub$0x4,%esp undo PASCAL calling convention ** mov0x20(%esp),%eax eax = from mov0x24(%eax),%eax mov0x24(%esi),%eax eax = from-action mov0x14(%esp),%ebx mov0x18(%esp),%esi add$0x18,%espadd$0x1c,%esp pop%ebx ret ret I think the problem is that 0x20(%esp) gets somehow corrupted at the instruction I highlighted with **. The simplest fix then would be to add a barrier() before and after SwitchToFiber. Paolo
Re: [Qemu-devel] qemu 1.6.1
Am 27.10.2013 07:54, schrieb Paolo Bonzini: Here is the code with annotations broken works - push %ebx sub$0x18,%espsub$0x1c,%esp mov%ebx,0x14(%esp) mov%esi,0x18(%esp) movl $0x6d62a8,(%esp) movl $0x6d62a8,(%esp) mov0x24(%esp),%ebx mov0x24(%esp),%ebx ebx = to; call ___emutls_get_address call ___emutls_get_address eax = current; mov(%eax),%esi esi = current; mov%ebx,(%eax) mov%ebx,(%eax) current = to; mov0x28(%esp),%eax mov0x28(%esp),%eax eax = action mov%eax,0x24(%ebx) mov%eax,0x24(%ebx) to-action = action mov0x20(%ebx),%eax mov0x20(%ebx),%eax eax = to-fiber mov%eax,(%esp) mov%eax,(%esp) push to-fiber call *0x835fc0 call *0x835fc0 SwitchToFiber(to-fiber) sub$0x4,%esp sub$0x4,%esp undo PASCAL calling convention ** mov0x20(%esp),%eax eax = from mov0x24(%eax),%eax mov0x24(%esi),%eax eax = from-action mov0x14(%esp),%ebx mov0x18(%esp),%esi add$0x18,%espadd$0x1c,%esp pop%ebx ret ret I think the problem is that 0x20(%esp) gets somehow corrupted at the instruction I highlighted with **. The simplest fix then would be to add a barrier() before and after SwitchToFiber. Paolo I tried adding two barrier() statements around SwitchToFiber(). That change did not result in different assembler code (= unchanged behaviour, QEMU still raises an assertion). Stefan
Re: [Qemu-devel] qemu 1.6.1
Am 27.10.2013 07:54, schrieb Paolo Bonzini: I think the problem is that 0x20(%esp) gets somehow corrupted at the instruction I highlighted with **. The simplest fix then would be to add a barrier() before and after SwitchToFiber. Paolo I added some debugging output (see code at http://qemu.weilnetz.de/test/coroutine-win32/). The result with pointer values replaced by function names is shown below. There are two threads (WinMain, qemu_tcg_cpu_thread_fn) and one coroutine (bdrv_rw_co_entry). Stefan qemu_coroutine_self:WinMain qemu_in_coroutine:WinMain,null qemu_coroutine_new:bdrv_rw_co_entry qemu_coroutine_create,co=bdrv_rw_co_entry qemu_coroutine_enter(bdrv_rw_co_entry,...),self=WinMain coroutine_swap(WinMain,bdrv_rw_co_entry) qemu_coroutine_yield,self=bdrv_rw_co_entry,to=WinMain coroutine_swap(bdrv_rw_co_entry,WinMain) qemu_coroutine_enter(bdrv_rw_co_entry,...),self=WinMain coroutine_swap(WinMain,bdrv_rw_co_entry) qemu_in_coroutine:bdrv_rw_co_entry,WinMain # active coroutine bdrv_rw_co_entry is deleted coroutine_delete(bdrv_rw_co_entry) qemu_coroutine_self:qemu_tcg_cpu_thread_fn qemu_in_coroutine:qemu_tcg_cpu_thread_fn,null qemu_coroutine_create,co=bdrv_rw_co_entry qemu_coroutine_enter(bdrv_rw_co_entry,...),self=qemu_tcg_cpu_thread_fn coroutine_swap(qemu_tcg_cpu_thread_fn,bdrv_rw_co_entry) qemu_coroutine_yield,self=bdrv_rw_co_entry,to=qemu_tcg_cpu_thread_fn coroutine_swap(bdrv_rw_co_entry,qemu_tcg_cpu_thread_fn) qemu_coroutine_enter(bdrv_rw_co_entry,...),self=qemu_tcg_cpu_thread_fn coroutine_swap(qemu_tcg_cpu_thread_fn,bdrv_rw_co_entry) qemu_in_coroutine:bdrv_rw_co_entry,qemu_tcg_cpu_thread_fn qemu_in_coroutine:bdrv_rw_co_entry,qemu_tcg_cpu_thread_fn # active coroutine bdrv_rw_co_entry is deleted coroutine_delete(bdrv_rw_co_entry) # now we are still in deleted coroutine bdrv_rw_co_entry qemu_in_coroutine:bdrv_rw_co_entry,null qemu_coroutine_create,co=bdrv_rw_co_entry qemu_coroutine_enter(bdrv_rw_co_entry,...),self=bdrv_rw_co_entry coroutine_swap(bdrv_rw_co_entry,bdrv_rw_co_entry) qemu_coroutine_yield,self=bdrv_rw_co_entry,to=bdrv_rw_co_entry coroutine_swap(bdrv_rw_co_entry,bdrv_rw_co_entry) qemu_in_coroutine:bdrv_rw_co_entry,null
Re: [Qemu-devel] qemu 1.6.1
Am 24.10.2013 23:47, schrieb Paolo Bonzini: Il 24/10/2013 17:37, Stefan Weil ha scritto: Yes, that works, too. It also fixes the problem with the assertion (tested with Wine). No, we cannot remove from_, because the same interface is also used for Linux and other hosts which don't have a 'current' variable. Or we would have to call qemu_coroutine_self() to get the current coroutine. Yes, I was thinking of using qemu_coroutine_self(). By the way, can you post the two assembly language outputs for just - CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, from_); + CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, current); which AIUI works and is enough to fix the bug? Paolo See disassembled code below. I removed compiler option -fstack-protector-all to simplify the assembler code and tested that the result was not affected by this removal. The C and assembler code from the test is also available at http://qemu.weilnetz.de/test/coroutine-win32/. Stefan unpatched QEMU, crash with assertion 00448670 _qemu_coroutine_switch: 448670: 53 push %ebx 448671: 83 ec 18sub$0x18,%esp 448674: c7 04 24 a8 62 6d 00movl $0x6d62a8,(%esp) 44867b: 8b 5c 24 24 mov0x24(%esp),%ebx 44867f: e8 ec 9e 27 00 call 6c2570 ___emutls_get_address 448684: 89 18 mov%ebx,(%eax) 448686: 8b 44 24 28 mov0x28(%esp),%eax 44868a: 89 43 24mov%eax,0x24(%ebx) 44868d: 8b 43 20mov0x20(%ebx),%eax 448690: 89 04 24mov%eax,(%esp) 448693: ff 15 c0 5f 83 00 call *0x835fc0 448699: 83 ec 04sub$0x4,%esp 44869c: 8b 44 24 20 mov0x20(%esp),%eax 4486a0: 8b 40 24mov0x24(%eax),%eax 4486a3: 83 c4 18add$0x18,%esp 4486a6: 5b pop%ebx 4486a7: c3 ret patched, works 00448620 _qemu_coroutine_switch: 448620: 83 ec 1csub$0x1c,%esp 448623: c7 04 24 a8 62 6d 00movl $0x6d62a8,(%esp) 44862a: 89 5c 24 14 mov%ebx,0x14(%esp) 44862e: 8b 5c 24 24 mov0x24(%esp),%ebx 448632: 89 74 24 18 mov%esi,0x18(%esp) 448636: e8 25 9f 27 00 call 6c2560 ___emutls_get_address 44863b: 8b 30 mov(%eax),%esi 44863d: 89 18 mov%ebx,(%eax) 44863f: 8b 44 24 28 mov0x28(%esp),%eax 448643: 89 43 24mov%eax,0x24(%ebx) 448646: 8b 43 20mov0x20(%ebx),%eax 448649: 89 04 24mov%eax,(%esp) 44864c: ff 15 c0 5f 83 00 call *0x835fc0 448652: 8b 46 24mov0x24(%esi),%eax 448655: 83 ec 04sub$0x4,%esp 448658: 8b 5c 24 14 mov0x14(%esp),%ebx 44865c: 8b 74 24 18 mov0x18(%esp),%esi 448660: 83 c4 1cadd$0x1c,%esp 448663: c3 ret
Re: [Qemu-devel] qemu 1.6.1
Il 23/10/2013 21:26, Stefan Weil ha scritto: Am 23.10.2013 11:00, schrieb Paolo Bonzini: Il 23/10/2013 08:39, Michael W. Bombardieri ha scritto: Hi, My newly built qemu/win32 binary (v1.6.1) crashes in qemu-system-i386 and qemu-system-x86_64 when booting from an install CD. C:\Program Files\qemuqemu-system-x86_64 -boot d -vnc 0.0.0.0:20 -cdrom NetBSD-6.1.2-amd64.iso Assertion failed: qemu_in_coroutine(), file qemu-coroutine-lock.c, line 99 This application has requested the Runtime to terminate it in an unusual way. Please contact the application's support team for more information. I noticed that qemu-system-sparc still booted OpenBSD/sparc 5.3 install CD correctly. No further info at this stage. Any ideas? It's a known problem that not everyone can reproduce. Please compile with --disable-coroutine-pool on the configure command line. Paolo This patch also helps (at least for me, tested native and on Linux / Wine): http://repo.or.cz/w/qemu/ar7.git/commit/c777d5d62a729fd8b19847aaa0aad3d7a1f73f47 It looks like a compiler problem related to thread local storage (variable current). Ugh. I recently got several bug reports from a Windows user and included patches to fix them in my personal tree http://repo.or.cz/w/qemu/ar7.git. The binaries on qemu.weilnetz.de are based on that tree. Does something like CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, current); also work? Then we can just remove from_. Paolo
Re: [Qemu-devel] qemu 1.6.1
Am 24.10.2013 12:38, schrieb Paolo Bonzini: Il 23/10/2013 21:26, Stefan Weil ha scritto: Am 23.10.2013 11:00, schrieb Paolo Bonzini: Il 23/10/2013 08:39, Michael W. Bombardieri ha scritto: Hi, My newly built qemu/win32 binary (v1.6.1) crashes in qemu-system-i386 and qemu-system-x86_64 when booting from an install CD. C:\Program Files\qemuqemu-system-x86_64 -boot d -vnc 0.0.0.0:20 -cdrom NetBSD-6.1.2-amd64.iso Assertion failed: qemu_in_coroutine(), file qemu-coroutine-lock.c, line 99 This application has requested the Runtime to terminate it in an unusual way. Please contact the application's support team for more information. I noticed that qemu-system-sparc still booted OpenBSD/sparc 5.3 install CD correctly. No further info at this stage. Any ideas? It's a known problem that not everyone can reproduce. Please compile with --disable-coroutine-pool on the configure command line. Paolo This patch also helps (at least for me, tested native and on Linux / Wine): http://repo.or.cz/w/qemu/ar7.git/commit/c777d5d62a729fd8b19847aaa0aad3d7a1f73f47 It looks like a compiler problem related to thread local storage (variable current). Ugh. I recently got several bug reports from a Windows user and included patches to fix them in my personal tree http://repo.or.cz/w/qemu/ar7.git. The binaries on qemu.weilnetz.de are based on that tree. Does something like CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, current); also work? Then we can just remove from_. Paolo Yes, that works, too. It also fixes the problem with the assertion (tested with Wine). No, we cannot remove from_, because the same interface is also used for Linux and other hosts which don't have a 'current' variable. Or we would have to call qemu_coroutine_self() to get the current coroutine. Stefan
Re: [Qemu-devel] qemu 1.6.1
Il 24/10/2013 17:37, Stefan Weil ha scritto: Yes, that works, too. It also fixes the problem with the assertion (tested with Wine). No, we cannot remove from_, because the same interface is also used for Linux and other hosts which don't have a 'current' variable. Or we would have to call qemu_coroutine_self() to get the current coroutine. Yes, I was thinking of using qemu_coroutine_self(). By the way, can you post the two assembly language outputs for just - CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, from_); + CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, current); which AIUI works and is enough to fix the bug? Paolo
Re: [Qemu-devel] qemu 1.6.1
Il 23/10/2013 08:39, Michael W. Bombardieri ha scritto: Hi, My newly built qemu/win32 binary (v1.6.1) crashes in qemu-system-i386 and qemu-system-x86_64 when booting from an install CD. C:\Program Files\qemuqemu-system-x86_64 -boot d -vnc 0.0.0.0:20 -cdrom NetBSD-6.1.2-amd64.iso Assertion failed: qemu_in_coroutine(), file qemu-coroutine-lock.c, line 99 This application has requested the Runtime to terminate it in an unusual way. Please contact the application's support team for more information. I noticed that qemu-system-sparc still booted OpenBSD/sparc 5.3 install CD correctly. No further info at this stage. Any ideas? It's a known problem that not everyone can reproduce. Please compile with --disable-coroutine-pool on the configure command line. Paolo
Re: [Qemu-devel] qemu 1.6.1
Am 23.10.2013 11:00, schrieb Paolo Bonzini: Il 23/10/2013 08:39, Michael W. Bombardieri ha scritto: Hi, My newly built qemu/win32 binary (v1.6.1) crashes in qemu-system-i386 and qemu-system-x86_64 when booting from an install CD. C:\Program Files\qemuqemu-system-x86_64 -boot d -vnc 0.0.0.0:20 -cdrom NetBSD-6.1.2-amd64.iso Assertion failed: qemu_in_coroutine(), file qemu-coroutine-lock.c, line 99 This application has requested the Runtime to terminate it in an unusual way. Please contact the application's support team for more information. I noticed that qemu-system-sparc still booted OpenBSD/sparc 5.3 install CD correctly. No further info at this stage. Any ideas? It's a known problem that not everyone can reproduce. Please compile with --disable-coroutine-pool on the configure command line. Paolo This patch also helps (at least for me, tested native and on Linux / Wine): http://repo.or.cz/w/qemu/ar7.git/commit/c777d5d62a729fd8b19847aaa0aad3d7a1f73f47 It looks like a compiler problem related to thread local storage (variable current). I recently got several bug reports from a Windows user and included patches to fix them in my personal tree http://repo.or.cz/w/qemu/ar7.git. The binaries on qemu.weilnetz.de are based on that tree. Stefan
