Skip to content

Conversation

gbaraldi
Copy link
Member

I'm slightly worried about using the exe handle here. I wonder I should instead do a specific dlopen of libunwind.

@gbaraldi gbaraldi requested review from topolarity and xal-0 August 13, 2025 17:06
@topolarity
Copy link
Member

I'm slightly worried about using the exe handle here. I wonder I should instead do a specific dlopen of libunwind.

Yeah, I don't think this will work as-is on Windows.

@topolarity topolarity changed the title Make libunwind more compatible if we don't load our own copy Avoid depending on Julia-patched libunwind ABI Aug 13, 2025
@gbaraldi
Copy link
Member Author

This already doesn't work on windows (it's behind an ifdef)

@xal-0
Copy link
Member

xal-0 commented Aug 13, 2025

With #58815, using the exe handle may or may not be the right choice depending on the intended behaviour. If we specifically want a version of libuwind depended on by libjulia-internal, we'd need no_deps=0 on the call to jl_dlsym. If we want to search for system libraries using the default search order, we should use jl_RTLD_DEFAULT_handle.

@gbaraldi
Copy link
Member Author

Well, I guess we would want to fallback to whatever libjulia-internal thinks libunwind is

@gbaraldi
Copy link
Member Author

Should this be changed when you dlfind changes?

@xal-0
Copy link
Member

xal-0 commented Aug 14, 2025

Should this be changed when you dlfind changes?

Yes, but it'll just require adding a 0 argument, which I will handle when merging.

@gbaraldi gbaraldi force-pushed the gb/libunwindcompat branch from 8b6e987 to ae5a7bb Compare August 15, 2025 17:29
@topolarity topolarity merged commit 63022fe into master Aug 21, 2025
7 checks passed
@topolarity topolarity deleted the gb/libunwindcompat branch August 21, 2025 00:08
@topolarity
Copy link
Member

@gbaraldi did you want this backported to 1.12?

@gbaraldi gbaraldi added the backport 1.12 Change should be backported to release-1.12 label Aug 22, 2025
This was referenced Sep 24, 2025
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Oct 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants