Patchwork [v2,1/5] compiler: support Darwin weak references

login
register
mail settings
Submitter Paolo Bonzini
Date Nov. 2, 2012, 2:43 p.m.
Message ID <1351867404-25510-2-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/196570/
State New
Headers show

Comments

Paolo Bonzini - Nov. 2, 2012, 2:43 p.m.
Weakrefs only tell you if the symbol was defined elsewhere, so you
need a further check at runtime to pick the default definition
when needed.

This could be automated by the compiler, but it does not do it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
        v1->v2: add unused attribute

 compiler.h     |  9 ++++++++-
 osdep.c        | 56 ++++++++++++++++++++++++++++++++------------------------
 oslib-win32.c  | 12 +++++++-----
 qemu-sockets.c | 40 ++++++++++++++++++++++------------------
 qmp.c          |  2 ++
 5 file modificati, 71 inserzioni(+), 48 rimozioni(-)
TeLeMan - Nov. 5, 2012, 7:50 a.m.
On Fri, Nov 2, 2012 at 10:43 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Weakrefs only tell you if the symbol was defined elsewhere, so you
> need a further check at runtime to pick the default definition
> when needed.
>
> This could be automated by the compiler, but it does not do it.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>         v1->v2: add unused attribute
>
>  compiler.h     |  9 ++++++++-
>  osdep.c        | 56 ++++++++++++++++++++++++++++++++------------------------
>  oslib-win32.c  | 12 +++++++-----
>  qemu-sockets.c | 40 ++++++++++++++++++++++------------------
>  qmp.c          |  2 ++
>  5 file modificati, 71 inserzioni(+), 48 rimozioni(-)
>
> diff --git a/compiler.h b/compiler.h
> index 58865d6..55d7d74 100644
> --- a/compiler.h
> +++ b/compiler.h
> @@ -50,8 +50,15 @@
>  #   define __printf__ __gnu_printf__
>  #  endif
>  # endif
> -# define QEMU_WEAK_ALIAS(newname, oldname) \
> +# if defined(__APPLE__)
> +#  define QEMU_WEAK_ALIAS(newname, oldname) \
> +        static typeof(oldname) weak_##newname __attribute__((unused, weakref(#oldname)))
> +#  define QEMU_WEAK_REF(newname, oldname) (weak_##newname ? weak_##newname : oldname)
> +# else
> +#  define QEMU_WEAK_ALIAS(newname, oldname) \
>          typeof(oldname) newname __attribute__((weak, alias (#oldname)))
> +#  define QEMU_WEAK_REF(newname, oldname) newname
> +# endif
>  #else
>  #define GCC_ATTR /**/
>  #define GCC_FMT_ATTR(n, m)
> diff --git a/osdep.c b/osdep.c
> index a87d4a4..2f7a491 100644
> --- a/osdep.c
> +++ b/osdep.c
> @@ -54,6 +54,38 @@ static bool fips_enabled = false;
>
>  static const char *qemu_version = QEMU_VERSION;
>
> +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);
> +#define monitor_fdset_get_fd \
> +    QEMU_WEAK_REF(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);
> +#define monitor_fdset_dup_fd_add \
> +    QEMU_WEAK_REF(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);
> +#define monitor_fdset_dup_fd_remove \
> +    QEMU_WEAK_REF(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);
> +#define monitor_fdset_dup_fd_find \
> +    QEMU_WEAK_REF(monitor_fdset_dup_fd_remove, default_fdset_dup_fd_find)
Should here be QEMU_WEAK_REF(monitor_fdset_dup_fd_find,
default_fdset_dup_fd_find)?

Another issue: Some weird codes were generated on MinGW + gcc 4.5.1:

244         fdset_id = monitor_fdset_dup_fd_find(fd);
(gdb) x/3i $pc
=> 0x553e9e <qemu_close+22>:    mov    -0x1c(%ebp),%eax
   0x553ea1 <qemu_close+25>:    mov    %eax,(%esp)
   0x553ea4 <qemu_close+28>:
    call   0x637bb6 <monitor_fdset_dup_fd_find_remove+29>

It's called directly into the middle of monitor_fdset_dup_fd_find_remove.

> +
>  int socket_set_cork(int fd, int v)
>  {
>  #if defined(SOL_TCP) && defined(TCP_CORK)
> @@ -400,27 +432,3 @@ 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/oslib-win32.c b/oslib-win32.c
> index 9ca83df..326a2bd 100644
> --- a/oslib-win32.c
> +++ b/oslib-win32.c
> @@ -32,6 +32,13 @@
>  #include "trace.h"
>  #include "qemu_socket.h"
>
> +static void default_qemu_fd_register(int fd)
> +{
> +}
> +QEMU_WEAK_ALIAS(qemu_fd_register, default_qemu_fd_register);
> +#define qemu_fd_register \
> +    QEMU_WEAK_REF(qemu_fd_register, default_qemu_fd_register)
> +
>  void *qemu_oom_check(void *ptr)
>  {
>      if (ptr == NULL) {
> @@ -150,8 +157,3 @@ int qemu_get_thread_id(void)
>  {
>      return GetCurrentThreadId();
>  }
> -
> -static void default_qemu_fd_register(int fd)
> -{
> -}
> -QEMU_WEAK_ALIAS(qemu_fd_register, default_qemu_fd_register);
> diff --git a/qemu-sockets.c b/qemu-sockets.c
> index f2a6371..abcd791 100644
> --- a/qemu-sockets.c
> +++ b/qemu-sockets.c
> @@ -61,6 +61,28 @@ static QemuOptsList dummy_opts = {
>      },
>  };
>
> +static int default_monitor_get_fd(Monitor *mon, const char *name, Error **errp)
> +{
> +    error_setg(errp, "only QEMU supports file descriptor passing");
> +    return -1;
> +}
> +QEMU_WEAK_ALIAS(monitor_get_fd, default_monitor_get_fd);
> +#define monitor_get_fd \
> +    QEMU_WEAK_REF(monitor_get_fd, default_monitor_get_fd)
> +
> +static int default_qemu_set_fd_handler2(int fd,
> +                                        IOCanReadHandler *fd_read_poll,
> +                                        IOHandler *fd_read,
> +                                        IOHandler *fd_write,
> +                                        void *opaque)
> +
> +{
> +    abort();
> +}
> +QEMU_WEAK_ALIAS(qemu_set_fd_handler2, default_qemu_set_fd_handler2);
> +#define qemu_set_fd_handler2 \
> +    QEMU_WEAK_REF(qemu_set_fd_handler2, default_qemu_set_fd_handler2)
> +
>  static int inet_getport(struct addrinfo *e)
>  {
>      struct sockaddr_in *i4;
> @@ -967,21 +989,3 @@ int socket_init(void)
>  #endif
>      return 0;
>  }
> -
> -static int default_monitor_get_fd(Monitor *mon, const char *name, Error **errp)
> -{
> -    error_setg(errp, "only QEMU supports file descriptor passing");
> -    return -1;
> -}
> -QEMU_WEAK_ALIAS(monitor_get_fd, default_monitor_get_fd);
> -
> -static int default_qemu_set_fd_handler2(int fd,
> -                                        IOCanReadHandler *fd_read_poll,
> -                                        IOHandler *fd_read,
> -                                        IOHandler *fd_write,
> -                                        void *opaque)
> -
> -{
> -    abort();
> -}
> -QEMU_WEAK_ALIAS(qemu_set_fd_handler2, default_qemu_set_fd_handler2);
> diff --git a/qmp.c b/qmp.c
> index 638888a..13e83a5 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -477,6 +477,8 @@ static CpuDefinitionInfoList *default_arch_query_cpu_definitions(Error **errp)
>      return NULL;
>  }
>  QEMU_WEAK_ALIAS(arch_query_cpu_definitions, default_arch_query_cpu_definitions);
> +#define arch_query_cpu_definitions \
> +    QEMU_WEAK_REF(arch_query_cpu_definitions, default_arch_query_cpu_definitions)
>
>  CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
>  {
> --
> 1.7.12.1
>
>
>

Patch

diff --git a/compiler.h b/compiler.h
index 58865d6..55d7d74 100644
--- a/compiler.h
+++ b/compiler.h
@@ -50,8 +50,15 @@ 
 #   define __printf__ __gnu_printf__
 #  endif
 # endif
-# define QEMU_WEAK_ALIAS(newname, oldname) \
+# if defined(__APPLE__)
+#  define QEMU_WEAK_ALIAS(newname, oldname) \
+        static typeof(oldname) weak_##newname __attribute__((unused, weakref(#oldname)))
+#  define QEMU_WEAK_REF(newname, oldname) (weak_##newname ? weak_##newname : oldname)
+# else
+#  define QEMU_WEAK_ALIAS(newname, oldname) \
         typeof(oldname) newname __attribute__((weak, alias (#oldname)))
+#  define QEMU_WEAK_REF(newname, oldname) newname
+# endif
 #else
 #define GCC_ATTR /**/
 #define GCC_FMT_ATTR(n, m)
diff --git a/osdep.c b/osdep.c
index a87d4a4..2f7a491 100644
--- a/osdep.c
+++ b/osdep.c
@@ -54,6 +54,38 @@  static bool fips_enabled = false;
 
 static const char *qemu_version = QEMU_VERSION;
 
+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);
+#define monitor_fdset_get_fd \
+    QEMU_WEAK_REF(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);
+#define monitor_fdset_dup_fd_add \
+    QEMU_WEAK_REF(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);
+#define monitor_fdset_dup_fd_remove \
+    QEMU_WEAK_REF(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);
+#define monitor_fdset_dup_fd_find \
+    QEMU_WEAK_REF(monitor_fdset_dup_fd_remove, default_fdset_dup_fd_find)
+
 int socket_set_cork(int fd, int v)
 {
 #if defined(SOL_TCP) && defined(TCP_CORK)
@@ -400,27 +432,3 @@  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/oslib-win32.c b/oslib-win32.c
index 9ca83df..326a2bd 100644
--- a/oslib-win32.c
+++ b/oslib-win32.c
@@ -32,6 +32,13 @@ 
 #include "trace.h"
 #include "qemu_socket.h"
 
+static void default_qemu_fd_register(int fd)
+{
+}
+QEMU_WEAK_ALIAS(qemu_fd_register, default_qemu_fd_register);
+#define qemu_fd_register \
+    QEMU_WEAK_REF(qemu_fd_register, default_qemu_fd_register)
+
 void *qemu_oom_check(void *ptr)
 {
     if (ptr == NULL) {
@@ -150,8 +157,3 @@  int qemu_get_thread_id(void)
 {
     return GetCurrentThreadId();
 }
-
-static void default_qemu_fd_register(int fd)
-{
-}
-QEMU_WEAK_ALIAS(qemu_fd_register, default_qemu_fd_register);
diff --git a/qemu-sockets.c b/qemu-sockets.c
index f2a6371..abcd791 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -61,6 +61,28 @@  static QemuOptsList dummy_opts = {
     },
 };
 
+static int default_monitor_get_fd(Monitor *mon, const char *name, Error **errp)
+{
+    error_setg(errp, "only QEMU supports file descriptor passing");
+    return -1;
+}
+QEMU_WEAK_ALIAS(monitor_get_fd, default_monitor_get_fd);
+#define monitor_get_fd \
+    QEMU_WEAK_REF(monitor_get_fd, default_monitor_get_fd)
+
+static int default_qemu_set_fd_handler2(int fd,
+                                        IOCanReadHandler *fd_read_poll,
+                                        IOHandler *fd_read,
+                                        IOHandler *fd_write,
+                                        void *opaque)
+
+{
+    abort();
+}
+QEMU_WEAK_ALIAS(qemu_set_fd_handler2, default_qemu_set_fd_handler2);
+#define qemu_set_fd_handler2 \
+    QEMU_WEAK_REF(qemu_set_fd_handler2, default_qemu_set_fd_handler2)
+
 static int inet_getport(struct addrinfo *e)
 {
     struct sockaddr_in *i4;
@@ -967,21 +989,3 @@  int socket_init(void)
 #endif
     return 0;
 }
-
-static int default_monitor_get_fd(Monitor *mon, const char *name, Error **errp)
-{
-    error_setg(errp, "only QEMU supports file descriptor passing");
-    return -1;
-}
-QEMU_WEAK_ALIAS(monitor_get_fd, default_monitor_get_fd);
-
-static int default_qemu_set_fd_handler2(int fd,
-                                        IOCanReadHandler *fd_read_poll,
-                                        IOHandler *fd_read,
-                                        IOHandler *fd_write,
-                                        void *opaque)
-
-{
-    abort();
-}
-QEMU_WEAK_ALIAS(qemu_set_fd_handler2, default_qemu_set_fd_handler2);
diff --git a/qmp.c b/qmp.c
index 638888a..13e83a5 100644
--- a/qmp.c
+++ b/qmp.c
@@ -477,6 +477,8 @@  static CpuDefinitionInfoList *default_arch_query_cpu_definitions(Error **errp)
     return NULL;
 }
 QEMU_WEAK_ALIAS(arch_query_cpu_definitions, default_arch_query_cpu_definitions);
+#define arch_query_cpu_definitions \
+    QEMU_WEAK_REF(arch_query_cpu_definitions, default_arch_query_cpu_definitions)
 
 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
 {