https://bugs.winehq.org/show_bug.cgi?id=38713
Bug ID: 38713 Summary: Link failure due to use of libunwind in ntdll Product: Wine Version: 1.7.44 Hardware: x86 OS: Linux Status: UNCONFIRMED Keywords: regression Severity: normal Priority: P2 Component: ntdll Assignee: wine-bugs@winehq.org Reporter: ken@codeweavers.com Regression SHA1: bbcfa6b4ee5d7fea47c61c84d0bbe789e1ec4466 Distribution: Other
Created attachment 51643 --> https://bugs.winehq.org/attachment.cgi?id=51643 build error
This was reported to me via direct email. The reporter didn't want to submit a bug report, so I'm doing so.
After my commit bbcfa6b4ee5d7fea47c61c84d0bbe789e1ec4466 which uses libunwind as an alternative to DWARF unwinding, ntdll fails to link on systems with libunwind installed.
The issue is that I didn't add any new libraries for libunwind support. On OS X, libunwind is part of the system library, so nothing was needed.
The reporter was using Manjaro Linux 64bit but I doubt it's relevant.
https://bugs.winehq.org/show_bug.cgi?id=38713
--- Comment #1 from Ken Thomases ken@codeweavers.com --- Evidently, on Linux with the libunwind package installed, it is necessary to link against another library. My guess is that the eventual solution will have the effect of adding "-lunwind" to the link line.
The proper solution is for somebody who can build and test on Linux to write a configure test which would set some configure variable such as "LIBUNWIND_LIBS" and add that to the EXTRALIBS variable in dlls/ntdll/Makefile.in.
I would say that this should be modeled after the check for -lrt. Search for "RT_LIBS" in configure.ac. However, there's a complication. As you can see from the link errors, the exact symbol name that's needed is not the normal function name. For example, the code uses unw_init_local() but that results in a reference to _ULx86_64_init_local() (or, without UNW_LOCAL_ONLY defined, _Ux86_64_init_local()). We wouldn't want the configure test to use that symbol name because it's an implementation detail. (For example, that's not the symbol name on OS X.)
So, the configure test is going to have to #define UNW_LOCAL_ONLY and #include <libunwind.h> to get code that actually depends on the symbol as appropriate for the platform. To do this, it may be necessary to copy the definition of AC_SEARCH_LIBS from autoconf to Wine's aclocal.m4 as, say, WINE_SEARCH_LIBS with an additional parameter of prologue code (as taken by AC_LANG_PROGRAM(), for example). Then, we'd use that with a prologue of:
#ifdef HAVE_LIBUNWIND_H # define UNW_LOCAL_ONLY # include <libunwind.h> #endif
and code like:
unw_context_t unw_context; unw_cursor_t cursor; unw_getcontext( &unw_context ); unw_init_local( &cursor, &unw_context );
The reason to test both unw_getcontext() and unw_init_local() is that they seem to be affected differently by the UNW_LOCAL_ONLY macro.
https://bugs.winehq.org/show_bug.cgi?id=38713
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download, source
https://bugs.winehq.org/show_bug.cgi?id=38713
--- Comment #2 from Ken Thomases ken@codeweavers.com --- (In reply to Ken Thomases from comment #1)
[I]t may be necessary to copy the definition of AC_SEARCH_LIBS from autoconf to Wine's aclocal.m4 ...
Well, we can't do that, as such. The autoconf sources are GPLv3.
https://bugs.winehq.org/show_bug.cgi?id=38713
Erich E. Hoover erich.e.hoover@wine-staging.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |erich.e.hoover@wine-staging | |.com
--- Comment #3 from Erich E. Hoover erich.e.hoover@wine-staging.com --- Created attachment 51673 --> https://bugs.winehq.org/attachment.cgi?id=51673 ntdll: Link to libunwind on x86-64 Linux.
(In reply to Ken Thomases from comment #1)
... The proper solution is for somebody who can build and test on Linux to write a configure test which would set some configure variable such as "LIBUNWIND_LIBS" and add that to the EXTRALIBS variable in dlls/ntdll/Makefile.in. ...
Please try the attached on Mac :) I used UNWIND_LIBS instead of LIBUNWIND_LIBS since that seems to match the current convention for these libraries.
https://bugs.winehq.org/show_bug.cgi?id=38713
--- Comment #4 from Erich E. Hoover erich.e.hoover@wine-staging.com --- Note: Presumably on Mac there is no libunwind or it is not desirable to link against it. If it's ok to link against then let me know and I can replace WINE_NOTICE_WITH with WINE_ERROR_WITH.
https://bugs.winehq.org/show_bug.cgi?id=38713
Erich E. Hoover erich.e.hoover@wine-staging.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |NEW Hardware|x86 |x86-64 Ever confirmed|0 |1
--- Comment #5 from Erich E. Hoover erich.e.hoover@wine-staging.com --- (Confirming)
https://bugs.winehq.org/show_bug.cgi?id=38713
--- Comment #6 from Ken Thomases ken@codeweavers.com --- Created attachment 51676 --> https://bugs.winehq.org/attachment.cgi?id=51676 configure check for libunwind
(In reply to Erich E. Hoover from comment #3)
Created attachment 51673 [details] ntdll: Link to libunwind on x86-64 Linux.
Thanks for this.
Please try the attached on Mac :) I used UNWIND_LIBS instead of LIBUNWIND_LIBS since that seems to match the current convention for these libraries.
Hmm. Well, on the Mac it's mostly harmless. There is no separate library for the libunwind functionality. It's present in the C library that's linked by default. So, your check doesn't find any library and the link goes fine.
However, it does emit an unnecessary notice.
I think that checking for _Unwind_RaiseException probably is a good solution to the symbol renaming that the header does for things like unw_init_local(). With that, we can use AC_SEARCH_LIBS, which I think is the more appropriate checking macro. It explicitly supports the possibility that no library may be necessary for the target symbol. I'm attaching an alternative patch using that approach.
It adds a new config.h define, HAVE_LIBUNWIND, and changes the code in signal_x86_64.c to test that. So, the code is only used if both the header and the symbol are available.
I've eliminated the notice. If libunwind is not available, I'm not sure that's a problem that the user/builder/packager needs to address. It will just go back to the situation before my commit, which was mostly fine. The only problem (that I'm aware of) was on OS X, where there's not really any question that libunwind will be available. If desired, we can add the notice back based on the variable ac_cv_search__Unwind_RaiseException.
Can you test on x86-64 Linux? Thanks again.
https://bugs.winehq.org/show_bug.cgi?id=38713
Ken Thomases ken@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #51676|0 |1 is obsolete| |
--- Comment #7 from Ken Thomases ken@codeweavers.com --- Created attachment 51677 --> https://bugs.winehq.org/attachment.cgi?id=51677 configure check for libunwind
Oops. Last attachment was a diff on top of Erich's. Fixed to be a self-contained patch.
https://bugs.winehq.org/show_bug.cgi?id=38713
Marcus Meissner marcus@jet.franken.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |marcus@jet.franken.de
https://bugs.winehq.org/show_bug.cgi?id=38713
Daniel Kamil Kozar dkk089@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |dkk089@gmail.com
--- Comment #8 from Daniel Kamil Kozar dkk089@gmail.com --- I got the error described here when compiling on Arch Linux x86-64, with libunwind installed as a separate package.
After applying the patch, the build completes successfully.
https://bugs.winehq.org/show_bug.cgi?id=38713
--- Comment #9 from Erich E. Hoover erich.e.hoover@wine-staging.com --- (In reply to Ken Thomases from comment #7)
Created attachment 51677 [details] configure check for libunwind
Oops. Last attachment was a diff on top of Erich's. Fixed to be a self-contained patch.
I assume that we actually want to use libunwind when it's available, unfortunately for me that's not working: checking libunwind.h usability... yes checking libunwind.h presence... yes checking for libunwind.h... yes checking for library containing _Unwind_RaiseException... none required
https://bugs.winehq.org/show_bug.cgi?id=38713
--- Comment #10 from Marcus Meissner marcus@jet.franken.de --- linux x86_64 libunwind does not contain this function _Unwind_RaiseException...
nm -D /usr/lib64/libunwind.so |grep -v x86
has these usable symbols: 00000000000027f0 T _U_dyn_cancel 0000000000002870 T _U_dyn_info_list_addr 0000000000002880 T _U_dyn_register 0000000000002720 T unw_backtrace
perhaps a AC_TRY_LINK() construct is needed.
https://bugs.winehq.org/show_bug.cgi?id=38713
Ken Thomases ken@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #51677|0 |1 is obsolete| |
--- Comment #11 from Ken Thomases ken@codeweavers.com --- Created attachment 51678 --> https://bugs.winehq.org/attachment.cgi?id=51678 configure check for libunwind using AC_LINK_IFELSE()
(In reply to Marcus Meissner from comment #10)
perhaps a AC_TRY_LINK() construct is needed.
Yeah, something like that. Wine does not currently use AC_TRY_LINK(), but it does use AC_LINK_IFELSE() which is similar. In fact, I already found a workalike for AC_SEARCH_LIBS in Wine's configure, for the search for libresolv, which uses AC_LINK_IFELSE(). So, I copied that and adapted it for libunwind.
Please try this new patch.
https://bugs.winehq.org/show_bug.cgi?id=38713
Sebastian Lackner sebastian@fds-team.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |sebastian@fds-team.de
--- Comment #12 from Sebastian Lackner sebastian@fds-team.de --- Created attachment 51679 --> https://bugs.winehq.org/attachment.cgi?id=51679 configure: Add libunwind configure flag.
I didn't have time to look at Kens last attempt, but here my own one. I think its also nice to offer a way to avoid linking against libunwind. Especially for 32-bit its a completely useless additional dependency.
https://bugs.winehq.org/show_bug.cgi?id=38713
--- Comment #13 from Ken Thomases ken@codeweavers.com --- It's true that it's (currently) only used for 64-bit, but I'm guessing it's not the first such architecture-specific dependency. I'm not sure it's worth making the check architecture-specific if there's any chance that it will be used for other architectures in the future (which I don't really know if it will).
Your patch is better than mine in that I forgot to #define UNW_LOCAL_ONLY for my check.
The loop construct in mine avoids the duplication of the test code in yours.
https://bugs.winehq.org/show_bug.cgi?id=38713
--- Comment #14 from Sebastian Lackner sebastian@fds-team.de --- (In reply to Ken Thomases from comment #13)
It's true that it's (currently) only used for 64-bit, but I'm guessing it's not the first such architecture-specific dependency. I'm not sure it's worth making the check architecture-specific if there's any chance that it will be used for other architectures in the future (which I don't really know if it will).
That might be true. But at least the configure flag should be added in my opinion, there are a lot of distros (Gentoo for example) which hate auto-magic linking against additional dependencies.
Your patch is better than mine in that I forgot to #define UNW_LOCAL_ONLY for my check.
The loop construct in mine avoids the duplication of the test code in yours.
I agree, feel free to create a merged version with the advantages from both attempts.
https://bugs.winehq.org/show_bug.cgi?id=38713
--- Comment #15 from Daniel Kamil Kozar dkk089@gmail.com --- (In reply to Ken Thomases from comment #13)
It's true that it's (currently) only used for 64-bit, but I'm guessing it's not the first such architecture-specific dependency. I'm not sure it's worth making the check architecture-specific if there's any chance that it will be used for other architectures in the future (which I don't really know if it will).
Your patch is better than mine in that I forgot to #define UNW_LOCAL_ONLY for my check.
The loop construct in mine avoids the duplication of the test code in yours.
UNW_LOCAL_ONLY is needed in this case. Your patch from comment #11 causes the linking of the test program to fail due to a missing symbol. My config.log says :
configure:8825: gcc -o conftest -O2 conftest.c -lunwind >&5 /tmp/cc297AN0.o: In function `main': conftest.c:(.text+0x20): undefined reference to `_Ux86_64_init_local' collect2: error: ld returned 1 exit status
(In reply to Sebastian Lackner from comment #12)
Created attachment 51679 [details] configure: Add libunwind configure flag.
I didn't have time to look at Kens last attempt, but here my own one. I think its also nice to offer a way to avoid linking against libunwind. Especially for 32-bit its a completely useless additional dependency.
This patch works okay, libunwind is correctly found and linked to ntdll.
https://bugs.winehq.org/show_bug.cgi?id=38713
--- Comment #16 from Marcus Meissner marcus@jet.franken.de --- Ken, I also had to add UNW_LOCAL_ONLY, then it worked as expected.
https://bugs.winehq.org/show_bug.cgi?id=38713
--- Comment #17 from Erich E. Hoover erich.e.hoover@wine-staging.com --- (In reply to Marcus Meissner from comment #10)
linux x86_64 libunwind does not contain this function _Unwind_RaiseException... ...
Must depend on the version: ehoover@lappy:~$ nm -D /usr/lib/x86_64-linux-gnu/libunwind.so | grep _Unwind_RaiseException 0000000000003770 T __libunwind_Unwind_RaiseException 0000000000003770 T _Unwind_RaiseException
https://bugs.winehq.org/show_bug.cgi?id=38713
--- Comment #18 from Ken Thomases ken@codeweavers.com --- Thanks, everybody, for the help and suggestions. I have submitted patches:
https://source.winehq.org/patches/data/112041 https://source.winehq.org/patches/data/112042 https://source.winehq.org/patches/data/112043
https://bugs.winehq.org/show_bug.cgi?id=38713
--- Comment #19 from Ken Thomases ken@codeweavers.com --- Based on Alexandre's response to my previous patches, I've submitted a new one which only uses libunwind on OS X:
http://source.winehq.org/patches/data/112077
https://bugs.winehq.org/show_bug.cgi?id=38713
Ken Thomases ken@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |36a9f9dd05c3b9df77c44c91663 | |e9bd6cae1c848 Status|NEW |RESOLVED Resolution|--- |FIXED
--- Comment #20 from Ken Thomases ken@codeweavers.com --- Fixed by http://source.winehq.org/git/wine.git/?a=commit;h=36a9f9dd05c3b9df77c44c91663e9bd6cae1c848.
https://bugs.winehq.org/show_bug.cgi?id=38713
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #21 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 1.7.46.
-
wine-bugs＠winehq.org