On macOS unixlibs that link to ntdll.so currently fail to load. It appears the macOS loader needs to be able to locate the file even if the library is already loaded.
This patch changes the LC_ID_DYLIB name of ntdll.so to "@rpath/ntdll.so" so that when other unixlibs link against this they will insert that name into their LC_LOAD_DYLIB entry for ntdll.so.
The patch also adds LC_RPATH entries for both @loader_path/ (running from installed tree) and @loader_path/../ntdll/ (running from build tree).
Although it's only required to explicitly change ntdll.so's LC_ID_DYLIB and to only add LC_RPATHs to unixlibs that load ntdll.so, it seems simpler to do the same for all unixlibs, which is what this patch does.
Signed-off-by: Huw Davies huw@codeweavers.com --- I'm not exactly happy with this patch, so any other ideas would be appreciated. tools/winegcc/winegcc.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/tools/winegcc/winegcc.c b/tools/winegcc/winegcc.c index 426336e4536..0a72e6fbe49 100644 --- a/tools/winegcc/winegcc.c +++ b/tools/winegcc/winegcc.c @@ -492,6 +492,13 @@ static strarray *get_link_args( struct options *opts, const char *output_name ) strarray_add( flags, opts->image_base ); } if (opts->strip) strarray_add( flags, "-Wl,-x" ); + if (opts->unix_lib) + { + strarray_add( flags, "-install_name" ); + strarray_add( flags, strmake( "@rpath/%s.so", output_name ) ); + strarray_add( flags, "-Wl,-rpath,@loader_path/" ); + strarray_add( flags, "-Wl,-rpath,@loader_path/../ntdll/" ); + } strarray_addall( link_args, flags ); return link_args;
This seems strange to me as I don't get this issue. Could this be related to a newer version of Apple cctools/Xcode?, could you give an example of what you attempt to run and get an issue with not finding ntdll.so?
The following make sense to keep for general usage on macOS, this would save the macOS specific step of fixing the libraries install_name.
diff --git a/tools/winegcc/winegcc.c b/tools/winegcc/winegcc.c index 426336e4536..0a72e6fbe49 100644 --- a/tools/winegcc/winegcc.c +++ b/tools/winegcc/winegcc.c @@ -492,6 +492,11 @@ static strarray *get_link_args( struct options *opts, const char *output_name ) strarray_add( flags, opts->image_base ); } if (opts->strip) strarray_add( flags, "-Wl,-x" ); + if (opts->unix_lib) + { + strarray_add( flags, "-install_name" ); + strarray_add( flags, strmake( "@rpath/%s.so", output_name ) ); + } strarray_addall( link_args, flags ); return link_args;
For the injecting LDFLAGS it would be better to add for example UNIX_LDFLAGS so any specific linker flags can be passed only for the unix libraries.
On Fri, Aug 20, 2021 at 8:23 AM Huw Davies huw@codeweavers.com wrote:
On macOS unixlibs that link to ntdll.so currently fail to load. It appears the macOS loader needs to be able to locate the file even if the library is already loaded.
This patch changes the LC_ID_DYLIB name of ntdll.so to "@rpath/ntdll.so" so that when other unixlibs link against this they will insert that name into their LC_LOAD_DYLIB entry for ntdll.so.
The patch also adds LC_RPATH entries for both @loader_path/ (running from installed tree) and @loader_path/../ntdll/ (running from build tree).
Although it's only required to explicitly change ntdll.so's LC_ID_DYLIB and to only add LC_RPATHs to unixlibs that load ntdll.so, it seems simpler to do the same for all unixlibs, which is what this patch does.
Signed-off-by: Huw Davies huw@codeweavers.com
I'm not exactly happy with this patch, so any other ideas would be appreciated. tools/winegcc/winegcc.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/tools/winegcc/winegcc.c b/tools/winegcc/winegcc.c index 426336e4536..0a72e6fbe49 100644 --- a/tools/winegcc/winegcc.c +++ b/tools/winegcc/winegcc.c @@ -492,6 +492,13 @@ static strarray *get_link_args( struct options *opts, const char *output_name ) strarray_add( flags, opts->image_base ); } if (opts->strip) strarray_add( flags, "-Wl,-x" );
if (opts->unix_lib){strarray_add( flags, "-install_name" );strarray_add( flags, strmake( "@rpath/%s.so", output_name ) );strarray_add( flags, "-Wl,-rpath,@loader_path/" );strarray_add( flags, "-Wl,-rpath,@loader_path/../ntdll/" );} strarray_addall( link_args, flags ); return link_args;-- 2.23.0
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=96011
Your paranoid android.
=== debiant2 (build log) ===
error: corrupt patch at line 15 Task: Patch failed to apply
=== debiant2 (build log) ===
error: corrupt patch at line 15 Task: Patch failed to apply
On Fri, Aug 20, 2021 at 09:45:17AM -0400, Dean Greer wrote:
This seems strange to me as I don't get this issue. Could this be related to a newer version of Apple cctools/Xcode?, could you give an example of what you attempt to run and get an issue with not finding ntdll.so?
This is with the command-line tools from Xcode 9 (32-bit) on Mojave.
Simply running "./wine notepad" with the current git from the build tree produces messages like: 0040:err:rpc:rpcrt4_ncacn_np_handoff Failed to retrieve the computer name, error 2 which is due to the ws2_32.so unixlib being unable to load ntdll.so.
% otool -l dlls/ntdll/ntdll.so [...] cmd LC_ID_DYLIB cmdsize 44 name dlls/ntdll/ntdll.so (offset 24) [...]
% otool -l dlls/ws2_32/ws2_32.so [...] cmd LC_LOAD_DYLIB cmdsize 44 name dlls/ntdll/ntdll.so (offset 24) [...]
Huw.
This makes sense now thank you, I’d still rather see the -rpath behind something like UNIX_LDFLAGS then it could be used for other -rpath configuration for the Unix.so libraries
For the build directory runs wouldn’t it be better to alter the wine loader wrapper script.
On Fri, Aug 20, 2021 at 10:02 AM Huw Davies huw@codeweavers.com wrote:
On Fri, Aug 20, 2021 at 09:45:17AM -0400, Dean Greer wrote:
This seems strange to me as I don't get this issue. Could this be
related to a
newer version of Apple cctools/Xcode?, could you give an example of what
you
attempt to run and get an issue with not finding ntdll.so?
This is with the command-line tools from Xcode 9 (32-bit) on Mojave.
Simply running "./wine notepad" with the current git from the build tree produces messages like: 0040:err:rpc:rpcrt4_ncacn_np_handoff Failed to retrieve the computer name, error 2 which is due to the ws2_32.so unixlib being unable to load ntdll.so.
% otool -l dlls/ntdll/ntdll.so [...] cmd LC_ID_DYLIB cmdsize 44 name dlls/ntdll/ntdll.so (offset 24) [...]
% otool -l dlls/ws2_32/ws2_32.so [...] cmd LC_LOAD_DYLIB cmdsize 44 name dlls/ntdll/ntdll.so (offset 24) [...]
Huw.
On Fri, Aug 20, 2021 at 10:13:01AM -0400, Dean Greer wrote:
This makes sense now thank you, I’d still rather see the -rpath behind something like UNIX_LDFLAGS then it could be used for other -rpath configuration for the Unix.so libraries
For the build directory runs wouldn’t it be better to alter the wine loader wrapper script.
Right, good idea. Adding $topdir/dlls/ntdll to DYLD_LIBRARY_PATH in the wine script will cover the build directory case.
So that leaves the installed case. The simplest approach would be to change the install-name to @loader_path/module.so and skip rpaths entirely. Is there a reason not to do that?
Huw.
That sounds good to me.
The reason why I’m requesting to not always inject -rpath in the manner is due to Bug 49199 to workaround this also requires adding some -rpath configurations directly into LDFLAGS but that’s ugly.
Instead having UNIX_LDFLAGS for example on macOS default to passing the needed -rpath,@loader_path/ and allows adding additional paths after this.
On Fri, Aug 20, 2021 at 11:01 AM Huw Davies huw@codeweavers.com wrote:
On Fri, Aug 20, 2021 at 10:13:01AM -0400, Dean Greer wrote:
This makes sense now thank you, I’d still rather see the -rpath behind something like UNIX_LDFLAGS then it could be used for other -rpath configuration for the Unix.so libraries
For the build directory runs wouldn’t it be better to alter the wine
loader
wrapper script.
Right, good idea. Adding $topdir/dlls/ntdll to DYLD_LIBRARY_PATH in the wine script will cover the build directory case.
So that leaves the installed case. The simplest approach would be to change the install-name to @loader_path/module.so and skip rpaths entirely. Is there a reason not to do that?
Huw.
To add to this I believe libraries by default look within there own directory so for this GOG might not even need to care about configuring -rpath
On Fri, Aug 20, 2021 at 11:05 AM Dean Greer gcenx83@gmail.com wrote:
That sounds good to me.
The reason why I’m requesting to not always inject -rpath in the manner is due to Bug 49199 to workaround this also requires adding some -rpath configurations directly into LDFLAGS but that’s ugly.
Instead having UNIX_LDFLAGS for example on macOS default to passing the needed -rpath,@loader_path/ and allows adding additional paths after this.
On Fri, Aug 20, 2021 at 11:01 AM Huw Davies huw@codeweavers.com wrote:
On Fri, Aug 20, 2021 at 10:13:01AM -0400, Dean Greer wrote:
This makes sense now thank you, I’d still rather see the -rpath behind something like UNIX_LDFLAGS then it could be used for other -rpath configuration for the Unix.so libraries
For the build directory runs wouldn’t it be better to alter the wine
loader
wrapper script.
Right, good idea. Adding $topdir/dlls/ntdll to DYLD_LIBRARY_PATH in the wine script will cover the build directory case.
So that leaves the installed case. The simplest approach would be to change the install-name to @loader_path/module.so and skip rpaths entirely. Is there a reason not to do that?
Huw.
iPhone correcting non typos sign…
On macOS by default libraries should check within there own directory then checkin other default paths, when additional paths are added those will be checked before standard locations like /usr/local/lib and /usr/lib etc
On Fri, Aug 20, 2021 at 11:14 AM Dean Greer gcenx83@gmail.com wrote:
To add to this I believe libraries by default look within there own directory so for this GOG might not even need to care about configuring -rpath
On Fri, Aug 20, 2021 at 11:05 AM Dean Greer gcenx83@gmail.com wrote:
That sounds good to me.
The reason why I’m requesting to not always inject -rpath in the manner is due to Bug 49199 to workaround this also requires adding some -rpath configurations directly into LDFLAGS but that’s ugly.
Instead having UNIX_LDFLAGS for example on macOS default to passing the needed -rpath,@loader_path/ and allows adding additional paths after this.
On Fri, Aug 20, 2021 at 11:01 AM Huw Davies huw@codeweavers.com wrote:
On Fri, Aug 20, 2021 at 10:13:01AM -0400, Dean Greer wrote:
This makes sense now thank you, I’d still rather see the -rpath behind something like UNIX_LDFLAGS then it could be used for other -rpath configuration for the Unix.so libraries
For the build directory runs wouldn’t it be better to alter the wine
loader
wrapper script.
Right, good idea. Adding $topdir/dlls/ntdll to DYLD_LIBRARY_PATH in the wine script will cover the build directory case.
So that leaves the installed case. The simplest approach would be to change the install-name to @loader_path/module.so and skip rpaths entirely. Is there a reason not to do that?
Huw.
On Fri, Aug 20, 2021 at 11:17:00AM -0400, Dean Greer wrote:
iPhone correcting non typos sign…
On macOS by default libraries should check within there own directory then checkin other default paths, when additional paths are added those will be checked before standard locations like /usr/local/lib and /usr/lib etc
I'm not sure this is correct, we definitely need the @loader_path/ prefix: setting LC_LOAD_DYLIB simply to ntdll.so results in the library failing to load.
Huw.
That’s strange as this should work maybe only dylibs do this correctly.. this is Apple.
Could you instead implement adding this in the manner I explain above instead so I can be used in a more general way?, aka UNIX_LDFLAGS that by default add this needed -rpath,@loader_path/ but also allows adding additional flags as needed.
On Fri, Aug 20, 2021 at 11:40 AM Huw Davies huw@codeweavers.com wrote:
On Fri, Aug 20, 2021 at 11:17:00AM -0400, Dean Greer wrote:
iPhone correcting non typos sign…
On macOS by default libraries should check within there own directory
then
checkin other default paths, when additional paths are added those will
be
checked before standard locations like /usr/local/lib and /usr/lib etc
I'm not sure this is correct, we definitely need the @loader_path/ prefix: setting LC_LOAD_DYLIB simply to ntdll.so results in the library failing to load.
Huw.
On Fri, Aug 20, 2021 at 11:45:24AM -0400, Dean Greer wrote:
That’s strange as this should work maybe only dylibs do this correctly.. this is Apple.
Could you instead implement adding this in the manner I explain above instead so I can be used in a more general way?, aka UNIX_LDFLAGS that by default add this needed -rpath,@loader_path/ but also allows adding additional flags as needed.
I'm not sure what your issue is with using LDFLAGS - they do only affect unix libraries. There shouldn't be an issue with passing an rpath in that.
Huw.
The only tweak I’d propose is adding -Wl,-headerpad_max_install_names
diff --git a/tools/winegcc/winegcc.c b/tools/winegcc/winegcc.c index 426336e4536..0a72e6fbe49 100644 --- a/tools/winegcc/winegcc.c +++ b/tools/winegcc/winegcc.c @@ -492,6 +492,13 @@ static strarray *get_link_args( struct options *opts, const char *output_name ) strarray_add( flags, opts->image_base ); } if (opts->strip) strarray_add( flags, "-Wl,-x" ); + if (opts->unix_lib) + { + strarray_add( flags, "-install_name" ); + strarray_add( flags, strmake( "@rpath/%s.so", output_name ) ); + strarray_add( flags, "-Wl,-headerpad_max_install_names" ); + strarray_add( flags, "-Wl,-rpath,@loader_path/" ); + } strarray_addall( link_args, flags ); return link_args;
On Fri, Aug 20, 2021 at 3:36 PM Huw Davies huw@codeweavers.com wrote:
On Fri, Aug 20, 2021 at 11:45:24AM -0400, Dean Greer wrote:
That’s strange as this should work maybe only dylibs do this correctly..
this
is Apple.
Could you instead implement adding this in the manner I explain above
instead
so I can be used in a more general way?, aka UNIX_LDFLAGS that by
default add
this needed -rpath,@loader_path/ but also allows adding additional flags
as
needed.
I'm not sure what your issue is with using LDFLAGS - they do only affect unix libraries. There shouldn't be an issue with passing an rpath in that.
Huw.
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=96028
Your paranoid android.
=== debiant2 (build log) ===
error: corrupt patch at line 9 Task: Patch failed to apply
=== debiant2 (build log) ===
error: corrupt patch at line 9 Task: Patch failed to apply
diff --git a/tools/winegcc/winegcc.c b/tools/winegcc/winegcc.c index 426336e4536..0a72e6fbe49 100644 --- a/tools/winegcc/winegcc.c +++ b/tools/winegcc/winegcc.c @@ -492,6 +492,13 @@ static strarray *get_link_args( struct options *opts, const char *output_name ) strarray_add( flags, opts->image_base ); } if (opts->strip) strarray_add( flags, "-Wl,-x" ); + if (opts->unix_lib) + { + strarray_add( flags, "-install_name" ); + strarray_add( flags, strmake( "@rpath/%s.so", output_name ) ); + strarray_add( flags, "-Wl,-headerpad_max_inatall_names" ); + strarray_add( flags, "-Wl,-rpath,@loader_path/" ); + } strarray_addall( link_args, flags ); return link_args;
On Fri, Aug 20, 2021 at 3:57 PM Dean Greer gcenx83@gmail.com wrote:
The only tweak I’d propose is adding -Wl,-headerpad_max_install_names
diff --git a/tools/winegcc/winegcc.c b/tools/winegcc/winegcc.c index 426336e4536..0a72e6fbe49 100644 --- a/tools/winegcc/winegcc.c +++ b/tools/winegcc/winegcc.c @@ -492,6 +492,13 @@ static strarray *get_link_args( struct options *opts, const char *output_name ) strarray_add( flags, opts->image_base ); } if (opts->strip) strarray_add( flags, "-Wl,-x" );
if (opts->unix_lib){strarray_add( flags, "-install_name" );strarray_add( flags, strmake( "@rpath/%s.so", output_name ) );strarray_add( flags, "-Wl,-headerpad_max_install_names" );strarray_add( flags, "-Wl,-rpath,@loader_path/" );} strarray_addall( link_args, flags ); return link_args;On Fri, Aug 20, 2021 at 3:36 PM Huw Davies huw@codeweavers.com wrote:
On Fri, Aug 20, 2021 at 11:45:24AM -0400, Dean Greer wrote:
That’s strange as this should work maybe only dylibs do this
correctly.. this
is Apple.
Could you instead implement adding this in the manner I explain above
instead
so I can be used in a more general way?, aka UNIX_LDFLAGS that by
default add
this needed -rpath,@loader_path/ but also allows adding additional
flags as
needed.
I'm not sure what your issue is with using LDFLAGS - they do only affect unix libraries. There shouldn't be an issue with passing an rpath in that.
Huw.
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=96029
Your paranoid android.
=== debiant2 (build log) ===
error: corrupt patch at line 6 Task: Patch failed to apply
=== debiant2 (build log) ===
error: corrupt patch at line 6 Task: Patch failed to apply
On Fri, Aug 20, 2021 at 03:57:08PM -0400, Dean Greer wrote:
The only tweak I’d propose is adding -Wl,-headerpad_max_install_names
That doesn't seem like the sort of thing that should go in by default. If you really need it you can add it via LDFLAGS.
Huw.
That’s something I’m already setting by default and causes no harm. Regardless that's not majorly important to hold up this patch.
On Mon, Aug 23, 2021 at 2:31 AM Huw Davies huw@codeweavers.com wrote:
On Fri, Aug 20, 2021 at 03:57:08PM -0400, Dean Greer wrote:
The only tweak I’d propose is adding -Wl,-headerpad_max_install_names
That doesn't seem like the sort of thing that should go in by default. If you really need it you can add it via LDFLAGS.
Huw.
Sorry for what’s frankly spam now I wasn’t finished explain.
So that leaves the installed case. The simplest approach would be to
change the install-name to @loader_path/module.so and skip rpaths entirely. Is there a reason not to do that?
This should be fine going off how macOS usually handles loading libraries.
On Fri, Aug 20, 2021 at 11:01 AM Huw Davies huw@codeweavers.com wrote:
On Fri, Aug 20, 2021 at 10:13:01AM -0400, Dean Greer wrote:
This makes sense now thank you, I’d still rather see the -rpath behind something like UNIX_LDFLAGS then it could be used for other -rpath configuration for the Unix.so libraries
For the build directory runs wouldn’t it be better to alter the wine
loader
wrapper script.
Right, good idea. Adding $topdir/dlls/ntdll to DYLD_LIBRARY_PATH in the wine script will cover the build directory case.
So that leaves the installed case. The simplest approach would be to change the install-name to @loader_path/module.so and skip rpaths entirely. Is there a reason not to do that?
Huw.
-
Dean Greer -
Huw Davies -
Marvin