From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59005) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TZ3lL-0006oz-A0 for qemu-devel@nongnu.org; Thu, 15 Nov 2012 13:02:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TZ3lI-0008Cp-83 for qemu-devel@nongnu.org; Thu, 15 Nov 2012 13:02:03 -0500 Received: from v220110690675601.yourvserver.net ([78.47.199.172]:40883) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TZ3lH-0008Cj-Sj for qemu-devel@nongnu.org; Thu, 15 Nov 2012 13:02:00 -0500 Message-ID: <50A52E14.2070106@weilnetz.de> Date: Thu, 15 Nov 2012 19:01:56 +0100 From: Stefan Weil MIME-Version: 1.0 References: <1351697456-16107-1-git-send-email-pbonzini@redhat.com> <1351697456-16107-6-git-send-email-pbonzini@redhat.com> In-Reply-To: <1351697456-16107-6-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 05/39] fdsets: use weak aliases instead of qemu-tool.c/qemu-user.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Blue Swirl , aliguori@us.ibm.com, qemu-devel@nongnu.org, stefanha@redhat.com Am 31.10.2012 16:30, schrieb Paolo Bonzini: > Signed-off-by: Paolo Bonzini > --- > cutils.c | 5 ----- > osdep.c | 30 ++++++++++++++++++++++++++++++ > qemu-common.h | 1 - > qemu-tool.c | 20 -------------------- > qemu-user.c | 20 -------------------- > 5 file modificati, 30 inserzioni(+), 46 rimozioni(-) > > diff --git a/cutils.c b/cutils.c > index 6f9f799..4f0692f 100644 > --- a/cutils.c > +++ b/cutils.c > @@ -280,11 +280,6 @@ int qemu_parse_fd(const char *param) > return fd; > } > > -int qemu_parse_fdset(const char *param) > -{ > - return qemu_parse_fd(param); > -} > - > /* round down to the nearest power of 2*/ > int64_t pow2floor(int64_t value) > { > diff --git a/osdep.c b/osdep.c > index 3b25297..0061f74 100644 > --- a/osdep.c > +++ b/osdep.c > @@ -144,6 +144,11 @@ fail: > errno = serrno; > return -1; > } > + > +static int qemu_parse_fdset(const char *param) > +{ > + return qemu_parse_fd(param); > +} > #endif > > /* > @@ -404,3 +409,28 @@ bool fips_get_state(void) > { > return fips_enabled; > } > + > + > +static int default_fdset_get_fd(int64_t fdset_id, int flags) > +{ > + return -1; > +} > +QEMU_WEAK_ALIAS(monitor_fdset_get_fd, default_fdset_get_fd); > + > +static int default_fdset_dup_fd_add(int64_t fdset_id, int dup_fd) > +{ > + return -1; > +} > +QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_add, default_fdset_dup_fd_add); > + > +static int default_fdset_dup_fd_remove(int dup_fd) > +{ > + return -1; > +} > +QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_remove, default_fdset_dup_fd_remove); > + > +static int default_fdset_dup_fd_find(int dup_fd) > +{ > + return -1; > +} > +QEMU_WEAK_ALIAS(monitor_fdset_dup_fd_find, default_fdset_dup_fd_find); > diff --git a/qemu-common.h b/qemu-common.h > index b54612b..36ce522 100644 > --- a/qemu-common.h > +++ b/qemu-common.h > @@ -167,7 +167,6 @@ int qemu_fls(int i); > int qemu_fdatasync(int fd); > int fcntl_setfl(int fd, int flag); > int qemu_parse_fd(const char *param); > -int qemu_parse_fdset(const char *param); > > /* > * strtosz() suffixes used to specify the default treatment of an > diff --git a/qemu-tool.c b/qemu-tool.c > index f2f9813..84273ae 100644 > --- a/qemu-tool.c > +++ b/qemu-tool.c > @@ -68,26 +68,6 @@ void monitor_protocol_event(MonitorEvent event, QObject *data) > { > } > > -int monitor_fdset_get_fd(int64_t fdset_id, int flags) > -{ > - return -1; > -} > - > -int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd) > -{ > - return -1; > -} > - > -int monitor_fdset_dup_fd_remove(int dup_fd) > -{ > - return -1; > -} > - > -int monitor_fdset_dup_fd_find(int dup_fd) > -{ > - return -1; > -} > - > int64_t cpu_get_clock(void) > { > return qemu_get_clock_ns(rt_clock); > diff --git a/qemu-user.c b/qemu-user.c > index 13fb9ae..08ccb0f 100644 > --- a/qemu-user.c > +++ b/qemu-user.c > @@ -35,23 +35,3 @@ void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) > void monitor_set_error(Monitor *mon, QError *qerror) > { > } > - > -int monitor_fdset_get_fd(int64_t fdset_id, int flags) > -{ > - return -1; > -} > - > -int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd) > -{ > - return -1; > -} > - > -int monitor_fdset_dup_fd_remove(int dup_fd) > -{ > - return -1; > -} > - > -int monitor_fdset_dup_fd_find(int dup_fd) > -{ > - return -1; > -} Hi Paolo, this patch breaks QEMU on 32 and 64 bit hosts, native and with Wine. It's easy to reproduce the SIGSEGV crash: just add a -snapshot option. Obviously the critical code is executed only when this option was used. Here is a simple command line using Wine: wine i386-softmmu/qemu-system-i386 -L pc-bios -snapshot Makefile The disk image does not matter, so I just selected QEMU's Makefile. It looks like weak symbols are not really working with MinGW (Blue Swirl previously pointed out that only ELF and a.out are officially supported). I can see in the debugger that QEMU wants to call monitor_fdset_dup_fd_find from qemu_close. In previous versions, this was just a dummy function returning 0. Now, it is the function in monitor.c, but the address does not match exactly, so the code addresses lines near the beginning of monitor_fdset_dup_fd_find which does not work of course. A trivial workaround is calling default_fdset_dup_fd_find which restores the old behaviour. I expect that all other weak functions would show the same problem if they were used. Regards, Stefan