Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set "symbol name" in raw-dylib import libraries to the decorated name #130586

Merged
merged 1 commit into from
Nov 8, 2024

Conversation

dpaoliello
Copy link
Contributor

@dpaoliello dpaoliello commented Sep 19, 2024

windows-rs received a bug report that mixing raw-dylib generated and the Windows SDK import libraries was causing linker failures: microsoft/windows-rs#3285

The root cause turned out to be #124958, that is we are not including the decorated name in the import library and so the import name type is also not being correctly set.

This change modifies the generation of import libraries to set the "symbol name" to the fully decorated name and correctly marks the import as being data vs function.

Note that this also required some changes to how the symbol is named within Rust: for MSVC we now need to use the decorated name but for MinGW we still need to use partially decorated (or undecorated) name.

Fixes #124958

Passing i686 MSVC and MinGW build: https://github.com/rust-lang/rust/actions/runs/11000433888?pr=130586

r? @ChrisDenton

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Sep 19, 2024
@rust-log-analyzer

This comment has been minimized.

@dpaoliello dpaoliello changed the title Test fixing raw-dylib Set "symbol name" in raw-dylib import libraries to the decorated name Sep 23, 2024
@dpaoliello dpaoliello marked this pull request as ready for review September 23, 2024 23:53
@rustbot
Copy link
Collaborator

rustbot commented Sep 23, 2024

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@dpaoliello
Copy link
Contributor Author

Ping @ChrisDenton @bjorn3

@ChrisDenton
Copy link
Member

Sorry, just catching up with my review queue. This looks right to me but this should really have a reviewer like r? cjgillot (picking at random, feel free to reassign!).

@rustbot rustbot assigned cjgillot and unassigned ChrisDenton Oct 16, 2024
@cjgillot
Copy link
Contributor

I have no knowledge of how windows works.
r? compiler
@wesleywiser ?

@rustbot rustbot assigned chenyukang and unassigned cjgillot Oct 27, 2024
@chenyukang
Copy link
Member

r? compiler

@rustbot rustbot assigned davidtwco and unassigned chenyukang Oct 28, 2024
@davidtwco
Copy link
Member

davidtwco commented Oct 31, 2024

Apologies for just passing this on again, but this requires Windows expertise
r? @wesleywiser

@rustbot rustbot assigned wesleywiser and unassigned davidtwco Oct 31, 2024
@jieyouxu
Copy link
Member

cc @bjorn3 or @petrochenkov maybe one of you know more about this? This has to do with windows import lib symbols which is uhh tricky.

@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 7, 2024

📌 Commit b2fd8a0 has been approved by wesleywiser

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 7, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 7, 2024
Set "symbol name" in raw-dylib import libraries to the decorated name

`windows-rs` received a bug report that mixing raw-dylib generated and the Windows SDK import libraries was causing linker failures: <microsoft/windows-rs#3285>

The root cause turned out to be rust-lang#124958, that is we are not including the decorated name in the import library and so the import name type is also not being correctly set.

This change modifies the generation of import libraries to set the "symbol name" to the fully decorated name and correctly marks the import as being data vs function.

Note that this also required some changes to how the symbol is named within Rust: for MSVC we now need to use the decorated name but for MinGW we still need to use partially decorated (or undecorated) name.

Fixes rust-lang#124958

Passing i686 MSVC and MinGW build: <https://github.com/rust-lang/rust/actions/runs/11000433888?pr=130586>

r? `@ChrisDenton`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 7, 2024
Set "symbol name" in raw-dylib import libraries to the decorated name

`windows-rs` received a bug report that mixing raw-dylib generated and the Windows SDK import libraries was causing linker failures: <microsoft/windows-rs#3285>

The root cause turned out to be rust-lang#124958, that is we are not including the decorated name in the import library and so the import name type is also not being correctly set.

This change modifies the generation of import libraries to set the "symbol name" to the fully decorated name and correctly marks the import as being data vs function.

Note that this also required some changes to how the symbol is named within Rust: for MSVC we now need to use the decorated name but for MinGW we still need to use partially decorated (or undecorated) name.

Fixes rust-lang#124958

Passing i686 MSVC and MinGW build: <https://github.com/rust-lang/rust/actions/runs/11000433888?pr=130586>

r? ``@ChrisDenton``
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 7, 2024
…llaumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#130586 (Set "symbol name" in raw-dylib import libraries to the decorated name)
 - rust-lang#131913 (Add `{ignore,needs}-{rustc,std}-debug-assertions` directive support)
 - rust-lang#132095 (Fix rust-lang#131977 parens mangled in shared mut static lint suggestion)
 - rust-lang#132131 ([StableMIR] API to retrieve definitions from crates)
 - rust-lang#132696 (Compile `test_num_f128` conditionally on `reliable_f128_math` config)
 - rust-lang#132738 (Initialize channel `Block`s directly on the heap)
 - rust-lang#132739 (Fix `librustdoc/scrape_examples.rs` formatting)
 - rust-lang#132740 (Update test for LLVM 20's new vector splat syntax)

r? `@ghost`
`@rustbot` modify labels: rollup
@workingjubilee
Copy link
Member

vibe-based triaging:

@bors rollup=iffy

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 8, 2024
…kingjubilee

Rollup of 10 pull requests

Successful merges:

 - rust-lang#130586 (Set "symbol name" in raw-dylib import libraries to the decorated name)
 - rust-lang#131913 (Add `{ignore,needs}-{rustc,std}-debug-assertions` directive support)
 - rust-lang#132095 (Fix rust-lang#131977 parens mangled in shared mut static lint suggestion)
 - rust-lang#132131 ([StableMIR] API to retrieve definitions from crates)
 - rust-lang#132639 (core: move intrinsics.rs into intrinsics folder)
 - rust-lang#132696 (Compile `test_num_f128` conditionally on `reliable_f128_math` config)
 - rust-lang#132737 (bootstrap: Print better message if lock pid isn't available)
 - rust-lang#132739 (Fix `librustdoc/scrape_examples.rs` formatting)
 - rust-lang#132740 (Update test for LLVM 20's new vector splat syntax)
 - rust-lang#132741 (Update mips64 data layout to match LLVM 20 change)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 60e8ab6 into rust-lang:master Nov 8, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 8, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 8, 2024
Rollup merge of rust-lang#130586 - dpaoliello:fixrawdylib, r=wesleywiser

Set "symbol name" in raw-dylib import libraries to the decorated name

`windows-rs` received a bug report that mixing raw-dylib generated and the Windows SDK import libraries was causing linker failures: <microsoft/windows-rs#3285>

The root cause turned out to be rust-lang#124958, that is we are not including the decorated name in the import library and so the import name type is also not being correctly set.

This change modifies the generation of import libraries to set the "symbol name" to the fully decorated name and correctly marks the import as being data vs function.

Note that this also required some changes to how the symbol is named within Rust: for MSVC we now need to use the decorated name but for MinGW we still need to use partially decorated (or undecorated) name.

Fixes rust-lang#124958

Passing i686 MSVC and MinGW build: <https://github.com/rust-lang/rust/actions/runs/11000433888?pr=130586>

r? `@ChrisDenton`
mati865 pushed a commit to mati865/rust that referenced this pull request Nov 12, 2024
Set "symbol name" in raw-dylib import libraries to the decorated name

`windows-rs` received a bug report that mixing raw-dylib generated and the Windows SDK import libraries was causing linker failures: <microsoft/windows-rs#3285>

The root cause turned out to be rust-lang#124958, that is we are not including the decorated name in the import library and so the import name type is also not being correctly set.

This change modifies the generation of import libraries to set the "symbol name" to the fully decorated name and correctly marks the import as being data vs function.

Note that this also required some changes to how the symbol is named within Rust: for MSVC we now need to use the decorated name but for MinGW we still need to use partially decorated (or undecorated) name.

Fixes rust-lang#124958

Passing i686 MSVC and MinGW build: <https://github.com/rust-lang/rust/actions/runs/11000433888?pr=130586>

r? `@ChrisDenton`
mati865 pushed a commit to mati865/rust that referenced this pull request Nov 12, 2024
…kingjubilee

Rollup of 10 pull requests

Successful merges:

 - rust-lang#130586 (Set "symbol name" in raw-dylib import libraries to the decorated name)
 - rust-lang#131913 (Add `{ignore,needs}-{rustc,std}-debug-assertions` directive support)
 - rust-lang#132095 (Fix rust-lang#131977 parens mangled in shared mut static lint suggestion)
 - rust-lang#132131 ([StableMIR] API to retrieve definitions from crates)
 - rust-lang#132639 (core: move intrinsics.rs into intrinsics folder)
 - rust-lang#132696 (Compile `test_num_f128` conditionally on `reliable_f128_math` config)
 - rust-lang#132737 (bootstrap: Print better message if lock pid isn't available)
 - rust-lang#132739 (Fix `librustdoc/scrape_examples.rs` formatting)
 - rust-lang#132740 (Update test for LLVM 20's new vector splat syntax)
 - rust-lang#132741 (Update mips64 data layout to match LLVM 20 change)

r? `@ghost`
`@rustbot` modify labels: rollup
@jieyouxu jieyouxu added CI-spurious-fail-mingw CI spurious failure: target env mingw and removed CI-spurious-fail-mingw CI spurious failure: target env mingw labels Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

in Windows x86, the symbol name generated by raw-dylib+undecorated are not as expected