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

Deny unsafe_op_in_unsafe_fn by default #801

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Deny unsafe_op_in_unsafe_fn by default #801

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
20 changes: 10 additions & 10 deletions builtins-test-intrinsics/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,14 +647,14 @@ fn something_with_a_dtor(f: &dyn Fn()) {
f();
}

#[no_mangle]
#[unsafe(no_mangle)]
#[cfg(not(thumb))]
fn main(_argc: core::ffi::c_int, _argv: *const *const u8) -> core::ffi::c_int {
run();
0
}

#[no_mangle]
#[unsafe(no_mangle)]
#[cfg(thumb)]
pub fn _start() -> ! {
run();
Expand All @@ -667,30 +667,30 @@ pub fn _start() -> ! {
extern "C" {}

// ARM targets need these symbols
#[no_mangle]
#[unsafe(no_mangle)]
pub fn __aeabi_unwind_cpp_pr0() {}

#[no_mangle]
#[unsafe(no_mangle)]
pub fn __aeabi_unwind_cpp_pr1() {}

#[cfg(not(any(windows, target_os = "cygwin")))]
#[allow(non_snake_case)]
#[no_mangle]
#[unsafe(no_mangle)]
pub fn _Unwind_Resume() {}

#[cfg(not(any(windows, target_os = "cygwin")))]
#[lang = "eh_personality"]
#[no_mangle]
#[unsafe(no_mangle)]
pub extern "C" fn eh_personality() {}

#[cfg(any(all(windows, target_env = "gnu"), target_os = "cygwin"))]
mod mingw_unwinding {
#[no_mangle]
#[unsafe(no_mangle)]
pub fn rust_eh_personality() {}
#[no_mangle]
#[unsafe(no_mangle)]
pub fn rust_eh_unwind_resume() {}
#[no_mangle]
#[unsafe(no_mangle)]
pub fn rust_eh_register_frames() {}
#[no_mangle]
#[unsafe(no_mangle)]
pub fn rust_eh_unregister_frames() {}
}
262 changes: 190 additions & 72 deletions src/arm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,143 +23,261 @@ intrinsics! {
#[naked]
#[cfg(not(target_env = "msvc"))]
pub unsafe extern "C" fn __aeabi_uidivmod() {
core::arch::naked_asm!(
"push {{lr}}",
"sub sp, sp, #4",
"mov r2, sp",
bl!("__udivmodsi4"),
"ldr r1, [sp]",
"add sp, sp, #4",
"pop {{pc}}",
);
unsafe {
core::arch::naked_asm!(
"push {{lr}}",
"sub sp, sp, #4",
"mov r2, sp",
bl!("__udivmodsi4"),
"ldr r1, [sp]",
"add sp, sp, #4",
"pop {{pc}}",
);
}
}

#[naked]
pub unsafe extern "C" fn __aeabi_uldivmod() {
core::arch::naked_asm!(
"push {{r4, lr}}",
"sub sp, sp, #16",
"add r4, sp, #8",
"str r4, [sp]",
bl!("__udivmoddi4"),
"ldr r2, [sp, #8]",
"ldr r3, [sp, #12]",
"add sp, sp, #16",
"pop {{r4, pc}}",
);
unsafe {
core::arch::naked_asm!(
"push {{r4, lr}}",
"sub sp, sp, #16",
"add r4, sp, #8",
"str r4, [sp]",
bl!("__udivmoddi4"),
"ldr r2, [sp, #8]",
"ldr r3, [sp, #12]",
"add sp, sp, #16",
"pop {{r4, pc}}",
);
}
}

#[naked]
pub unsafe extern "C" fn __aeabi_idivmod() {
core::arch::naked_asm!(
"push {{r0, r1, r4, lr}}",
bl!("__aeabi_idiv"),
"pop {{r1, r2}}",
"muls r2, r2, r0",
"subs r1, r1, r2",
"pop {{r4, pc}}",
);
unsafe {
core::arch::naked_asm!(
"push {{r0, r1, r4, lr}}",
bl!("__aeabi_idiv"),
"pop {{r1, r2}}",
"muls r2, r2, r0",
"subs r1, r1, r2",
"pop {{r4, pc}}",
);
}
}

#[naked]
pub unsafe extern "C" fn __aeabi_ldivmod() {
core::arch::naked_asm!(
"push {{r4, lr}}",
"sub sp, sp, #16",
"add r4, sp, #8",
"str r4, [sp]",
bl!("__divmoddi4"),
"ldr r2, [sp, #8]",
"ldr r3, [sp, #12]",
"add sp, sp, #16",
"pop {{r4, pc}}",
);
unsafe {
core::arch::naked_asm!(
"push {{r4, lr}}",
"sub sp, sp, #16",
"add r4, sp, #8",
"str r4, [sp]",
bl!("__divmoddi4"),
"ldr r2, [sp, #8]",
"ldr r3, [sp, #12]",
"add sp, sp, #16",
"pop {{r4, pc}}",
);
}
}

// FIXME: The `*4` and `*8` variants should be defined as aliases.
// FIXME(arm): The `*4` and `*8` variants should be defined as aliases.

/// `memcpy` provided with the `aapcs` ABI.
///
/// # Safety
///
/// Usual `memcpy` requirements apply.
#[cfg(not(target_vendor = "apple"))]
pub unsafe extern "aapcs" fn __aeabi_memcpy(dest: *mut u8, src: *const u8, n: usize) {
crate::mem::memcpy(dest, src, n);
pub unsafe extern "aapcs" fn __aeabi_memcpy(dst: *mut u8, src: *const u8, n: usize) {
// SAFETY: memcpy preconditions apply.
unsafe { crate::mem::memcpy(dst, src, n) };
}

/// `memcpy` for 4-byte alignment.
///
/// # Safety
///
/// Usual `memcpy` requirements apply. Additionally, `dest` and `src` must be aligned to
/// four bytes.
#[cfg(not(target_vendor = "apple"))]
pub unsafe extern "aapcs" fn __aeabi_memcpy4(dest: *mut u8, src: *const u8, n: usize) {
pub unsafe extern "aapcs" fn __aeabi_memcpy4(dst: *mut u8, src: *const u8, n: usize) {
// We are guaranteed 4-alignment, so accessing at u32 is okay.
let mut dest = dest as *mut u32;
let mut src = src as *mut u32;
let mut dst = dst.cast::<u32>();
let mut src = src.cast::<u32>();
debug_assert!(dst.is_aligned());
debug_assert!(src.is_aligned());
let mut n = n;

while n >= 4 {
*dest = *src;
dest = dest.offset(1);
src = src.offset(1);
// SAFETY: `dst` and `src` are both valid for at least 4 bytes, from
// `memcpy` preconditions and the loop guard.
unsafe { *dst = *src };

// TODO
unsafe {
dst = dst.offset(1);
src = src.offset(1);
}
Comment on lines +120 to +124
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RalfJung is using offset to create a pointer one past the end of the allocation allowed? This caught my eye when adding safety comments; the docs say "the entire memory range between self and the result must be in bounds of that allocated object" and Miri doesn't complain, but I had been thinking that it was at least indeterminate (and now I can't find the issue).

Copy link
Member

@RalfJung RalfJung Mar 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If an allocation has size 4, and a ptr starts at offset 0 and you add 4 to it, then the memory range between the old and new pointer has size 4. Clearly that is entirely inbounds. Not sure what you mean by "indeterminate" here -- we don't use that word in Rust, and in C it is used to refer to values/representations, not operations, so I can't make sense of it here.

I think "one past the end" is bad terminology, since the pointer is not past the end, it is at the end -- it points after the last element.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's straightforward enough. I must be misremembering; I thought there was an open issue somewhere related to differences between Rust and C here regarding creating a pointer past (or at) the end of an allocation and how that related to memcpy, but maybe this was only about wrapping at the end of the address space.

Not sure what you mean by "indeterminate" here -- we don't use that word in Rust

Indeterminate only because it had not yet been decided, not technical terminology.

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'm probably thinking of rust-lang/unsafe-code-guidelines#465 and related discussion, which has been decided and I'm just a year out of date :) So as I understand it, a Rust program can never access the last byte of the address space and that effectively means our memcpy doesn't have to worry about it - with the result that these offset calculations will never wrap.


n -= 4;
}

__aeabi_memcpy(dest as *mut u8, src as *const u8, n);
// SAFETY: `dst` and `src` will still be valid for `n` bytes
unsafe { __aeabi_memcpy(dst.cast::<u8>(), src.cast::<u8>(), n) };
}

/// `memcpy` for 8-byte alignment.
///
/// # Safety
///
/// Usual `memcpy` requirements apply. Additionally, `dest` and `src` must be aligned to
/// eight bytes.
#[cfg(not(target_vendor = "apple"))]
pub unsafe extern "aapcs" fn __aeabi_memcpy8(dest: *mut u8, src: *const u8, n: usize) {
__aeabi_memcpy4(dest, src, n);
pub unsafe extern "aapcs" fn __aeabi_memcpy8(dst: *mut u8, src: *const u8, n: usize) {
debug_assert!(dst.addr() & 7 == 0);
debug_assert!(src.addr() & 7 == 0);

// SAFETY: memcpy preconditions apply, less strict alignment.
unsafe { __aeabi_memcpy4(dst, src, n) };
}

/// `memmove` provided with the `aapcs` ABI.
///
/// # Safety
///
/// Usual `memmove` requirements apply.
#[cfg(not(target_vendor = "apple"))]
pub unsafe extern "aapcs" fn __aeabi_memmove(dest: *mut u8, src: *const u8, n: usize) {
crate::mem::memmove(dest, src, n);
pub unsafe extern "aapcs" fn __aeabi_memmove(dst: *mut u8, src: *const u8, n: usize) {
// SAFETY: memmove preconditions apply.
unsafe { crate::mem::memmove(dst, src, n) };
}

/// `memmove` for 4-byte alignment.
///
/// # Safety
///
/// Usual `memmove` requirements apply. Additionally, `dest` and `src` must be aligned to
/// four bytes.
#[cfg(not(any(target_vendor = "apple", target_env = "msvc")))]
pub unsafe extern "aapcs" fn __aeabi_memmove4(dest: *mut u8, src: *const u8, n: usize) {
__aeabi_memmove(dest, src, n);
pub unsafe extern "aapcs" fn __aeabi_memmove4(dst: *mut u8, src: *const u8, n: usize) {
debug_assert!(dst.addr() & 3 == 0);
debug_assert!(src.addr() & 3 == 0);

// SAFETY: same preconditions, less strict aligment.
unsafe { __aeabi_memmove(dst, src, n) };
}

/// `memmove` for 8-byte alignment.
///
/// # Safety
///
/// Usual `memmove` requirements apply. Additionally, `dst` and `src` must be aligned to
/// eight bytes.
#[cfg(not(any(target_vendor = "apple", target_env = "msvc")))]
pub unsafe extern "aapcs" fn __aeabi_memmove8(dest: *mut u8, src: *const u8, n: usize) {
__aeabi_memmove(dest, src, n);
pub unsafe extern "aapcs" fn __aeabi_memmove8(dst: *mut u8, src: *const u8, n: usize) {
debug_assert!(dst.addr() & 7 == 0);
debug_assert!(src.addr() & 7 == 0);

// SAFETY: memmove preconditions apply, less strict alignment.
unsafe { __aeabi_memmove(dst, src, n) };
}

/// `memset` provided with the `aapcs` ABI.
///
/// # Safety
///
/// Usual `memset` requirements apply.
#[cfg(not(target_vendor = "apple"))]
pub unsafe extern "aapcs" fn __aeabi_memset(dest: *mut u8, n: usize, c: i32) {
pub unsafe extern "aapcs" fn __aeabi_memset(dst: *mut u8, n: usize, c: i32) {
// Note the different argument order
crate::mem::memset(dest, c, n);
// SAFETY: memset preconditions apply.
unsafe { crate::mem::memset(dst, c, n) };
}

/// `memset` for 4-byte alignment.
///
/// # Safety
///
/// Usual `memset` requirements apply. Additionally, `dest` and `src` must be aligned to
/// four bytes.
#[cfg(not(target_vendor = "apple"))]
pub unsafe extern "aapcs" fn __aeabi_memset4(dest: *mut u8, n: usize, c: i32) {
let mut dest = dest as *mut u32;
pub unsafe extern "aapcs" fn __aeabi_memset4(dst: *mut u8, n: usize, c: i32) {
let mut dst = dst.cast::<u32>();
debug_assert!(dst.is_aligned());
let mut n = n;

let byte = (c as u32) & 0xff;
let c = (byte << 24) | (byte << 16) | (byte << 8) | byte;

while n >= 4 {
*dest = c;
dest = dest.offset(1);
// SAFETY: `dst` is valid for at least 4 bytes, from `memset` preconditions and
// the loop guard.
unsafe { *dst = c };
// TODO
unsafe {
dst = dst.offset(1);
}
n -= 4;
}

__aeabi_memset(dest as *mut u8, n, byte as i32);
// SAFETY: `dst` will still be valid for `n` bytes
unsafe { __aeabi_memset(dst.cast::<u8>(), n, byte as i32) };
}

/// `memset` for 8-byte alignment.
///
/// # Safety
///
/// Usual `memset` requirements apply. Additionally, `dst` and `src` must be aligned to
/// eight bytes.
#[cfg(not(target_vendor = "apple"))]
pub unsafe extern "aapcs" fn __aeabi_memset8(dest: *mut u8, n: usize, c: i32) {
__aeabi_memset4(dest, n, c);
pub unsafe extern "aapcs" fn __aeabi_memset8(dst: *mut u8, n: usize, c: i32) {
debug_assert!(dst.addr() & 7 == 0);

// SAFETY: memset preconditions apply, less strict alignment.
unsafe { __aeabi_memset4(dst, n, c) };
}

/// `memclr` provided with the `aapcs` ABI.
///
/// # Safety
///
/// Usual `memclr` requirements apply.
#[cfg(not(target_vendor = "apple"))]
pub unsafe extern "aapcs" fn __aeabi_memclr(dest: *mut u8, n: usize) {
__aeabi_memset(dest, n, 0);
pub unsafe extern "aapcs" fn __aeabi_memclr(dst: *mut u8, n: usize) {
// SAFETY: memclr preconditions apply, less strict alignment.
unsafe { __aeabi_memset(dst, n, 0) };
}

/// `memclr` for 4-byte alignment.
///
/// # Safety
///
/// Usual `memclr` requirements apply. Additionally, `dest` and `src` must be aligned to
/// four bytes.
#[cfg(not(any(target_vendor = "apple", target_env = "msvc")))]
pub unsafe extern "aapcs" fn __aeabi_memclr4(dest: *mut u8, n: usize) {
__aeabi_memset4(dest, n, 0);
pub unsafe extern "aapcs" fn __aeabi_memclr4(dst: *mut u8, n: usize) {
debug_assert!(dst.addr() & 3 == 0);

// SAFETY: memclr preconditions apply, less strict alignment.
unsafe { __aeabi_memset4(dst, n, 0) };
}

/// `memclr` for 8-byte alignment.
///
/// # Safety
///
/// Usual `memclr` requirements apply. Additionally, `dst` and `src` must be aligned to
/// eight bytes.
#[cfg(not(any(target_vendor = "apple", target_env = "msvc")))]
pub unsafe extern "aapcs" fn __aeabi_memclr8(dest: *mut u8, n: usize) {
__aeabi_memset4(dest, n, 0);
pub unsafe extern "aapcs" fn __aeabi_memclr8(dst: *mut u8, n: usize) {
debug_assert!(dst.addr() & 7 == 0);

// SAFETY: memclr preconditions apply, less strict alignment.
unsafe { __aeabi_memset4(dst, n, 0) };
}
}
3 changes: 3 additions & 0 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
#![allow(clippy::manual_swap)]
// Support compiling on both stage0 and stage1 which may differ in supported stable features.
#![allow(stable_features)]
// By default, disallow this as it is forbidden in edition 2024. There is a lot of unsafe code to
// be migrated, however, so exceptions exist.
#![warn(unsafe_op_in_unsafe_fn)]

// We disable #[no_mangle] for tests so that we can verify the test results
// against the native compiler-rt implementations of the builtins.
Expand Down
Loading
Loading