Skip to content

Clarify why SGX code specifies linkage/symbol names for certain statics #139795

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

Merged
merged 1 commit into from
Apr 21, 2025

Conversation

jethrogb
Copy link
Contributor

Specifying linkage/symbol name is solely to ensure a single instance between the std crate and its unit tests.

Also update the symbol names as items have moved around a bit. The actual name isn't that important, it just needs to be unique. But for debugging it can be useful for it to point to the right place.

Also update the symbol names as items have moved around a bit. The actual
name isn't that important, it just needs to be unique. But for debugging
it can be useful for it to point to the right place.
@rustbot
Copy link
Collaborator

rustbot commented Apr 14, 2025

r? @joboet

rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added O-SGX Target: SGX S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 14, 2025
@thaliaarchi
Copy link
Contributor

What is unique about SGX that requires this, unlike other platforms?

@jethrogb
Copy link
Contributor Author

I can't speak for all platforms, but most “regular” platforms like Linux, etc. relegate this kind of once-per-process state to libc.

@thaliaarchi
Copy link
Contributor

As a possible counter-example, Unix platforms store their argc/argv values in static atomics.

@jethrogb
Copy link
Contributor Author

Linux also does some linking tricks there:

/// glibc passes argc, argv, and envp to functions in .init_array, as a non-standard extension.
/// This allows `std::env::args` to work even in a `cdylib`, as it does on macOS and Windows.
#[cfg(all(target_os = "linux", target_env = "gnu"))]
#[used]
#[unsafe(link_section = ".init_array.00099")]
static ARGV_INIT_ARRAY: extern "C" fn(
crate::os::raw::c_int,
*const *const u8,
*const *const u8,
) = {
extern "C" fn init_wrapper(

Presumably when looking at a build of the unit tests for std, you'd find that there are two copies of init_wrapper that are both being called by the dynamic loader, so that both versions of those symbols end up having the same value.

@thaliaarchi
Copy link
Contributor

No, I think ARGV_INIT_ARRAY is different. It's a list of function pointers to be sent the args and env upon initialization, as the comment says, and needs the particular link section to be recognized as such. It looks like it's doing the same thing as linkme by specifying a single element, which the linker coalesces with all others marked the same into one array. So, it's the opposite of what you're doing.

In any case, this comment change looks good.

@joboet
Copy link
Member

joboet commented Apr 20, 2025

Adding a note is great even if there might be better ways to achieve the desired behaviour, so:
@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 20, 2025

📌 Commit 8dc7732 has been approved by joboet

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 Apr 20, 2025
@bjorn3
Copy link
Member

bjorn3 commented Apr 20, 2025

A lot of places in libstd use #[cfg(not(test))] and instead import from "realstd" (which is the libstd that libtest links against rather than the libstd that is being tested) when testing to avoid duplicate lang items. Is there any reason why SGX can't do the same?

jhpratt added a commit to jhpratt/rust that referenced this pull request Apr 21, 2025
…r=joboet

Clarify why SGX code specifies linkage/symbol names for certain statics

Specifying linkage/symbol name is solely to ensure a single instance between the `std` crate and its unit tests.

Also update the symbol names as items have moved around a bit. The actual name isn't that important, it just needs to be unique. But for debugging it can be useful for it to point to the right place.
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 21, 2025
Rollup of 11 pull requests

Successful merges:

 - rust-lang#139795 (Clarify why SGX code specifies linkage/symbol names for certain statics)
 - rust-lang#139946 (fix missing word in comment)
 - rust-lang#139982 (SystemTime doc tweaks)
 - rust-lang#140009 (docs(LocalKey<T>): clarify that T's Drop shouldn't panic)
 - rust-lang#140021 (Don't ICE on pending obligations from deep normalization in a loop)
 - rust-lang#140036 (Advent of `tests/ui` (misc cleanups and improvements) [4/N])
 - rust-lang#140047 (remove a couple clones)
 - rust-lang#140052 (Fix error when an intra doc link is trying to resolve an empty associated item)
 - rust-lang#140074 (rustdoc-json: Improve test for auto-trait impls)
 - rust-lang#140076 (jsondocck: Require command is at start of line)
 - rust-lang#140081 (Update `libc` to 0.2.172)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 21, 2025
Rollup of 11 pull requests

Successful merges:

 - rust-lang#139795 (Clarify why SGX code specifies linkage/symbol names for certain statics)
 - rust-lang#139946 (fix missing word in comment)
 - rust-lang#139982 (SystemTime doc tweaks)
 - rust-lang#140009 (docs(LocalKey<T>): clarify that T's Drop shouldn't panic)
 - rust-lang#140021 (Don't ICE on pending obligations from deep normalization in a loop)
 - rust-lang#140036 (Advent of `tests/ui` (misc cleanups and improvements) [4/N])
 - rust-lang#140047 (remove a couple clones)
 - rust-lang#140052 (Fix error when an intra doc link is trying to resolve an empty associated item)
 - rust-lang#140074 (rustdoc-json: Improve test for auto-trait impls)
 - rust-lang#140076 (jsondocck: Require command is at start of line)
 - rust-lang#140081 (Update `libc` to 0.2.172)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 21, 2025
Rollup of 11 pull requests

Successful merges:

 - rust-lang#139795 (Clarify why SGX code specifies linkage/symbol names for certain statics)
 - rust-lang#139946 (fix missing word in comment)
 - rust-lang#139982 (SystemTime doc tweaks)
 - rust-lang#140009 (docs(LocalKey<T>): clarify that T's Drop shouldn't panic)
 - rust-lang#140021 (Don't ICE on pending obligations from deep normalization in a loop)
 - rust-lang#140036 (Advent of `tests/ui` (misc cleanups and improvements) [4/N])
 - rust-lang#140047 (remove a couple clones)
 - rust-lang#140052 (Fix error when an intra doc link is trying to resolve an empty associated item)
 - rust-lang#140074 (rustdoc-json: Improve test for auto-trait impls)
 - rust-lang#140076 (jsondocck: Require command is at start of line)
 - rust-lang#140081 (Update `libc` to 0.2.172)

r? `@ghost`
`@rustbot` modify labels: rollup
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Apr 21, 2025
…r=joboet

Clarify why SGX code specifies linkage/symbol names for certain statics

Specifying linkage/symbol name is solely to ensure a single instance between the `std` crate and its unit tests.

Also update the symbol names as items have moved around a bit. The actual name isn't that important, it just needs to be unique. But for debugging it can be useful for it to point to the right place.
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 21, 2025
Rollup of 12 pull requests

Successful merges:

 - rust-lang#134213 (Stabilize `naked_functions`)
 - rust-lang#139795 (Clarify why SGX code specifies linkage/symbol names for certain statics)
 - rust-lang#139981 (Don't compute name of associated item if it's an RPITIT)
 - rust-lang#140036 (Advent of `tests/ui` (misc cleanups and improvements) [4/N])
 - rust-lang#140047 (remove a couple clones)
 - rust-lang#140052 (Fix error when an intra doc link is trying to resolve an empty associated item)
 - rust-lang#140074 (rustdoc-json: Improve test for auto-trait impls)
 - rust-lang#140076 (jsondocck: Require command is at start of line)
 - rust-lang#140077 (Construct OutputType using macro and print [=FILENAME] help info)
 - rust-lang#140081 (Update `libc` to 0.2.172)
 - rust-lang#140091 (build_helper: try to rename dir before delete)
 - rust-lang#140107 (rustc-dev-guide subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 21, 2025
…enton

Rollup of 11 pull requests

Successful merges:

 - rust-lang#134213 (Stabilize `naked_functions`)
 - rust-lang#139711 (Hermit: Unify `std::env::args` with Unix)
 - rust-lang#139795 (Clarify why SGX code specifies linkage/symbol names for certain statics)
 - rust-lang#140036 (Advent of `tests/ui` (misc cleanups and improvements) [4/N])
 - rust-lang#140047 (remove a couple clones)
 - rust-lang#140052 (Fix error when an intra doc link is trying to resolve an empty associated item)
 - rust-lang#140074 (rustdoc-json: Improve test for auto-trait impls)
 - rust-lang#140076 (jsondocck: Require command is at start of line)
 - rust-lang#140107 (rustc-dev-guide subtree update)
 - rust-lang#140111 (cleanup redundant pattern instances)
 - rust-lang#140118 ({B,C}Str: minor cleanup)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit cac8bc3 into rust-lang:master Apr 21, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 21, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 21, 2025
Rollup merge of rust-lang#139795 - jethrogb:jb/sgx-linkage-comments, r=joboet

Clarify why SGX code specifies linkage/symbol names for certain statics

Specifying linkage/symbol name is solely to ensure a single instance between the `std` crate and its unit tests.

Also update the symbol names as items have moved around a bit. The actual name isn't that important, it just needs to be unique. But for debugging it can be useful for it to point to the right place.
@jethrogb
Copy link
Contributor Author

@bjorn3 you can only import public items that way. None of the items at issue here are public.

github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Apr 23, 2025
…r=joboet

Clarify why SGX code specifies linkage/symbol names for certain statics

Specifying linkage/symbol name is solely to ensure a single instance between the `std` crate and its unit tests.

Also update the symbol names as items have moved around a bit. The actual name isn't that important, it just needs to be unique. But for debugging it can be useful for it to point to the right place.
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Apr 23, 2025
…enton

Rollup of 11 pull requests

Successful merges:

 - rust-lang#134213 (Stabilize `naked_functions`)
 - rust-lang#139711 (Hermit: Unify `std::env::args` with Unix)
 - rust-lang#139795 (Clarify why SGX code specifies linkage/symbol names for certain statics)
 - rust-lang#140036 (Advent of `tests/ui` (misc cleanups and improvements) [4/N])
 - rust-lang#140047 (remove a couple clones)
 - rust-lang#140052 (Fix error when an intra doc link is trying to resolve an empty associated item)
 - rust-lang#140074 (rustdoc-json: Improve test for auto-trait impls)
 - rust-lang#140076 (jsondocck: Require command is at start of line)
 - rust-lang#140107 (rustc-dev-guide subtree update)
 - rust-lang#140111 (cleanup redundant pattern instances)
 - rust-lang#140118 ({B,C}Str: minor cleanup)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-SGX Target: SGX S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants