-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
LTO preserves too many extern
symbols
#34985
Comments
I'd like to give this a shot if that's ok. What's the best way to test a fix? Running the above test case? |
@wesleywiser sure! I'm not sure there's a great test case we can check in for this, but to manually test it yeah the commands in the description should be good enough. |
Ok cool! I believe your hunch was correct. The code was balling out too soon if the function's ABI wasn't Rust. Removing the early return and running the above commands no longer shows those symbols. Is there any other tests I should run before opening a PR? (The |
Awesome! I'd run a full |
Previously, all extern symbols were exported even when performing LTO. Now, we only export symbols that are also marked #[no_mangle]. Fixes rust-lang#34985
Only export #[no_mangle] extern symbols during LTO Fixes #34985
Right now the standard library ends up exporting a bunch of
extern
symbols from liblibc:These are all defined in the libc crate, but LTO should be stripping them out.
I believe the reason that this is happening is we attempt to preserve
extern
symbols from upstream crates (as they may be part of the ABI), but we don't check for#[no_mangle]
(which none of these functions have).I think the relevant clause is here which ends up calling this function coming over to this function. Although there's a clause which looks like it checks for this, I think we're falling through too quickly and we may wish to change that.
Note that this is mostly just a hunch though, some more investigation may be required! For an aspiring trans hacker though I don't think this should be too hard and I'd be willing to help out!
The text was updated successfully, but these errors were encountered: