Re: [Qemu-devel] [PATCH v2 05/39] fdsets: use weak aliases instead of qemu-tool.c/qemu-user.c

[Stefan Weil <sw@weilnetz.de>]

Am 16.11.2012 10:35, schrieb Paolo Bonzini:
> Il 15/11/2012 23:18, Stefan Weil ha scritto:
>> Am 15.11.2012 21:52, schrieb Paolo Bonzini:
>>> Il 15/11/2012 19:01, Stefan Weil ha scritto:
>>>> 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.
>>>
>>> I cannot reproduce this, so it must be an assembler or linker bug.
>>>
>>> Can you try the alternative code that is used for Mac OS X?
>>
>> The code which is used for Mac OS X also compiles and
>> results in the same run-time bug with Wine:
>
> Ok, I reproduced the original binutils bug, and found a typo in the
> weakrefs implementation.  Does this work for you?
>
> diff --git a/compiler.h b/compiler.h
> index 55d7d74..d552757 100644
> --- a/compiler.h
> +++ b/compiler.h
> @@ -50,11 +50,12 @@
>   #   define __printf__ __gnu_printf__
>   #  endif
>   # endif
> -# if defined(__APPLE__)
> +# if defined(__APPLE__) || defined(_WIN32)
>   #  define QEMU_WEAK_ALIAS(newname, oldname) \
> -        static typeof(oldname) weak_##newname __attribute__((unused, weakref(#oldname)))
> +        static typeof(oldname) weak_##newname __attribute__((unused, weakref(#newname)))
>   #  define QEMU_WEAK_REF(newname, oldname) (weak_##newname ? weak_##newname : oldname)
>   # else
> +#error
>   #  define QEMU_WEAK_ALIAS(newname, oldname) \
>           typeof(oldname) newname __attribute__((weak, alias (#oldname)))
>   #  define QEMU_WEAK_REF(newname, oldname) newname
>
>
> If it still doesn't work, let's make sure that this reduced testcase works
> for you:

Tested-by: Stefan Weil <sw@weilnetz.de>

Great, the above patch fixes w32/w64 (native and with Wine).
Is this modification needed / does it work with MacOS X, too?

With the reduced testcase, I get the same results as in your test.

Regards
Stefan


>
> g1.c:
> #include<stdio.h>
> int f() { printf("strong"); return 82; }
> int g() { printf("strong"); return 83; }
>
> g2.c:
> #include<stdio.h>
> static int weak_f() { return 42; }
> static int weak_g() { return 43; }
> typeof(weak_f) f __attribute__((__weak__, __alias__("weak_f")));
> typeof(weak_g) g __attribute__((__weak__, __alias__("weak_g")));
> int main() { printf("%d/%d\n", f(), g()); }
>
> g3.c:
> #include<stdio.h>
> static int default_f() { return 42; }
> static int default_g() { return 43; }
> static typeof(default_f) weak_f __attribute__((__weakref__("f")));
> static typeof(default_g) weak_g __attribute__((__weakref__("g")));
> int main() { printf("%d/%d\n", (weak_f?:default_f)(), (weak_g?:default_g)()); }
>
> Output should be:
> - 42/43 for "gcc g2.c"
> - 42/43 for "gcc g3.c"
> - strongstrong82/83 for "gcc g1.c g2.c"
> - strongstrong82/83 for "gcc g1.c g3.c"
>
> Output on Windows is:
> - 42/42 for "gcc g2.c"
> - 42/43 for "gcc g3.c"
> - segfault for "gcc g1.c g2.c"
> - strongstrong82/83 for "gcc g1.c g3.c"
>
> So, indeed "normal" weak symbols are broken, but weakrefs seem to work.
>
> If the above patch doesn't work, and/or the reduced testcase fails,
> please send to me:
>
> - the .exe for "gcc g1.c g3.c"
> - the .s file for g3.s
> - the .o file for osdep.o
> - the linked .exe (I assume both the windows and console versions fail)
> - perhaps the .s file for osdep too; add --save-temps to the compiler command line to get it
>
> Paolo