Re: [Qemu-devel] [PATCH v2 1/5] compiler: support Darwin weak references

[TeLeMan]

On Fri, Nov 2, 2012 at 10:43 PM, Paolo Bonzini <address@hidden> 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 <address@hidden>
> ---
>         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
>
>
>