Skip to content

Exclude profiler-generated symbols from MSVC __imp_-symbol workaround. #59812

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
Apr 14, 2019

Conversation

michaelwoerister
Copy link
Member

@michaelwoerister michaelwoerister commented Apr 9, 2019

LLVM's profiling instrumentation adds a few symbols that are used by the profiler runtime. Since these show up as globals in the LLVM IR, the compiler generates dllimport-related __imp_ stubs for them. This can lead to linker errors because the instrumentation symbols have weak linkage or are in a comdat section, but the __imp_ stubs aren't.

Instead of trying to replicate the linkage/comdat setup for the stubs, this PR just excludes the profiler-related symbols from stub-generation since they aren't supposed to be referenced via __declspec(dllimport) anywhere anyway.

r? @alexcrichton

EDIT: I considered making this more general, i.e. inferring from the symbol name if it is a Rust symbol or not. But then I figured out that that would yield false negatives for #[no_mangle] et al, so I went with a blacklist approach.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 9, 2019
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Apr 9, 2019

📌 Commit 724ca05 has been approved by alexcrichton

@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 Apr 9, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
…mp, r=alexcrichton

Exclude profiler-generated symbols from MSVC __imp_-symbol workaround.

LLVM's profiling instrumentation adds a few symbols that are used by the profiler runtime. Since these show up as globals in the LLVM IR, the compiler generates `dllimport`-related `__imp_` stubs for them. This can lead to linker errors because the instrumentation symbols have weak linkage or are in a comdat section, but the `__imp_` stubs aren't.

Instead of trying to replicate the linkage/comdat setup for the stubs, this PR just excludes the profiler-related symbols from stub-generation since they aren't supposed to be referenced via `__declspec(dllimport)` anywhere anyway.

r? @alexcrichton

EDIT: I considered making this more general, i.e. inferring from the symbol name if it is a Rust symbol or not. But then I figured out that that would yield false negatives for `#[no_mangle]` et al, so I went with a blacklist approach.
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
Rollup of 9 pull requests

Successful merges:

 - rust-lang#59655 (Use a proc macro to declare preallocated symbols)
 - rust-lang#59769 (compiletest normalization: preserve non-JSON lines such as ICEs)
 - rust-lang#59776 (Apply resource-suffix to search-index and source-files scripts as well)
 - rust-lang#59784 (Suggest importing macros from the crate root)
 - rust-lang#59812 (Exclude profiler-generated symbols from MSVC __imp_-symbol workaround.)
 - rust-lang#59856 (update polonius-engine)
 - rust-lang#59874 (Clean up handling of `-Z pgo-gen` commandline option.)
 - rust-lang#59890 (Don't generate empty json variables)
 - rust-lang#59911 (Revert "compile crates under test w/ -Zemit-stack-sizes")

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
…mp, r=alexcrichton

Exclude profiler-generated symbols from MSVC __imp_-symbol workaround.

LLVM's profiling instrumentation adds a few symbols that are used by the profiler runtime. Since these show up as globals in the LLVM IR, the compiler generates `dllimport`-related `__imp_` stubs for them. This can lead to linker errors because the instrumentation symbols have weak linkage or are in a comdat section, but the `__imp_` stubs aren't.

Instead of trying to replicate the linkage/comdat setup for the stubs, this PR just excludes the profiler-related symbols from stub-generation since they aren't supposed to be referenced via `__declspec(dllimport)` anywhere anyway.

r? @alexcrichton

EDIT: I considered making this more general, i.e. inferring from the symbol name if it is a Rust symbol or not. But then I figured out that that would yield false negatives for `#[no_mangle]` et al, so I went with a blacklist approach.
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
…mp, r=alexcrichton

Exclude profiler-generated symbols from MSVC __imp_-symbol workaround.

LLVM's profiling instrumentation adds a few symbols that are used by the profiler runtime. Since these show up as globals in the LLVM IR, the compiler generates `dllimport`-related `__imp_` stubs for them. This can lead to linker errors because the instrumentation symbols have weak linkage or are in a comdat section, but the `__imp_` stubs aren't.

Instead of trying to replicate the linkage/comdat setup for the stubs, this PR just excludes the profiler-related symbols from stub-generation since they aren't supposed to be referenced via `__declspec(dllimport)` anywhere anyway.

r? @alexcrichton

EDIT: I considered making this more general, i.e. inferring from the symbol name if it is a Rust symbol or not. But then I figured out that that would yield false negatives for `#[no_mangle]` et al, so I went with a blacklist approach.
bors added a commit that referenced this pull request Apr 14, 2019
Rollup of 6 pull requests

Successful merges:

 - #59776 (Apply resource-suffix to search-index and source-files scripts as well)
 - #59784 (Suggest importing macros from the crate root)
 - #59812 (Exclude profiler-generated symbols from MSVC __imp_-symbol workaround.)
 - #59874 (Clean up handling of `-Z pgo-gen` commandline option.)
 - #59890 (Don't generate empty json variables)
 - #59911 (Revert "compile crates under test w/ -Zemit-stack-sizes")

Failed merges:

r? @ghost
@bors bors merged commit 724ca05 into rust-lang:master Apr 14, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants