-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
#[used] and symbol visibility is unclear #54135
Comments
I should also note, I'm not even sure that such a tweak here would actually fix our use case in |
In my experience, which pretty much only involves ELF objects, whenever I use Some info about my use case: I always pair #[no_mangle]
pub fn main() { .. }
I sometimes pair #[link_section = ".vector_table.reset_vector"]
#[no_mangle]
pub static RESET_VECTOR: fn() = reset;
#[link_section = ".vector_table.exceptions"]
#[no_mangle]
pub static EXCEPTIONS: [fn(); 14] = [..];
I don't have much problem with this issue but we have embedded crates that With that background info out of the way, My proposal would be to instead have
There's also the third option of adding a new attribute to control visibility:
Some people (myself included) would find that surprising as (a) LD's // src/main.rs
fn main() {}
// internal visibility
#[no_mangle]
#[used]
static FOO: u32 = 1; $ cargo rustc --release -- -C link-arg=-Wl,-uFOO
$ nm target/release/hello | grep FOO
U FOO // src/main.rs
fn main() {}
// external visibility
#[no_mangle]
pub static FOO: u32 = 1; $ cargo rustc --release -- -C link-arg=-Wl,-uFOO
$ nm target/release/hello | grep FOO
000000000004e000 R FOO (b) Internal symbols do not work for intra-Rust FFI which is the only case I // bar/src/lib.rs
#[no_mangle]
fn foo() {}
#[used]
static KEEP: fn() = foo; // src/main.rs
extern crate bar;
fn main() {
extern "Rust" {
fn foo();
}
unsafe {
foo();
}
} This fails to link: $ cargo run
(..)
/home/japaric/tmp/hello/src/main.rs:9: undefined reference to `foo' If you tweak // bar/src/lib.rs
#[no_mangle]
pub fn foo() {} Then the program links. |
One reason would be when using |
Due to parallel codegen I don't think it's guaranteed that the I have had plenty of trouble with multiple codegen units and I have found that the only thing that reliably works is |
Thank for writing that up @japaric! I think I agree with you that I hadn't really considered it before but forcing all We do have an unstable |
👍. Just want to add: let's apply the same logic to |
Ah sorry yeah, |
Coming from the perspective of the Windows linker model, I am absolutely in agreement that |
I don't know if this kind of change needs a RFC or FCP but I have put up a PR that implements this in #54414. |
@alexcrichton you are right that setting breakpoints on |
From the PR:
How would one make sure, that privacy of Also, such change would make all |
@nagisa the PR's changes propose doing that, yes, circumventing privacy. That seems to me, though, simply a fact of how |
This commit updates the compiler to allow the `#[no_mangle]` (and `#[export_name]` attributes) to be located anywhere within a crate. These attributes are unconditionally processed, causing the compiler to always generate an exported symbol with the appropriate name. After some discussion on rust-lang#54135 it was found that not a great reason this hasn't been allowed already, and it seems to match the behavior that many expect! Previously the compiler would only export a `#[no_mangle]` symbol if it were *publicly reachable*, meaning that it itself is `pub` and it's otherwise publicly reachable from the root of the crate. This new definition is that `#[no_mangle]` *is always reachable*, no matter where it is in a crate or whether it has `pub` or not. This should make it much easier to declare an exported symbol with a known and unique name, even when it's an internal implementation detail of the crate itself. Note that these symbols will persist beyond LTO as well, always making their way to the linker. Along the way this commit removes the `private_no_mangle_functions` lint (also for statics) as there's no longer any need to lint these situations. Furthermore a good number of tests were updated now that symbol visibility has been changed. Closes rust-lang#54135
…elwoerister rustc: Allow `#[no_mangle]` anywhere in a crate This commit updates the compiler to allow the `#[no_mangle]` (and `#[export_name]` attributes) to be located anywhere within a crate. These attributes are unconditionally processed, causing the compiler to always generate an exported symbol with the appropriate name. After some discussion on #54135 it was found that not a great reason this hasn't been allowed already, and it seems to match the behavior that many expect! Previously the compiler would only export a `#[no_mangle]` symbol if it were *publicly reachable*, meaning that it itself is `pub` and it's otherwise publicly reachable from the root of the crate. This new definition is that `#[no_mangle]` *is always reachable*, no matter where it is in a crate or whether it has `pub` or not. This should make it much easier to declare an exported symbol with a known and unique name, even when it's an internal implementation detail of the crate itself. Note that these symbols will persist beyond LTO as well, always making their way to the linker. Along the way this commit removes the `private_no_mangle_functions` lint (also for statics) as there's no longer any need to lint these situations. Furthermore a good number of tests were updated now that symbol visibility has been changed. Closes #54135
This commit updates the compiler to allow the `#[no_mangle]` (and `#[export_name]` attributes) to be located anywhere within a crate. These attributes are unconditionally processed, causing the compiler to always generate an exported symbol with the appropriate name. After some discussion on rust-lang#54135 it was found that not a great reason this hasn't been allowed already, and it seems to match the behavior that many expect! Previously the compiler would only export a `#[no_mangle]` symbol if it were *publicly reachable*, meaning that it itself is `pub` and it's otherwise publicly reachable from the root of the crate. This new definition is that `#[no_mangle]` *is always reachable*, no matter where it is in a crate or whether it has `pub` or not. This should make it much easier to declare an exported symbol with a known and unique name, even when it's an internal implementation detail of the crate itself. Note that these symbols will persist beyond LTO as well, always making their way to the linker. Along the way this commit removes the `private_no_mangle_functions` lint (also for statics) as there's no longer any need to lint these situations. Furthermore a good number of tests were updated now that symbol visibility has been changed. Closes rust-lang#54135
I came across this issue when searching for ways to do the opposite. I'm linking with C code via The problem is, now the functions that are referred to by Is there any way to circumvent that and make sure that symbols remain visible only internally between translation units, but not exposed from the final artifact? |
I filed a new issue for that. |
I was excited to see
#[used]
stabilized (yay!) as one of the issues we suffer from inwasm-bindgen
is related to symbols being removed. Unfortunately though#[used]
doesn't solve our use case!First I'll try to explain our issue a bit. The
#[wasm_bindgen]
attribute allows you to import JS functionality into a Rust program. This doesn't work, however, when you import JS functions into a private Rust submodule. (akamod foo { ... }
). When importing a function we also generate an internal exported function which the CLIwasm-bindgen
tool uses (and then removes), but it suffices to say that we're generating code that looks like:Today the symbol
foo
is not considered alive by rustc itself as it's not reachable. As a result, it's not even translated into the object file. If we instead change this though:This still doesn't work! Unfortunately for us the
#[used]
works as intended but doesn't affect the symbol visibility. The above program generates this IR:the problem here is that the symbol
foo
, while not mangled, is still marked asinternal
. This in turns means that it does indeed reach the linker, but for our purposes inwasm-bindgen
we need it to survive the linker, not just reach the linker.Ok so that's the problem statement for
wasm-bindgen
, but you can generalize it today for rustc by asking: what does#[used]
do to symbol visibility? The overall story for symbol visibility in rustc is a little muddied and not always great (especially on ABI-particulars like#[no_mangle]
things).What should the symbol visibility of
foo
be here?We've always had a basic rule of thumb in Rust that "reachable symbols" have non-internal visibility, but it's not clear what to do here.
foo
is indeed a reachable symbol because of#[used]
, but it's in a private module. Does that mean because ofpub
and#[no_mangle]
it shouldn't haveinternal
visibility? Should only#[no_mangle]
imply that? It's unclear to me!I'd naively like to send a patch that makes foo not-
internal
because it has#[no_mangle]
andpub
(not that it's "publicly reachable"). I think though that this may be deeper in the compiler. I just took a look at how#[used]
works, and it's actually a little suprising!In
src/librustc_mir/monomorphize/collector.rs
we attempt to not translate anything not reachable in a crate as a form of DCE. I didn't find any handling of#[used]
, though, and it turns out we unconditionally translate all statics all the time! Then becuase we put it inllvm.used
it ends up not getting gc'd by LLVM.I think that we may want to future-proof this by updating the
src/librustc/middle/reachable.rs
collection step to basically push#[used]
statics onto the worklist to process. The initial worklist is seeded with all items that are public by visibility, and I think we could change it to also be seeded with any#[used]
statics. This means that anything referenced by a#[used]
static will be pulled in as a result.Do others think this is a reasonable strategy for having
#[used]
affect symbol visibility?cc @michaelwoerister
cc @fitzgen
cc @japaric
The text was updated successfully, but these errors were encountered: