Skip to content

RFC-2841: add codegen flag export symbols from executable - #85673

Merged
bors merged 1 commit into
rust-lang:masterfrom
csmoe:export-exe-sym
Jul 25, 2022
Merged

RFC-2841: add codegen flag export symbols from executable#85673
bors merged 1 commit into
rust-lang:masterfrom
csmoe:export-exe-sym

Conversation

@csmoe

Copy link
Copy Markdown
Contributor

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 25, 2021
@rust-log-analyzer

This comment has been minimized.

@crlf0710crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 11, 2021
@crlf0710

Copy link
Copy Markdown
Member

@csmoe Ping from triage, CI is still red here. Would you mind fixing that?

Comment threadcompiler/rustc_interface/src/tests.rs Outdated
Comment threadcompiler/rustc_session/src/options.rs Outdated

@nikomatsakisnikomatsakis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, do we have any tests for symbol visibility? Maybe in the run-make folder? It'd be good to have a test for this.

@crlf0710

Copy link
Copy Markdown
Member

@csmoe Ping from triage, would you mind adding the tests?

@JohnCSimonJohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 25, 2021
@rust-log-analyzer

This comment has been minimized.

Comment threadsrc/test/run-make/export-executable-symbols/main.rs Outdated
@csmoe
csmoeforce-pushed the export-exe-sym branch 2 times, most recently from 79e3b3d to 6b14c11CompareAugust 22, 2021 13:35
Comment threadsrc/test/run-make/export-executable-symbols/main.rs Outdated
Comment threadsrc/test/run-make/export-executable-symbols/main.rs Outdated
@JohnCSimonJohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 12, 2021
@csmoe

csmoe commented Sep 12, 2021

Copy link
Copy Markdown
ContributorAuthor

@bjorn3-Cexport-executable-symbols seems not work at all as dlopen errors with undefined symbol: exported_symbol, RUSTC_LOG=rustc_codegen_ssa::back::linker=debug printed that the symbol was exported indeed:

 INFO rustc_codegen_ssa::back::link preparing Executable to "main"
DEBUG rustc_codegen_ssa::back::linker EXPORTED SYMBOLS:
DEBUG rustc_codegen_ssa::back::linker exported_symbol;
DEBUG rustc_codegen_ssa::back::linker main;
DEBUG rustc_codegen_ssa::back::linker rust_eh_personality;

Need more help to move on 😿
(btw, don't know why the CI did not error on the test)

@bjorn3

Copy link
Copy Markdown
Member

Are you on Linux or macOS? What is the full linker invocation?

@csmoe

csmoe commented Sep 13, 2021

Copy link
Copy Markdown
ContributorAuthor

"cc" "-Wl,--version-script=/repo/rust/./rustcqC1COd/list" "-m64" "main.main.cbc7ddee-cgu.0.rcgu.o" "main.main.cbc7ddee-cgu.1.rcgu.o" "main.main.cbc7ddee-cgu.10.rcgu.o" "main.main.cbc7ddee-cgu.11.rcgu.o" "main.main.cbc7ddee-cgu.12.rcgu.o" "main.main.cbc7ddee-cgu.13.rcgu.o" "main.main.cbc7ddee-cgu.14.rcgu.o" "main.main.cbc7ddee-cgu.15.rcgu.o" "main.main.cbc7ddee-cgu.2.rcgu.o" "main.main.cbc7ddee-cgu.3.rcgu.o" "main.main.cbc7ddee-cgu.4.rcgu.o" "main.main.cbc7ddee-cgu.5.rcgu.o" "main.main.cbc7ddee-cgu.6.rcgu.o" "main.main.cbc7ddee-cgu.7.rcgu.o" "main.main.cbc7ddee-cgu.8.rcgu.o" "main.main.cbc7ddee-cgu.9.rcgu.o" "main.4kqwyhdgeomtzpxx.rcgu.o" "-Wl,--as-needed" "-L" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,--start-group" "-Wl,-Bstatic" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-ea06e5b9ebe84ef4.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libpanic_unwind-608126472e4595bf.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libminiz_oxide-96b872de832ae456.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libadler-c96f7126ed0a8243.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libobject-d4333fc5a8e6acd5.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libmemchr-286e391a7bef4a33.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libaddr2line-9b3b5b41ef9d89ac.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libgimli-a01624f3c4d2f24b.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd_detect-606d386967062702.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_demangle-9fec6174a0598006.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libhashbrown-612142634ae08506.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_alloc-8a18ecd4776dc12c.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libunwind-11234a29b766d417.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcfg_if-5c49a6fde622d4a9.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/liblibc-01242e2bb437fc18.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/liballoc-a1d9b6c563f72c92.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_core-5a76a6f677cfd881.rlib" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-99c01d738b9e1246.rlib" "-Wl,--end-group" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-8ee0ab4b35b97256.rlib" "-Wl,-Bdynamic" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-Wl,--eh-frame-hdr" "-Wl,-znoexecstack" "-L" "/repo/rust/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "main" "-Wl,--gc-sections" "-pie" "-Wl,-zrelro" "-Wl,-znow" "-nodefaultlibs"

@bjorn3 linux, cc args as above.

@bjorn3

Copy link
Copy Markdown
Member

Could you compile with -Csave-temps and post contents of the version script passed to -Wl,--version-script=?

@bjorn3

bjorn3 commented Sep 13, 2021

Copy link
Copy Markdown
Member

Maybe --gc-keep-exported is necessary?

When --gc-sections is enabled, this option prevents garbage collection of unused input sections that contain global symbols having default or protected visibility. This
option is intended to be used for executables where unreferenced sections would otherwise be garbage collected regardless of the external visibility of contained symbols.
Note that this option has no effect when linking shared objects since it is already the default behaviour. This option is only supported for ELF format targets.

@csmoe

Copy link
Copy Markdown
ContributorAuthor

@bjorn3 version-script:

08:40 $ cat rustcXKEpHW/list
{
global:
exported_symbol;
main;
rust_eh_personality;
local:
*;
};

--gc-keep-exported is not available in old gcc(v6.3.0), I'll try a newer.

@csmoe

This comment has been minimized.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use crate_type != Executable || !export_executable_symbols?

@bjorn3

Copy link
Copy Markdown
Member

I am not entirely confident in -Zexport-executable-symbols being implemented correctly here, but I am confident that this doesn't break anything if -Zexport-executable-symbols isn't used. As such r=me with or without my nits above.

@csmoe

Copy link
Copy Markdown
ContributorAuthor

I tested locally in a small crate with

#[no_mangle]#[test]pubfntest_foo(){ ...}

the test_foo symbol was exported as expected in the unit test executable.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Comment threadcompiler/rustc_codegen_ssa/src/back/link.rs Outdated
@bjorn3

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Jul 21, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit 419e62fa4bad3447421437d96053d2bbe27fe92a has been approved by bjorn3

It is now in the queue for this repository.

@bors

bors commented Jul 21, 2022

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #98162) made this pull request unmergeable. Please resolve the merge conflicts.

@bjorn3

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Jul 21, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit 4ae6ff3e91ba8f62049667fc960791f0c342f145 has been approved by bjorn3

It is now in the queue for this repository.

@bors

bors commented Jul 21, 2022

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 4ae6ff3e91ba8f62049667fc960791f0c342f145 with merge 00bddad26f9229d8db1983b7ebf87fec998dd206...

@bors

bors commented Jul 21, 2022

Copy link
Copy Markdown
Collaborator

💔 Test failed - checks-actions

@bjorn3

Copy link
Copy Markdown
Member

The test needs to be ignored on nvptx64-nvidia-cuda. Maybe an // only-unix annotation works? Not sure if that is valid though.

@rust-log-analyzer

This comment has been minimized.

@csmoe

csmoe commented Jul 21, 2022

Copy link
Copy Markdown
ContributorAuthor

only-unix seems not working, as it ignored linux. I just limited the test with only-linux.

@bjorn3

Copy link
Copy Markdown
Member

👍 Could you please squash? I noticed that one of the commits introduced merge conflict markers which another solves again. And besides, this is pretty much an atimic change. r=me after squashing

@csmoe

Copy link
Copy Markdown
ContributorAuthor

@bors r=@bjorn3

@bors

bors commented Jul 25, 2022

Copy link
Copy Markdown
Collaborator

@csmoe: 🔑 Insufficient privileges: Not in reviewers

@bjorn3

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Jul 25, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit 6674c94 has been approved by bjorn3

It is now in the queue for this repository.

@bors

bors commented Jul 25, 2022

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 6674c94 with merge dc2d232...

@bors

bors commented Jul 25, 2022

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: bjorn3
Pushing dc2d232 to master...

@bjorn3

Copy link
Copy Markdown
Member

That only took 10 tries...

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (dc2d232): comparison url.

Instruction count

  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 🎉 relevant improvement found
mean1maxcount2
Regressions 😿
(primary)
N/AN/A0
Regressions 😿
(secondary)
N/AN/A0
Improvements 🎉
(primary)
N/AN/A0
Improvements 🎉
(secondary)
-0.2%-0.2%1
All 😿🎉 (primary)N/AN/A0

Max RSS (memory usage)

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 😿 relevant regression found
mean1maxcount2
Regressions 😿
(primary)
N/AN/A0
Regressions 😿
(secondary)
2.7%2.7%1
Improvements 🎉
(primary)
N/AN/A0
Improvements 🎉
(secondary)
N/AN/A0
All 😿🎉 (primary)N/AN/A0

Cycles

Results
  • Primary benchmarks: 😿 relevant regression found
  • Secondary benchmarks: mixed results
mean1maxcount2
Regressions 😿
(primary)
2.3%2.3%1
Regressions 😿
(secondary)
2.9%3.8%4
Improvements 🎉
(primary)
N/AN/A0
Improvements 🎉
(secondary)
-2.3%-2.6%4
All 😿🎉 (primary)2.3%2.3%1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change23

  2. number of relevant changes23

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tracking Issue for -C export-executable-symbols

14 participants

@csmoe@rust-log-analyzer@crlf0710@bjorn3@JohnCSimon@nikomatsakis@bors@Dylan-DPC@matthiaskrgr@RalfJung@rust-timer@apiraino@rust-highfive@rustbot