Implement `#[rustc_align_static(N)]` on `static`s #146178

folkertdev · 2025-09-03T20:56:42Z

Tracking issue: #146177

#![feature(static_align)]

#[rustc_align_static(64)]
static SO_ALIGNED: u64 = 0;

We need a different attribute than rustc_align because unstable attributes are tied to their feature (we can't have two unstable features use the same unstable attribute). Otherwise this uses all of the same infrastructure as #[rustc_align].

r? @traviscross

rustbot · 2025-09-03T20:56:48Z

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

RalfJung · 2025-09-03T21:13:33Z

we can't have two unstable features use the same unstable attribute

What do you mean by "use"? How does a feature "use" an attribute?
All rustc_ attributes are unstable anyway...

compiler/rustc_const_eval/src/interpret/memory.rs

compiler/rustc_codegen_gcc/src/consts.rs

folkertdev · 2025-09-03T21:16:31Z

compiler/rustc_feature/src/builtin_attrs.rs

@@ -621,6 +621,7 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
    ),
    // FIXME(#82232, #143834): temporarily renamed to mitigate `#[align]` nameres ambiguity
    gated!(rustc_align, Normal, template!(List: &["alignment"]), DuplicatesOk, EncodeCrossCrate::No, fn_align, experimental!(rustc_align)),
+    gated!(rustc_align_static, Normal, template!(List: &["alignment"]), DuplicatesOk, EncodeCrossCrate::No, static_align, experimental!(rustc_align_static)),


@RalfJung this is where the attribute gets declared, and the feature name on this line (static_align in this case) must be unique

That's very odd, why does it have to be unique? That's not how features work anywhere else in the compiler, AFAIK.

ah, sorry, what I meant by unique is that an attribute must be tied to one unstable feature. #[rustc_align] is already tied to fn_align, so it can't also be used for static_align.

So, one unstable feature can "contain" multiple attributes, but an attribute always is tied to only one unstable feature.

Ah, by "attribute used for feature" you mean "attribute is gated by feature". That's a rather confusing and non-standard way of saying things.^^

So what you want is to have a separate feature gate for aligning functions vs aligning statics, and the attribute registration macro isn't flexible enough to allow that. rustc_align is for functions only, despite its name sounding more general.

Since these are all temporary names anyway, I guess it doesn't matter too much, but when it comes to stabilization we very much might need the ability to stabilize an attribute in some positions while leaving it unstable elsewhere. Otherwise we have no way of unstably extending where an existing stable attribute can be used. So this limitation of attribute feature gating seems like it could become problematic. Cc @jdonszelmann

Jules-Bertholet · 2025-09-03T21:29:12Z

This needs tests for:

Thread locals (#[thread_local] and thread_local!)
extern block statics

folkertdev · 2025-09-03T22:18:19Z

extern

Is there any interesting behavior to test here? (I'll add a test for the attribute being valid).

thread_local!

hmm, the thread_local! macro actually applies the attribute to a constant

rust/library/std/src/sys/thread_local/native/mod.rs

Lines 107 to 110 in fd75a9c

    
           ($(#[$attr:meta])* $vis:vis $name:ident, $t:ty, $($init:tt)*) => { 
        
               $(#[$attr])* $vis const $name: $crate::thread::LocalKey<$t> = 
        
                   $crate::thread::local_impl::thread_local_inner!(@key $t, $($init)*); 
        
           },

We need a different attribute than `rustc_align` because unstable attributes are tied to their feature (we can't have two unstable features use the same unstable attribute). Otherwise this uses all of the same infrastructure as `#[rustc_align]`.

folkertdev · 2025-09-04T20:19:46Z

compiler/rustc_const_eval/src/interpret/util.rs

-    let alloc = Allocation::try_new(layout.size, layout.align.abi, AllocInit::Uninit, ())?;
+    // Take alignment from attributes into account here so that there is one source
+    // of truth for the alignment of this allocation.
+    let align = match ecx.tcx.codegen_fn_attrs(static_def_id).alignment {
+        Some(align_from_attribute) => Ord::max(align_from_attribute, layout.align.abi),
+        None => layout.align.abi,
+    };


@RalfJung we have the information here to retrieve the alignment. Or perhaps you still prefer passing it in as an argument?

The final line of this function

ecx.machine.static_root_ids = Some((alloc_id, static_def_id)); assert!(ecx.memory.alloc_map.insert(alloc_id, (MemoryKind::Stack, alloc)).is_none()); interp_ok(ecx.ptr_to_mplace(Pointer::from(alloc_id).into(), layout))

uses the passed-in layout, which is not over-aligned. is that OK?

hmm, what should we do here?

rust/compiler/rustc_middle/src/mir/interpret/mod.rs

Lines 366 to 391 in 0d6a806

GlobalAlloc::Static(def_id) => {

let DefKind::Static { nested, .. } = tcx.def_kind(def_id) else {

bug!("GlobalAlloc::Static is not a static")

};

if nested {

// Nested anonymous statics are untyped, so let's get their

// size and alignment from the allocation itself. This always

// succeeds, as the query is fed at DefId creation time, so no

// evaluation actually occurs.

let alloc = tcx.eval_static_initializer(def_id).unwrap();

(alloc.0.size(), alloc.0.align)

} else {

// Use size and align of the type for everything else. We need

// to do that to

// * avoid cycle errors in case of self-referential statics,

// * be able to get information on extern statics.

let ty = tcx

.type_of(def_id)

.no_bound_vars()

.expect("statics should not have generic parameters");

let layout = tcx.layout_of(typing_env.as_query_input(ty)).unwrap();

assert!(layout.is_sized());

(layout.size, layout.align.abi)

}

}

Obviously the type does not take the additional alignment into account. The comment also suggests that unconditionally using the allocation is incorrect. So do we check the attributes again here (thus kind of sacrificing the one source of truth)?

Nested statics can't carry an attribute so I don't think you have to do anything here.

we have the information here to retrieve the alignment.

Nice!

uses the passed-in layout, which is not over-aligned. is that OK?

Yes, that just determines the type of the place, which is not changed by the over-alignment.

Jules-Bertholet · 2025-09-05T03:28:53Z

Here is an implementation for thread_local!: Jules-Bertholet@ad038e6

RalfJung · 2025-09-05T06:57:13Z

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -953,6 +953,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {

        // # Global allocations
        if let Some(global_alloc) = self.tcx.try_get_global_alloc(id) {
+            // NOTE: static alignment from attributes has already been applied to the allocation.
            let (size, align) = global_alloc.size_and_align(*self.tcx, self.typing_env);


The GlobalAlloc::Static case in size_and_align still needs to check the attribute I think.

I can't think of a good way to deduplicate that with create_static_alloc.

I think I found a way to dedup this (pushed to the PR), let's see if that works.

i.e. no longer have each backend do that, which is error-prone

rustbot · 2025-09-05T09:51:23Z

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

rustbot assigned traviscross Sep 3, 2025

RalfJung reviewed Sep 3, 2025

View reviewed changes

compiler/rustc_const_eval/src/interpret/memory.rs Outdated Show resolved Hide resolved

RalfJung reviewed Sep 3, 2025

View reviewed changes

compiler/rustc_codegen_gcc/src/consts.rs Outdated Show resolved Hide resolved

folkertdev commented Sep 3, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

folkertdev force-pushed the static-align branch from 190ad9c to 07f4381 Compare September 3, 2025 23:07

traviscross changed the title ~~implement #[rustc_align_static(N)] on statics~~ Implement #[rustc_align_static(N)] on statics Sep 3, 2025

traviscross added the F-static_align #![feature(static_align)] label Sep 3, 2025

This comment has been minimized.

Sign in to view

folkertdev force-pushed the static-align branch from 07f4381 to d66847e Compare September 4, 2025 20:17

folkertdev commented Sep 4, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

RalfJung reviewed Sep 5, 2025

View reviewed changes

folkertdev and others added 3 commits September 5, 2025 11:51

consider the align attribute when allocating a static

f3b5aae

i.e. no longer have each backend do that, which is error-prone

Support #[rustc_align_static] inside thread_local!

2b480d3

add thread_local! example to the feature gate test

6f0825e

folkertdev force-pushed the static-align branch from d66847e to 6f0825e Compare September 5, 2025 09:51

dedup static alignment logic

1e8ff95

	GlobalAlloc::Static(def_id) => {
	let DefKind::Static { nested, .. } = tcx.def_kind(def_id) else {
	bug!("GlobalAlloc::Static is not a static")
	};

	if nested {
	// Nested anonymous statics are untyped, so let's get their
	// size and alignment from the allocation itself. This always
	// succeeds, as the query is fed at DefId creation time, so no
	// evaluation actually occurs.
	let alloc = tcx.eval_static_initializer(def_id).unwrap();
	(alloc.0.size(), alloc.0.align)
	} else {
	// Use size and align of the type for everything else. We need
	// to do that to
	// * avoid cycle errors in case of self-referential statics,
	// * be able to get information on extern statics.
	let ty = tcx
	.type_of(def_id)
	.no_bound_vars()
	.expect("statics should not have generic parameters");
	let layout = tcx.layout_of(typing_env.as_query_input(ty)).unwrap();
	assert!(layout.is_sized());
	(layout.size, layout.align.abi)
	}
	}

Implement #[rustc_align_static(N)] on statics #146178

Are you sure you want to change the base?

Implement #[rustc_align_static(N)] on statics #146178

Conversation

folkertdev commented Sep 3, 2025

Uh oh!

rustbot commented Sep 3, 2025

Uh oh!

RalfJung commented Sep 3, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jules-Bertholet commented Sep 3, 2025

Uh oh!

This comment has been minimized.

folkertdev commented Sep 3, 2025

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Jules-Bertholet commented Sep 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Sep 5, 2025

Uh oh!

Uh oh!

Implement `#[rustc_align_static(N)]` on `static`s #146178

Implement `#[rustc_align_static(N)]` on `static`s #146178