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

Rollup of 7 pull requests #136872

Closed
wants to merge 22 commits into from
Closed

Rollup of 7 pull requests #136872

wants to merge 22 commits into from

Conversation

fmease
Copy link
Member

@fmease fmease commented Feb 11, 2025

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

calebzulawski and others added 22 commits January 27, 2025 23:44
…e conversion functions

Having these implementation available crate-wide means that platforms not using sockets for their networking code have to stub out the libc definitions required to support them. This PR moves the conversions to private helper functions that are only available where actually needed.

I also fixed the signature of the function converting from a C socket address to a Rust one: taking a reference to a `sockaddr_storage` resulted in unsound usage inside  `LookupHost::next`, which could create a reference to a structure smaller than `sockaddr_storage`. Thus I've replaced the argument type with a pointer and made the function `unsafe`.
Co-authored-by: Ookiineko <chiisaineko@protonmail.com>
Co-authored-by: nora <48135649+Noratrieb@users.noreply.github.com>
Co-authored-by: Jubilee <workingjubilee@gmail.com>
Pointers for variables all need to be in the same address space for
correct compilation. Therefore ensure that even if an `alloca` is
created in a different address space, it is casted to the default
address space before its value is used.

This is necessary for the amdgpu target and others where the default
address space for `alloca`s is not 0.

For example the following code compiles incorrectly when not casting the
address space to the default one:

```rust
fn f(p: *const i8 /* addrspace(0) */) -> *const i8 /* addrspace(0) */ {
    let local = 0i8; /* addrspace(5) */
    let res = if cond { p } else { &raw const local };
    res
}
```

results in

```llvm
    %local = alloca addrspace(5) i8
    %res = alloca addrspace(5) ptr

if:
    ; Store 64-bit flat pointer
    store ptr %p, ptr addrspace(5) %res

else:
    ; Store 32-bit scratch pointer
    store ptr addrspace(5) %local, ptr addrspace(5) %res

ret:
    ; Load and return 64-bit flat pointer
    %res.load = load ptr, ptr addrspace(5) %res
    ret ptr %res.load
```

For amdgpu, `addrspace(0)` are 64-bit pointers, `addrspace(5)` are
32-bit pointers.
The above code may store a 32-bit pointer and read it back as a 64-bit
pointer, which is obviously wrong and cannot work. Instead, we need to
`addrspacecast %local to ptr addrspace(0)`, then we store and load the
correct type.
Stabilize target_feature_11

# Stabilization report

This is an updated version of rust-lang#116114, which is itself a redo of rust-lang#99767. Most of this commit and report were copied from those PRs. Thanks `@LeSeulArtichaut` and `@calebzulawski!`

## Summary
Allows for safe functions to be marked with `#[target_feature]` attributes.

Functions marked with `#[target_feature]` are generally considered as unsafe functions: they are unsafe to call, cannot *generally* be assigned to safe function pointers, and don't implement the `Fn*` traits.

However, calling them from other `#[target_feature]` functions with a superset of features is safe.

```rust
// Demonstration function
#[target_feature(enable = "avx2")]
fn avx2() {}

fn foo() {
    // Calling `avx2` here is unsafe, as we must ensure
    // that AVX is available first.
    unsafe {
        avx2();
    }
}

#[target_feature(enable = "avx2")]
fn bar() {
    // Calling `avx2` here is safe.
    avx2();
}
```

Moreover, once rust-lang#135504 is merged, they can be converted to safe function pointers in a context in which calling them is safe:

```rust
// Demonstration function
#[target_feature(enable = "avx2")]
fn avx2() {}

fn foo() -> fn() {
    // Converting `avx2` to fn() is a compilation error here.
    avx2
}

#[target_feature(enable = "avx2")]
fn bar() -> fn() {
    // `avx2` coerces to fn() here
    avx2
}
```

See the section "Closures" below for justification of this behaviour.

## Test cases
Tests for this feature can be found in [`tests/ui/target_feature/`](https://github.com/rust-lang/rust/tree/f6cb952dc115fd1311b02b694933e31d8dc8b002/tests/ui/target-feature).

## Edge cases
### Closures
 * [target-feature 1.1: should closures inherit target-feature annotations? rust-lang#73631](rust-lang#73631)

Closures defined inside functions marked with #[target_feature] inherit the target features of their parent function. They can still be assigned to safe function pointers and implement the appropriate `Fn*` traits.

```rust
#[target_feature(enable = "avx2")]
fn qux() {
    let my_closure = || avx2(); // this call to `avx2` is safe
    let f: fn() = my_closure;
}
```
This means that in order to call a function with #[target_feature], you must guarantee that the target-feature is available while the function, any closures defined inside it, as well as any safe function pointers obtained from target-feature functions inside it, execute.

This is usually ensured because target features are assumed to never disappear, and:
- on any unsafe call to a `#[target_feature]` function, presence of the target feature is guaranteed by the programmer through the safety requirements of the unsafe call.
- on any safe call, this is guaranteed recursively by the caller.

If you work in an environment where target features can be disabled, it is your responsibility to ensure that no code inside a target feature function (including inside a closure) runs after this (until the feature is enabled again).

**Note:** this has an effect on existing code, as nowadays closures do not inherit features from the enclosing function, and thus this strengthens a safety requirement. It was originally proposed in rust-lang#73631 to solve this by adding a new type of UB: “taking a target feature away from your process after having run code that uses that target feature is UB” .
This was motivated by userspace code already assuming in a few places that CPU features never disappear from a program during execution (see i.e. https://github.com/rust-lang/stdarch/blob/2e29bdf90832931ea499755bb4ad7a6b0809295a/crates/std_detect/src/detect/arch/x86.rs); however, concerns were raised in the context of the Linux kernel; thus, we propose to relax that requirement to "causing the set of usable features to be reduced is unsafe; when doing so, the programmer is required to ensure that no closures or safe fn pointers that use removed features are still in scope".

* [Fix #[inline(always)] on closures with target feature 1.1 rust-lang#111836](rust-lang#111836)

Closures accept `#[inline(always)]`, even within functions marked with `#[target_feature]`. Since these attributes conflict, `#[inline(always)]` wins out to maintain compatibility.

### ABI concerns
* [The extern "C" ABI of SIMD vector types depends on target features rust-lang#116558](rust-lang#116558)

The ABI of some types can change when compiling a function with different target features. This could have introduced unsoundness with target_feature_11, but recent fixes (rust-lang#133102, rust-lang#132173) either make those situations invalid or make the ABI no longer dependent on features. Thus, those issues should no longer occur.

### Special functions
The `#[target_feature]` attribute is forbidden from a variety of special functions, such as main, current and future lang items (e.g. `#[start]`, `#[panic_handler]`), safe default trait implementations and safe trait methods.

This was not disallowed at the time of the first stabilization PR for target_features_11, and resulted in the following issues/PRs:
* [`#[target_feature]` is allowed on `main` rust-lang#108645](rust-lang#108645)
* [`#[target_feature]` is allowed on default implementations rust-lang#108646](rust-lang#108646)
* [#[target_feature] is allowed on #[panic_handler] with target_feature 1.1 rust-lang#109411](rust-lang#109411)
* [Prevent using `#[target_feature]` on lang item functions rust-lang#115910](rust-lang#115910)

## Documentation
 * Reference: [Document the `target_feature_11` feature reference#1181](rust-lang/reference#1181)
---

cc tracking issue rust-lang#69098
cc `@workingjubilee`
cc `@RalfJung`
r? `@rust-lang/lang`
…=chenyukang,workingjubilee

Add cygwin target.

This PR simply adds cygwin target together with msys2 target, based on ``@ookiineko`` 's (the account has been deleted) [work](https://github.com/ookiineko-cygport/rust) on cygwin target. My full work is here: rust-lang/rust@master...Berrysoft:rust:dev/cygwin

I have succeeded in building a new rustc for cygwin target, and eventually distributed a new version of [fish-shell](https://github.com/Berrysoft/fish-shell/releases) (rewritten by Rust) for MSYS2.

I will open a new PR to fix std if this PR is accepted.
Cast allocas to default address space

Pointers for variables all need to be in the same address space for correct compilation. Therefore ensure that even if an `alloca` is created in a different address space, it is casted to the default address space before its value is used.

This is necessary for the amdgpu target and others where the default address space for `alloca`s is not 0.

For example the following code compiles incorrectly when not casting the address space to the default one:

```rust
fn f(p: *const i8 /* addrspace(0) */) -> *const i8 /* addrspace(0) */ {
    let local = 0i8; /* addrspace(5) */
    let res = if cond { p } else { &raw const local };
    res
}
```

results in

```llvm
    %local = alloca addrspace(5) i8
    %res = alloca addrspace(5) ptr

if:
    ; Store 64-bit flat pointer
    store ptr %p, ptr addrspace(5) %res

else:
    ; Store 32-bit scratch pointer
    store ptr addrspace(5) %local, ptr addrspace(5) %res

ret:
    ; Load and return 64-bit flat pointer
    %res.load = load ptr, ptr addrspace(5) %res
    ret ptr %res.load
```

For amdgpu, `addrspace(0)` are 64-bit pointers, `addrspace(5)` are 32-bit pointers.
The above code may store a 32-bit pointer and read it back as a 64-bit pointer, which is obviously wrong and cannot work. Instead, we need to `addrspacecast %local to ptr addrspace(0)`, then we store and load the correct type.

Tracking issue: rust-lang#135024
x86: make SSE2 required for i686 hardfloat targets and use it to pass SIMD types

The primary goal of this is to make SSE2 *required* for our i686 targets (at least for the ones that use Pentium 4 as their baseline), to ensure they cannot be affected by rust-lang#114479. This has been MCPd in rust-lang/compiler-team#808, and is tracked in rust-lang#133611.

We do this by defining a new ABI that these targets select, and making SSE2 required by the ABI (that's the first commit). That's kind of a hack, but (a) it is the easiest way to make a target feature required via the target spec, and (b) we actually *can* use SSE2 for the Rust ABI now that it is required, so the second commit goes ahead and does that. Specifically, we use it in two ways: to return `f64` values in a register rather than by-ptr, and to pass vectors of size up to 128bit in a register (or, well, whatever LLVM does when passing `<4 x float>` by-val, I don't actually know if this ends up in a register).

Cc `@workingjubilee`
Fixes rust-lang#133611
…ler-errors

Document some safety constraints and use more safe wrappers

Lots of unsafe codegen_llvm code has safe wrappers already, so I used some of them and added some where applicable. I stopped here because this diff is large enough and should probably be reviewed independently of other changes.
…=chenyukang

Implement pattern type ffi checks

Previously we just rejected pattern types outright in FFI, but that was never meant to be a permanent situation. We'll need them supported to use them as the building block for `NonZero` and `NonNull` after all (both of which are FFI safe).

best reviewed commit by commit.
std: replace the `FromInner` implementation for addresses with private conversion functions

Having these implementation available crate-wide means that platforms not using sockets for their networking code have to stub out the libc definitions required to support them. This PR moves the conversions to private helper functions that are only available where actually needed.

I also fixed the signature of the function converting from a C socket address to a Rust one: taking a reference to a `sockaddr_storage` resulted in unsound usage inside  `LookupHost::next`, which could create a reference to a structure smaller than `sockaddr_storage`. Thus I've replaced the argument type with a pointer and made the function `unsafe`.
@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) O-solid Operating System: SOLID S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Feb 11, 2025
@fmease
Copy link
Member Author

fmease commented Feb 11, 2025

@bors rollup=never p=5 r+

@bors
Copy link
Contributor

bors commented Feb 11, 2025

📌 Commit bd0a316 has been approved by fmease

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 11, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 11, 2025
Rollup of 7 pull requests

Successful merges:

 - rust-lang#134090 (Stabilize target_feature_11)
 - rust-lang#134999 (Add cygwin target.)
 - rust-lang#135025 (Cast allocas to default address space)
 - rust-lang#135408 (x86: make SSE2 required for i686 hardfloat targets and use it to pass SIMD types)
 - rust-lang#135549 (Document some safety constraints and use more safe wrappers)
 - rust-lang#136193 (Implement pattern type ffi checks)
 - rust-lang#136699 (std: replace the `FromInner` implementation for addresses with private conversion functions)

Failed merges:

 - rust-lang#136758 (tests: `-Copt-level=3` instead of `-O` in assembly tests)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Feb 11, 2025

⌛ Testing commit bd0a316 with merge d4307b4...

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-nopt failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
test [ui] tests/ui/target-feature/implied-features.rs ... ok
test [ui] tests/ui/target-feature/no-llvm-leaks.rs#aarch64 ... ok
test [ui] tests/ui/target-feature/no-llvm-leaks.rs#x86-64 ... ok
test [ui] tests/ui/target-feature/similar-feature-suggestion.rs ... ok
test [ui] tests/ui/target-feature/target-cpu-lacks-required-target-feature.rs ... ok
test [ui] tests/ui/target-feature/tied-features-cli.rs#one ... ok
test [ui] tests/ui/target-feature/tied-features-cli.rs#three ... ok
test [ui] tests/ui/target-feature/tied-features-cli.rs#two ... ok
test [ui] tests/ui/target-feature/tied-features-no-implication-1.rs#paca ... ok
---
test [codegen] tests/codegen/abi-win64-zst.rs#linux ... ok
test [codegen] tests/codegen/abi-win64-zst.rs#windows-msvc ... ok
test [codegen] tests/codegen/abi-x86-interrupt.rs ... ok
test [codegen] tests/codegen/abi-win64-zst.rs#windows-gnu ... ok
test [codegen] tests/codegen/abi-x86-sse.rs#x86-32 ... ok
test [codegen] tests/codegen/abi-x86-sse.rs#x86-64 ... ok
test [codegen] tests/codegen/abi-x86-sse.rs#x86-32-nosse ... FAILED
test [codegen] tests/codegen/abi-x86_64_sysv.rs ... ok
test [codegen] tests/codegen/adjustments.rs ... ok
test [codegen] tests/codegen/align-byval-alignment-mismatch.rs#i686-linux ... ok
test [codegen] tests/codegen/align-byval-alignment-mismatch.rs#x86_64-linux ... ok
---
test [codegen] tests/codegen/zip.rs ... ok

failures:

---- [codegen] tests/codegen/abi-x86-sse.rs#x86-32-nosse stdout ----

error in revision `x86-32-nosse`: verification with 'FileCheck' failed
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/ci-llvm/bin/FileCheck" "--input-file" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/abi-x86-sse.x86-32-nosse/abi-x86-sse.ll" "/checkout/tests/codegen/abi-x86-sse.rs" "--check-prefix=CHECK" "--check-prefix" "x86-32-nosse" "--allow-unused-prefixes" "--dump-input-context" "100"
--- stderr -------------------------------
--- stderr -------------------------------
/checkout/tests/codegen/abi-x86-sse.rs:32:18: error: x86-32-nosse: expected string not found in input
// x86-32-nosse: void @sse_id(ptr {{[^,]*}} sret{{[^,]*}}, ptr {{[^,]*}})
                 ^
/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/abi-x86-sse.x86-32-nosse/abi-x86-sse.ll:1:1: note: scanning from here
; ModuleID = 'abi_x86_sse.330e7dc655e63bbb-cgu.0'
^
/checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/abi-x86-sse.x86-32-nosse/abi-x86-sse.ll:7:8: note: possible intended match here
define void @sse_id(ptr sret([16 x i8]) align 16 %_0, ptr align 16 %x) unnamed_addr #0 {


Input file: /checkout/obj/build/x86_64-unknown-linux-gnu/test/codegen/abi-x86-sse.x86-32-nosse/abi-x86-sse.ll
Check file: /checkout/tests/codegen/abi-x86-sse.rs

-dump-input=help explains the following input dump.
Input was:
<<<<<<
<<<<<<
            1: ; ModuleID = 'abi_x86_sse.330e7dc655e63bbb-cgu.0' 
check:32'0     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
            2: source_filename = "abi_x86_sse.330e7dc655e63bbb-cgu.0" 
check:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            3: target datalayout = "e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-i128:128-f64:32:64-f80:32-n8:16:32-S128" 
check:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            5:  
check:32'0     ~
            6: ; Function Attrs: uwtable 
            6: ; Function Attrs: uwtable 
check:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~
            7: define void @sse_id(ptr sret([16 x i8]) align 16 %_0, ptr align 16 %x) unnamed_addr #0 { 
check:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:32'1            ?                                                                                  possible intended match
check:32'0     ~~~~~~~
check:32'0     ~~~~~~~
            9:  %0 = load <4 x float>, ptr %x, align 16 
check:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           10:  store <4 x float> %0, ptr %_0, align 16 
check:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           11:  ret void 
check:32'0     ~~~~~~~~~~
           12: } 
check:32'0     ~~
check:32'0     ~
check:32'0     ~
           14: attributes #0 = { uwtable "probe-stack"="inline-asm" "target-cpu"="pentium" } 
check:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
check:32'0     ~
check:32'0     ~
           16: !llvm.module.flags = !{!0} 
check:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~
           17: !llvm.ident = !{!1} 
check:32'0     ~~~~~~~~~~~~~~~~~~~~
check:32'0     ~
check:32'0     ~
           19: !0 = !{i32 8, !"PIC Level", i32 2} 
check:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           20: !1 = !{!"rustc version 1.86.0-nightly (d4307b44d 2025-02-11)"} 
check:32'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
------------------------------------------



@bors
Copy link
Contributor

bors commented Feb 11, 2025

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 11, 2025
@fmease
Copy link
Member Author

fmease commented Feb 11, 2025

@bors r-

@fmease fmease closed this Feb 11, 2025
@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 11, 2025
@fmease fmease deleted the rollup-12c4v1x branch February 11, 2025 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) O-solid Operating System: SOLID rollup A PR which is a rollup S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants