windows-gnullvm: always link libunwind statically - #160712
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
windows-gnullvm: always link libunwind statically try-job: *gnullvm
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@bors try jobs=*llvm-mingw |
This comment has been minimized.
This comment has been minimized.
windows-gnullvm: always link libunwind statically try-job: *llvm-mingw
This comment has been minimized.
This comment has been minimized.
501c282 to
aa3688d
Compare
This comment has been minimized.
This comment has been minimized.
aa3688d to
e2eb974
Compare
|
Some changes occurred in src/doc/rustc/src/platform-support cc @Noratrieb |
|
rustbot has assigned @petrochenkov. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
I'm not sure what kind of approval this change will require (r+, FCP, MCP+second?). |
|
Is it correct to link multiple copies of libunwind.a together when you link a dylib (with one copy of libunwind) into an executable (another copy of libunwind)? For non-gnullvm windows-gnu targets it's apparently not ok (unwinding across DLL boundaries breaks), that's why we have flags like |
That was in a zulip DM.
According to @mati865 for as long as 32bit libgcc_s isn't involved as unwinder, multiple unwinder copies will interact just fine with each other. In the case of -gnullvm, we already always use libunwind rather than libgcc_s as unwinder. |
Then my question is what are the downsides of supporting the static vs dynamic choice under an option like in #159782. |
|
Unlike on -gnu there is no breakage that would occur from not supporting it. It only increases the size of dylibs by 40KiB by adding a copy of libunwind, but in doing so makes distribution of the dylibs easier by not having to ship libunwind.dll as user. |
On Windows with 32-bit libgcc built with Dwarf-2 exception model? Not at all. [*] IIRC, on 32-bits libunwind will break if specific libgcc_{eh,s} symbols are present in the binary.
Just like Bjorn wrote, but remember that they no longer need to ship Decided to double-check if memory serves me right to avoid potentially misleading everyone. Heck, libgcc is even worse than I remembered (or it has regressed). 32-bit GCC preparation:Runing libgcc case: Don’t even get me started on what happened here, instead I'll move to something sane. 32-bit Clang preparationRunning libunwind case: |
|
Ok, thanks for the explanations! |
…r=petrochenkov
windows-gnullvm: always link libunwind statically
Previously shared library was used by default, meaning that programs and libraries couldn't be loaded if `libunwind.dll` was missing from the PATH. Using Wine (on Linux) because it better shows the problem (and is more convenient):
```
❯ cargo new hello &> /dev/null
❯ cargo rustc --target x86_64-pc-windows-gnullvm &> /dev/null
❯ wine target/x86_64-pc-windows-gnullvm/debug/hello.exe
0024:err:module:import_dll Library libunwind.dll (which is needed by L"Z:\\tmp\\hello\\target\\x86_64-pc-windows-gnullvm\\debug\\hello.exe") not found
0024:err:module:loader_init Importing dlls for L"Z:\\tmp\\hello\\target\\x86_64-pc-windows-gnullvm\\debug\\hello.exe" failed, status c0000135
❯ llvm-readobj --coff-imports target/x86_64-pc-windows-gnullvm/debug/hello.exe
...
Import {
Name: libunwind.dll
ImportLookupTableRVA: 0x3D308
ImportAddressTableRVA: 0x3D678
Symbol: _GCC_specific_handler (0)
Symbol: _Unwind_DeleteException (0)
Symbol: _Unwind_GetDataRelBase (0)
Symbol: _Unwind_GetIPInfo (0)
Symbol: _Unwind_GetLanguageSpecificData (0)
Symbol: _Unwind_GetRegionStart (0)
Symbol: _Unwind_GetTextRelBase (0)
Symbol: _Unwind_RaiseException (0)
Symbol: _Unwind_Resume (0)
Symbol: _Unwind_SetGR (0)
Symbol: _Unwind_SetIP (0)
}
...
```
Optionally libunwind could be linked statically via `+crt-static`:
```
❯ cargo rustc --target x86_64-pc-windows-gnullvm -- -C target-feature=+crt-static &> /dev/null
❯ wine target/x86_64-pc-windows-gnullvm/debug/hello.exe
Hello, world!
❯ llvm-readobj --coff-imports target/x86_64-pc-windows-gnullvm/debug/hello.exe | rg 'libunwind.dll' || echo "doesn't depend on shared libunwind"
doesn't depend on shared libunwind
```
After a discussion of approach in rust-lang#159782 with @bjorn3 (thanks BTW!), I changed the proposed approach to always link static libunwind.
I don't have a good solution for rust-lang#121794 that will resurface. I guess the user has three options:
- symlink `libunwind.dll.a` as `libunwind.a`
- add `--unwindlib=none -lunwind` to the linker args
- create linker wrapper
- use self-contained mode which is likely is undesirable
I think the ease of use (not having to deal with additional DLL dependency) outweights the benefit of working with incomplete C toolchain.
The size bloat is also not a problem, sizes (in bytes) of the binary for the literal hello world project:
- debug build:
- shared libunwind 4194816
- static libunwind 4323328
- release build:
- shared libunwind 382464
- static libunwind 423424
Debug diff +125.5 KiB, release diff: +40 KiB.
Size of `libunwind.dll` that has to be provided when linking shared libunwind: 204288 bytes (199.5 KiB).
…uwer Rollup of 6 pull requests Successful merges: - #162309 (offload: automate manual clang-linker-wrapper step) - #160505 (delegation: supporting inherent impls) - #160712 (windows-gnullvm: always link libunwind statically) - #161423 (trait_selection: Keep type-op region constraints in borrowck) - #162461 (limit the api of `fold_predicate` and `visit_predicate`) - #162475 (Fix unsoundness bug on next trait solver for dyn const generics placeholder)
Rollup merge of #160712 - mati865:gnullvm-static-libunwind, r=petrochenkov windows-gnullvm: always link libunwind statically Previously shared library was used by default, meaning that programs and libraries couldn't be loaded if `libunwind.dll` was missing from the PATH. Using Wine (on Linux) because it better shows the problem (and is more convenient): ``` ❯ cargo new hello &> /dev/null ❯ cargo rustc --target x86_64-pc-windows-gnullvm &> /dev/null ❯ wine target/x86_64-pc-windows-gnullvm/debug/hello.exe 0024:err:module:import_dll Library libunwind.dll (which is needed by L"Z:\\tmp\\hello\\target\\x86_64-pc-windows-gnullvm\\debug\\hello.exe") not found 0024:err:module:loader_init Importing dlls for L"Z:\\tmp\\hello\\target\\x86_64-pc-windows-gnullvm\\debug\\hello.exe" failed, status c0000135 ❯ llvm-readobj --coff-imports target/x86_64-pc-windows-gnullvm/debug/hello.exe ... Import { Name: libunwind.dll ImportLookupTableRVA: 0x3D308 ImportAddressTableRVA: 0x3D678 Symbol: _GCC_specific_handler (0) Symbol: _Unwind_DeleteException (0) Symbol: _Unwind_GetDataRelBase (0) Symbol: _Unwind_GetIPInfo (0) Symbol: _Unwind_GetLanguageSpecificData (0) Symbol: _Unwind_GetRegionStart (0) Symbol: _Unwind_GetTextRelBase (0) Symbol: _Unwind_RaiseException (0) Symbol: _Unwind_Resume (0) Symbol: _Unwind_SetGR (0) Symbol: _Unwind_SetIP (0) } ... ``` Optionally libunwind could be linked statically via `+crt-static`: ``` ❯ cargo rustc --target x86_64-pc-windows-gnullvm -- -C target-feature=+crt-static &> /dev/null ❯ wine target/x86_64-pc-windows-gnullvm/debug/hello.exe Hello, world! ❯ llvm-readobj --coff-imports target/x86_64-pc-windows-gnullvm/debug/hello.exe | rg 'libunwind.dll' || echo "doesn't depend on shared libunwind" doesn't depend on shared libunwind ``` After a discussion of approach in #159782 with @bjorn3 (thanks BTW!), I changed the proposed approach to always link static libunwind. I don't have a good solution for #121794 that will resurface. I guess the user has three options: - symlink `libunwind.dll.a` as `libunwind.a` - add `--unwindlib=none -lunwind` to the linker args - create linker wrapper - use self-contained mode which is likely is undesirable I think the ease of use (not having to deal with additional DLL dependency) outweights the benefit of working with incomplete C toolchain. The size bloat is also not a problem, sizes (in bytes) of the binary for the literal hello world project: - debug build: - shared libunwind 4194816 - static libunwind 4323328 - release build: - shared libunwind 382464 - static libunwind 423424 Debug diff +125.5 KiB, release diff: +40 KiB. Size of `libunwind.dll` that has to be provided when linking shared libunwind: 204288 bytes (199.5 KiB).
View all comments
Previously shared library was used by default, meaning that programs and libraries couldn't be loaded if
libunwind.dllwas missing from the PATH. Using Wine (on Linux) because it better shows the problem (and is more convenient):Optionally libunwind could be linked statically via
+crt-static:After a discussion of approach in #159782 with @bjorn3 (thanks BTW!), I changed the proposed approach to always link static libunwind.
I don't have a good solution for #121794 that will resurface. I guess the user has three options:
libunwind.dll.aaslibunwind.a--unwindlib=none -lunwindto the linker argsI think the ease of use (not having to deal with additional DLL dependency) outweights the benefit of working with incomplete C toolchain.
The size bloat is also not a problem, sizes (in bytes) of the binary for the literal hello world project:
Debug diff +125.5 KiB, release diff: +40 KiB.
Size of
libunwind.dllthat has to be provided when linking shared libunwind: 204288 bytes (199.5 KiB).