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.

Sign up for GitHub

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

Issue error when -C link-self-contained option is used on unsupported platforms #104137

Merged
merged 1 commit into from
Nov 16, 2022
Merged

Issue error when -C link-self-contained option is used on unsupported platforms #104137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions compiler/rustc_codegen_ssa/src/back/link.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1588,6 +1588,9 @@ fn detect_self_contained_mingw(sess: &Session) -> bool {
/// We only provide such support for a very limited number of targets.
fn self_contained(sess: &Session, crate_type: CrateType) -> bool {
if let Some(self_contained) = sess.opts.cg.link_self_contained {
if sess.target.link_self_contained == LinkSelfContainedDefault::False {
sess.emit_err(errors::UnsupportedLinkSelfContained);
}
return self_contained;
}

Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_ssa/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,3 +530,7 @@ pub enum AppleSdkRootError<'a> {
pub struct ReadFileError {
pub message: std::io::Error,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_unsupported_link_self_contained)]
pub struct UnsupportedLinkSelfContained;
2 changes: 2 additions & 0 deletions compiler/rustc_error_messages/locales/en-US/codegen_ssa.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,3 +184,5 @@ codegen_ssa_unsupported_arch = unsupported arch `{$arch}` for os `{$os}`
codegen_ssa_apple_sdk_error_sdk_path = failed to get {$sdk_name} SDK path: {error}

codegen_ssa_read_file = failed to read file: {message}

codegen_ssa_unsupported_link_self_contained = option `-C link-self-contained` is not supported on this target
6 changes: 5 additions & 1 deletion compiler/rustc_target/src/spec/wasm32_wasi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@
//! best we can with this target. Don't start relying on too much here unless
//! you know what you're getting in to!

use super::{crt_objects, wasm_base, Cc, LinkerFlavor, Target};
use super::crt_objects::{self, LinkSelfContainedDefault};
use super::{wasm_base, Cc, LinkerFlavor, Target};

pub fn target() -> Target {
let mut options = wasm_base::options();
Expand All @@ -83,6 +84,9 @@ pub fn target() -> Target {
options.pre_link_objects_self_contained = crt_objects::pre_wasi_self_contained();
options.post_link_objects_self_contained = crt_objects::post_wasi_self_contained();

// FIXME: Figure out cases in which WASM needs to link with a native toolchain.
options.link_self_contained = LinkSelfContainedDefault::True;

// Right now this is a bit of a workaround but we're currently saying that
// the target by default has a static crt which we're taking as a signal
// for "use the bundled crt". If that's turned off then the system's crt
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_target/src/spec/wasm_base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use super::crt_objects::LinkSelfContainedDefault;
use super::{cvs, Cc, LinkerFlavor, PanicStrategy, RelocModel, TargetOptions, TlsModel};

pub fn options() -> TargetOptions {
Expand Down Expand Up @@ -95,9 +94,6 @@ pub fn options() -> TargetOptions {

pre_link_args,

// FIXME: Figure out cases in which WASM needs to link with a native toolchain.
link_self_contained: LinkSelfContainedDefault::True,
Copy link
Member

Choose a reason for hiding this comment

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

wasm32-unknown-unknown is always fully self contained. Both for linker and libraries. Why did you change it to no longer allow -Clink-self-contained?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was previously said that a self contained build was only supported for the wasi environment RELEASES.md, #104137 (comment)

Do we also support -Clink-self-contained for wasm32-unknown-emscripten?

Copy link
Member

@bjorn3 bjorn3 Nov 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

echo "fn main() {}" | rustc -Clink-self-contained=yes --target wasm32-unknown-unknown - works just fine. I don't think -Clink-self-contained=yes has any effect though as we never use any external things when linking. We use the wasm-ld we ship as linker and there is no libc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shall I set LinkSelfContainedDefault::True in wasm32_unknown_unknown.rs then?

Copy link
Member

Choose a reason for hiding this comment

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

I think so. Probably same for wasm64-unknown-unknown.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can roll back my last change then

Copy link
Member

Choose a reason for hiding this comment

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

Just as a heads up, it seems like -Clink-self-contained (especially its default value of "True" here) did have a meaningful (side-)effect for wasm32 (and presumably also wasm64): It caused the subsequent link step to include -nostartfiles, which we need on wasm32-unknown-unknown so that it will not attempt to link in crt1.o with other linkers like clang. See #108910 for more details.

Copy link
Member

Choose a reason for hiding this comment

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

(also, @cuviper has suggested on zulip that perhaps this example motivates a separate per-target flag for controlling whether we have crt objects.)

Copy link
Member

@pnkfelix pnkfelix Mar 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

(marking conversation unresolved to make this conversation easier to see while #108910 is open and others are skimming this to find connection with that issue.)


// This has no effect in LLVM 8 or prior, but in LLVM 9 and later when
// PIC code is implemented this has quite a drastic effect if it stays
// at the default, `pic`. In an effort to keep wasm binaries as minimal
Expand Down
4 changes: 2 additions & 2 deletions src/doc/rustc/src/codegen-options/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ metrics.

## link-self-contained

On targets that support it this flag controls whether the linker will use libraries and objects
shipped with Rust instead or those in the system.
On `windows-gnu`, `linux-musl`, and `wasi` targets, this flag controls whether the
linker will use libraries and objects shipped with Rust instead or those in the system.
It takes one of the following values:

* no value: rustc will use heuristic to disable self-contained mode if system has necessary tools.
Expand Down