Skip to content

Add safety doc for aarch64/mte #2243

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

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

Add safety doc for aarch64/mte #2243

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
52 changes: 45 additions & 7 deletions crates/core_arch/src/aarch64/mte.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,16 @@ unsafe extern "llvm-intrinsic" {
/// The returned pointer contains a copy of the `src` address, but with a
/// randomly generated logical tag, excluding any specified by `mask`.

@adamgemmell adamgemmell Oct 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One issue with these intrinsics is that while the hardware may consider identical pointers with different tags dereferenceable to the same address when TBI is enabled, the Rust memory model does not without rust-lang/rfcs#3700. I think it would be a good idea to draw attention to the fact that at the moment Rust will consider these pointers as pointing to different allocations.

@yilin0518 yilin0518 Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your patience and review. I didn't read this rfc before so I misunderstand that Rust memory model will treat the logical tag approariately.

///
/// SAFETY: The pointer provided by this intrinsic will be invalid until the memory
/// has been appropriately tagged with `__arm_mte_set_tag`. If using that intrinsic
/// on the provided pointer is itself invalid, then it will be permanently invalid
/// and Undefined Behavior to dereference it.
/// This intrinsic does not access memory or change allocation tags.
/// The allocation tag can be changed using [`__arm_mte_set_tag`],
/// provided that function's safety requirements are satisfied.

@adamgemmell adamgemmell Oct 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pointer arithmetic can still be unsafe as overflow is not defined for them, see https://doc.rust-lang.org/core/primitive.pointer.html#method.add

Granted if you're just changing tags I'm struggling to come up with an example where this can make sound instances of pointer arithmetic unsound.

@yilin0518 yilin0518 Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In my intuition I think that the internal irg_ directly return a pointer. So should it enforce the pointer overflow rule?

Also, the size of pointer is the same as usize (if the type is Sized). I think the returned pointer probably doe not happen overflow, because it just return a pointer whose size is the same as usize.

///
/// If the returned pointer's logical tag does not match the allocation
/// tag of the memory, then it is invalid and Undefined Behavior to dereference it.
///
/// # Safety
///
/// The current platform must support the `mte` target feature.

@adamgemmell adamgemmell Oct 12, 2026

Copy link
Copy Markdown
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 if the target feature requirement needs to be documented as it's more a property of the language than the function implementation. The target feature attribute should already be rendered in documentation.

@yilin0518 yilin0518 Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your review. I understnd your opinion, and I think that target feature doe not need to be documented. I will update this PR.

But I think that since these APIs are marked unsafe, it's better to document these APIs and give them concrete requirement.

#[inline]
#[target_feature(enable = "mte")]
#[unstable(feature = "stdarch_aarch64_mte", issue = "129010")]
Expand All @@ -46,11 +52,21 @@ pub unsafe fn __arm_mte_create_random_tag<T>(src: *const T, mask: u64) -> *const
/// Adds offset to the logical address tag in `src`, wrapping if the result is
/// outside of the valid 16 tags.
///
/// SAFETY: See `__arm_mte_create_random_tag`.
/// This intrinsic does not access memory or change allocation tags.
/// The allocation tag can be changed using [`__arm_mte_set_tag`],
/// provided that function's safety requirements are satisfied.
///
/// If the returned pointer's logical tag does not match the allocation
/// tag of the memory, then it is invalid and Undefined Behavior to dereference it.
///
/// # Safety
///
/// The current platform must support the `mte` target feature.
#[inline]
#[target_feature(enable = "mte")]
#[unstable(feature = "stdarch_aarch64_mte", issue = "129010")]
pub unsafe fn __arm_mte_increment_tag<const OFFSET: i64, T>(src: *const T) -> *const T {
static_assert_uimm_bits!(OFFSET, 4);
addg_(src as *const (), OFFSET) as *const T
}

Expand All @@ -62,6 +78,12 @@ pub unsafe fn __arm_mte_increment_tag<const OFFSET: i64, T>(src: *const T) -> *c
///
/// Adds the logical tag stored in `src` to the set in `excluded`, and returns
/// the result.
///
/// This intrinsic does not access memory or change allocation tags.
///
/// # Safety
///
/// The current platform must support the `mte` target feature.
#[inline]
#[target_feature(enable = "mte")]
#[unstable(feature = "stdarch_aarch64_mte", issue = "129010")]
Expand All @@ -74,8 +96,14 @@ pub unsafe fn __arm_mte_exclude_tag<T>(src: *const T, excluded: u64) -> u64 {
/// `tag_address`: A pointer containing an address and a logical tag, which
/// must be 16-byte aligned.
///
/// SAFETY: `tag_address` must be 16-byte aligned. The tag will apply to the
/// entire 16-byte memory granule.
/// This changes only the allocation tag. The size of `T` does not
/// affect the size of the tagged region.
///
/// # Safety
///
/// * The current platform must support the `mte` target feature.
/// * `tag_address` must be 16-byte aligned. The tag will apply to the
/// entire 16-byte memory granule.

@adamgemmell adamgemmell Oct 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

According to the ACLE "These intrinsics generate an unchecked access to memory" - sic. presumably as it seems like the comment only refers to this intrinsic. Does this mean tag_address needs to be dereferenceable?

@yilin0518 yilin0518 Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this API doe not dereference the tag_address, and the actual allocated memory doe not be accessed.

#[inline]
#[target_feature(enable = "mte")]
#[unstable(feature = "stdarch_aarch64_mte", issue = "129010")]
Expand All @@ -88,6 +116,12 @@ pub unsafe fn __arm_mte_set_tag<T>(tag_address: *const T) {
///
/// `address`: A pointer containing an address from which allocation tag memory
/// is read. This does not need to be 16-byte aligned.
///
/// # Safety
///
/// * The current platform must support the `mte` target feature.
/// * The address part of `address` must identify memory whose allocation

@adamgemmell adamgemmell Oct 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What are the specific constraints on the address that must be upheld here? Can any address be used, or must the memory previously have a tag set, or is this unique to the implementation of the allocator?

@yilin0518 yilin0518 Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To be honest I don't know how internal ldg_ works. I don't know whether ldg_ works the same when address points at a real allocated memory or a memory that has not been allocated (in this case the allocation tag is 0). If ldg_ works the same, I think the requirement of address is that the address part of address must point at a real memory.

/// tag will be read.
#[inline]
#[target_feature(enable = "mte")]
#[unstable(feature = "stdarch_aarch64_mte", issue = "129010")]
Expand All @@ -97,6 +131,10 @@ pub unsafe fn __arm_mte_get_tag<T>(address: *const T) -> *const T {

/// Calculate the difference between the address parts of two pointers, ignoring
/// the tags, and sign-extending the result.
///
/// # Safety
///
/// The current platform must support the `mte` target feature.
#[inline]
#[target_feature(enable = "mte")]
#[unstable(feature = "stdarch_aarch64_mte", issue = "129010")]
Expand Down
Loading