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

extern "C" functions are not internalized with -C lto #18134

Closed
Zoxc opened this issue Oct 18, 2014 · 16 comments
Closed

extern "C" functions are not internalized with -C lto #18134

Zoxc opened this issue Oct 18, 2014 · 16 comments
Labels
A-codegen Area: Code generation I-slow Issue: Problems and improvements with respect to performance of generated code. P-low Low priority

Comments

@Zoxc
Copy link
Contributor

Zoxc commented Oct 18, 2014

When marking public functions with #[no_mangle] and then linking to the crate with the functions using -C lto, the functions will be exported in the output.

This can result in errors about duplicate definitions (when linking things linked with -C lto together) and also prevents the removal of unused functions.

@Zoxc
Copy link
Contributor Author

Zoxc commented Oct 18, 2014

Some further investigation shows that this is due to extern "C" being on function definitions, not #[no_mangle]. It's also not due to IR differences in from adding extern "C".

@Zoxc Zoxc changed the title #[no_mangle] functions are not internalized with -C lto extern "C" functions are not internalized with -C lto Oct 18, 2014
@thestinger
Copy link
Contributor

At some point, there was a decision to treat extern functions differently in the visibility rules. There may be no way to make these functions have internal visibility now, so it's a performance bug.

@thestinger thestinger added I-slow Issue: Problems and improvements with respect to performance of generated code. A-codegen Area: Code generation labels Oct 18, 2014
@huonw
Copy link
Member

huonw commented Oct 18, 2014

Why would there be no way to make them internally visible?

@thestinger
Copy link
Contributor

I can't tell you the reasoning behind it because I strongly disagreed with it and made that clear when it happened. I can point you at the commit that implemented this. I didn't understand why the ABI is relevant to visibility then and I don't understand it now. IMO, it's an awful hack to work around broken code that could have just been fixed.

@thestinger
Copy link
Contributor

Nominating since fixing this would be a backwards incompatible language change. It would begin causing errors due to missing symbols in previously valid code.

@alexcrichton
Copy link
Member

@Zoxc can you provide a test case? I'm unable to reproduce this with a few different permutations

@thestinger
Copy link
Contributor

Define a pub fn foo() and a pub extern "C" fn foo() in a crate, and link against it with LTO. The Rust ABI function is marked internal is the executable and the C ABI function is not.

@Zoxc
Copy link
Contributor Author

Zoxc commented Oct 18, 2014

etest.rs:

#![crate_name = "etest"]
#![crate_type = "rlib"]
#![no_std]

pub fn func1() {}
pub extern fn func2() {}

bare.rs:

#![no_std]
#![no_main]
#![feature(lang_items)]

extern crate etest;

#[lang = "begin_unwind"] extern fn begin_unwind() {}
#[lang = "stack_exhausted"] extern fn stack_exhausted() {}
#[lang = "eh_personality"] extern fn eh_personality() {}
#[lang = "fail_fmt"] fn fail_fmt() {}

Buliding:
rustc etest.rs
rustc -L . -C lto bare.rs --emit=ir

func2 is exported in the bare create, but func1 is not.

@pnkfelix
Copy link
Member

Assigning P-low, not 1.0.

@pnkfelix pnkfelix added P-low Low priority and removed I-nominated labels Oct 23, 2014
@alexcrichton
Copy link
Member

I'm actually going to close this as working as intended now that I've had some time to investigate it more thoroughly. The impetus for this functionality is #14500, which I do think is a real and valid issue to consider. Any reachable functions with a Rust ABI are implementation details (and therefore candidates to be optimized away). Any reachable functions with a defined ABI, however, are not implementation details, and consequently are not candidates to be optimized away.

If you do not compile with -C lto, then there will be linkage conflicts because the objects in rlibs all have their symbols already exposed (for all ABIs). You can control the visibility of a symbol in an object file through the visibility of the function itself. If a function is not reachable, then it won't be marked as visible.

@thestinger
Copy link
Contributor

Rust is exposing unreachable functions from the final executable. It would only be reachable if the executable crate exposed it as public at the top-level. The included library crate is private at the top-level so those symbols shouldn't get exported in an LTO build. If you want them exported, you should be able to mark the crate as public (or use pub use).

There's a valid performance bug as long as there's not an equivalent to -fvisibility=hidden for C and C++. Rust is almost there because it's handling the Rust ABI functions sanely, but I don't understand why the ABI is a valid reason to make a distinction in visibility rules. It's forcing code that's not actually exported from the executable to be exposed.

@alexcrichton
Copy link
Member

Currently we have no notion of a "private crate" or a "public crate", we just link crates together and the interface they expose is always public. I'm not sure why you say we don't have the equivalent of -fvisibility=hidden because that's basically wrapping your entire crate in mod foo { ... } to make all symbols private.

I also don't quite understand why you think that we're exposing unreachable functions. All of the exported functions are reachable with respect to privacy (as the example from @Zoxc shows). Do you have an example where an un-reachable extern function gets exposed? I do think that would be a bug (I haven't seen an example in this thread). Some exact reproducible code would also be more helpful than a description of a test case.

@DemiMarie
Copy link
Contributor

One case where this would be useful is where is using lto so that cross-language optimizations can take place. For instance, one can (or should be able to) inline a small C function compiled with Clang into Rust code, or inline a Rust function into C code. In both cases, the function may be private to the combined library.

@Zoxc
Copy link
Contributor Author

Zoxc commented Oct 10, 2016

It appears extern functions are removed now due to a recent change (tested on 3210fd5). However doing so isn't always the right thing either. This breaks rlibc.

@Mark-Simulacrum
Copy link
Member

@alexcrichton So we first made extern "C" functions stay around with LTO, but now we no longer do this, so I'm not sure -- should we close this issue? It's not clear whether we want or don't want this.

@alexcrichton
Copy link
Member

I believe bugs have been fixed in the meantime, it looks like this issue is no longer applicable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation I-slow Issue: Problems and improvements with respect to performance of generated code. P-low Low priority
Projects
None yet
Development

No branches or pull requests

7 participants