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

Target-feature documented as unsafe #64145

Merged
merged 1 commit into from
Oct 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/doc/rustc/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
- [Targets](targets/index.md)
- [Built-in Targets](targets/built-in.md)
- [Custom Targets](targets/custom.md)
- [Known Issues](targets/known-issues.md)
- [Profile-guided Optimization](profile-guided-optimization.md)
- [Linker-plugin based LTO](linker-plugin-lto.md)
- [Contributing to `rustc`](contributing.md)
2 changes: 2 additions & 0 deletions src/doc/rustc/src/codegen-options/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ enabling or disabling a feature.
To see the valid options and an example of use, run `rustc --print
target-features`.

Using this flag is unsafe and might result in [undefined runtime behavior](../targets/known-issues.md).

## passes

This flag can be used to add extra LLVM passes to the compilation.
Expand Down
2 changes: 1 addition & 1 deletion src/doc/rustc/src/command-line-arguments.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ of print values are:
target CPU may be selected with the `-C target-cpu=val` flag.
- `target-features` — List of available target features for the current
target. Target features may be enabled with the `-C target-feature=val`
flag.
flag. This flag is unsafe. See [known issues](targets/known-issues.md) for more details.
- `relocation-models` — List of relocation models. Relocation models may be
selected with the `-C relocation-model=val` flag.
- `code-models` — List of code models. Code models may be selected with the
Expand Down
6 changes: 6 additions & 0 deletions src/doc/rustc/src/targets/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,9 @@ To compile to a particular target, use the `--target` flag:
```bash
$ rustc src/main.rs --target=wasm32-unknown-unknown
```
## Target Features
`x86`, and `ARMv8` are two popular CPU architectures. Their instruction sets form a common baseline across most CPUs. However, some CPUs extend these with custom instruction sets, e.g. vector (`AVX`), bitwise manipulation (`BMI`) or cryptographic (`AES`).

Developers, who know on which CPUs their compiled code is going to run can choose to add (or remove) CPU specific instruction sets via the `-C target-feature=val` flag.

Please note, that this flag is generally considered as unsafe. More details can be found in [this section](known-issues.md).
13 changes: 13 additions & 0 deletions src/doc/rustc/src/targets/known-issues.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Known Issues
This section informs you about known "gotchas". Keep in mind, that this section is (and always will be) incomplete. For suggestions and amendments, feel free to [contribute](../contributing.md) to this guide.

## Target Features
Copy link
Contributor

Choose a reason for hiding this comment

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

Why isn't this in the target_feature section? (i.e. why do we need a new section for this?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I understood, there are waay more issues, which are not documented yet.
Niko asked me to create a dedicated page, where all issues are listed.
The benefit is, that this vital info is not scattered/fragmented across many pages.

I am always open to suggestions :)

Most target-feature problems arise, when mixing code that have the target-feature _enabled_ with code that have it _disabled_. If you want to avoid undefined behavior, it is recommended to build _all code_ (including the standard library and imported crates) with a common set of target-features.

By default, compiling your code with the `-C target-feature` flag will not recompile the entire standard library and/or imported crates with matching target features. Therefore, target features are generally considered as unsafe. Using `#[target_feature]` on individual functions makes the function unsafe.

Examples:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how useful this would be. I don't think it is practical for us to document all features on all architectures that could cause problems when combined in different ways, and would very much prefer a solution to the problem that does not rely on documentation.

Right now Rust does not know much about how target-features interact with call ABI. It probably should.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're invited to wish for a better solution but we're not even generating shims for the more specific sub-problem of vector values and we've been suffering from that problem for the several years. Documenting that it is de facto unsafe now and underlining that with some scary examples is a good step forward. I agree that an exhaustive list is not practical but that's not necessary.

Copy link
Contributor

@gnzlbg gnzlbg Sep 17, 2019

Choose a reason for hiding this comment

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

@eddyb a quick workaround for this form of UB would be to pass f32 and f64 by memory in the Rust ABI, just like we do for Vector types.

This would be a temporary solution, until we can either diagnose these issues or generate appropriate shims. It might be horrible for performance of numeric code, I don't know.


| Target-Feature | Issue | Seen on | Description | Details |
| -------------- | ----- | ------- | ----------- | ------- |
| `+soft-float` <br> and <br> `-sse` | Segfaults and ABI mismatches | `x86` and `x86-64` | The `x86` and `x86_64` architecture uses SSE registers (aka `xmm`) for floating point operations. Using software emulated floats ("soft-floats") disables usage of `xmm` registers, but parts of Rust's core libraries (e.g. `std::f32` or `std::f64`) are compiled without soft-floats and expect parameters to be passed in `xmm` registers. This leads to ABI mismatches. <br><br> Attempting to compile with disabled SSE causes the same error, too. | [#63466](https://github.com/rust-lang/rust/issues/63466) |
3 changes: 2 additions & 1 deletion src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1149,7 +1149,8 @@ options! {CodegenOptions, CodegenSetter, basic_codegen_options,
target_cpu: Option<String> = (None, parse_opt_string, [TRACKED],
"select target processor (`rustc --print target-cpus` for details)"),
target_feature: String = (String::new(), parse_string, [TRACKED],
"target specific attributes (`rustc --print target-features` for details)"),
"target specific attributes. (`rustc --print target-features` for details). \
This feature is unsafe."),
passes: Vec<String> = (Vec::new(), parse_list, [TRACKED],
"a list of extra LLVM passes to run (space separated)"),
llvm_args: Vec<String> = (Vec::new(), parse_list, [TRACKED],
Expand Down