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

compiler/rustc_const_eval/src/interpret/memory.rs

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

folkertdev · 2025-09-05T12:20:27Z

That's odd, the miri tests pass for me locally. Do those somehow cache the standard library even when you don't pass --keep-stage?

RalfJung · 2025-09-05T12:58:54Z

Miri has its own standard library build but it should usually be recompiled automatically when anything changed.

RalfJung · 2025-09-05T13:08:21Z

The error might only occur after merging the branch with latest master (which happens implicitly in CI).

Jules-Bertholet · 2025-09-05T13:27:21Z

which happens implicitly in CI

Doesn’t that apply only to Bors CI? (E.g. Bors try or Bors r+)

RalfJung · 2025-09-05T13:28:37Z

No, Github merges the PR with the master branch for PR CI.

Pretty terrible IMO in terms of reproducibility, but well, it is what it is.

folkertdev · 2025-09-05T13:39:45Z

So would a miri sync fix this (or at least make it reproducible locally)?

RalfJung · 2025-09-05T13:40:48Z

If my theory is correct, rebasing over the latest master commit should let you reproduce the issue.

RalfJung · 2025-09-05T13:42:55Z

Oh, I just noticed the target. The failure is on x86_64-pc-windows-gnu. The new thread-local macro logic is probably just broken on Windows.
./x test miri --target x86_64-pc-windows-gnu -- align should reproduce this.

EDIT: And indeed, it does.

rust-log-analyzer · 2025-09-05T14:00:45Z

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

Click to see the possible cause of the failure (guessed by this bot)

tests/pass/shims/x86/rounding-error.rs ... ok
tests/pass/shims/x86/intrinsics-x86-gfni.rs ... ok

FAILED TEST: tests/pass/static_align.rs
command: MIRI_ENV_VAR_TEST="0" MIRI_TEMP="/tmp/miri-uitest-5oy4bb" RUST_BACKTRACE="1" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/miri" "--error-format=json" "--sysroot=/checkout/obj/build/x86_64-unknown-linux-gnu/miri-sysroot" "-Dwarnings" "-Dunused" "-Ainternal_features" "-Zui-testing" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/tests/pass" "tests/pass/static_align.rs" "--edition" "2021" "--target" "x86_64-pc-windows-gnu"

error: test got exit status: 1, but expected 0
 = note: compilation failed, but was expected to succeed

error: no output was expected
---
LL | / thread_local! {
LL | |     #[rustc_align_static(512)]
LL | |     static LOCAL: u64 = 0;
...  |
LL | |     static HASDROP_CONST_LOCAL: HasDrop = const { HasDrop };
LL | | }
   | |_^ no rules expected this token in macro call
   |
note: while trying to match `]`
  --> /checkout/library/std/src/sys/thread_local/os.rs:14:38
   |
LL |     (@key $t:ty, $(#[$align_attr:meta])*, const $init:expr) => {
   |                                      ^
   = note: this error originates in the macro `$crate::thread::local_impl::thread_local_inner` which comes from the expansion of the macro `thread_local` (in Nightly builds, run with -Z macro-backtrace for more info)

error: no rules expected `:`
##[error]  --> tests/pass/static_align.rs:17:1
   |
LL | / thread_local! {
LL | |     #[rustc_align_static(512)]
LL | |     static LOCAL: u64 = 0;
...  |
LL | |     static HASDROP_CONST_LOCAL: HasDrop = const { HasDrop };
LL | | }
   | |_^ no rules expected this token in macro call
   |
note: while trying to match `]`
  --> /checkout/library/std/src/sys/thread_local/os.rs:14:38
   |
LL |     (@key $t:ty, $(#[$align_attr:meta])*, const $init:expr) => {
   |                                      ^
   = note: this error originates in the macro `$crate::thread::local_impl::thread_local_inner` which comes from the expansion of the macro `thread_local` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 2 previous errors

---
LL | / thread_local! {
LL | |     #[rustc_align_static(512)]
LL | |     static LOCAL: u64 = 0;
...  |
LL | |     static HASDROP_CONST_LOCAL: HasDrop = const { HasDrop };
LL | | }
   | |_^ no rules expected this token in macro call
   |
note: while trying to match `]`
  --> /checkout/library/std/src/sys/thread_local/os.rs:14:38
   |
LL |     (@key $t:ty, $(#[$align_attr:meta])*, const $init:expr) => {
   |                                      ^
   = note: this error originates in the macro `$crate::thread::local_impl::thread_local_inner` which comes from the expansion of the macro `thread_local` (in Nightly builds, run with -Z macro-backtrace for more info)

error: no rules expected `:`
##[error]  --> tests/pass/static_align.rs:17:1
   |
LL | / thread_local! {
LL | |     #[rustc_align_static(512)]
LL | |     static LOCAL: u64 = 0;
...  |
LL | |     static HASDROP_CONST_LOCAL: HasDrop = const { HasDrop };
LL | | }
   | |_^ no rules expected this token in macro call
   |
note: while trying to match `]`
  --> /checkout/library/std/src/sys/thread_local/os.rs:14:38
   |
LL |     (@key $t:ty, $(#[$align_attr:meta])*, const $init:expr) => {
   |                                      ^
   = note: this error originates in the macro `$crate::thread::local_impl::thread_local_inner` which comes from the expansion of the macro `thread_local` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 2 previous errors

---
Location:
   /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/ui_test-0.30.2/src/lib.rs:365

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
   1: <color_eyre[d59da046dd8adab5]::config::EyreHook>::into_eyre_hook::{closure#0}<unknown>
      at <unknown source file>:<unknown line>
   2: eyre[ab4be8728fadfcd1]::private::format_err<unknown>
      at <unknown source file>:<unknown line>
   3: ui_test[20cbde7395fa2762]::run_tests_generic::<ui_test[20cbde7395fa2762]::default_file_filter, ui[61df357ba6878874]::run_tests::{closure#1}, alloc[38d5a7c5648cda0b]::boxed::Box<dyn ui_test[20cbde7395fa2762]::status_emitter::StatusEmitter>><unknown>
      at <unknown source file>:<unknown line>
   4: ui[61df357ba6878874]::ui<unknown>
      at <unknown source file>:<unknown line>
   5: ui[61df357ba6878874]::main<unknown>
      at <unknown source file>:<unknown line>
   6: std[3b892ea2c9c77f31]::sys::backtrace::__rust_begin_short_backtrace::<fn() -> core[b6d069dd0f613f31]::result::Result<(), eyre[ab4be8728fadfcd1]::Report>, core[b6d069dd0f613f31]::result::Result<(), eyre[ab4be8728fadfcd1]::Report>><unknown>
      at <unknown source file>:<unknown line>
   7: std[3b892ea2c9c77f31]::rt::lang_start::<core[b6d069dd0f613f31]::result::Result<(), eyre[ab4be8728fadfcd1]::Report>>::{closure#0}<unknown>
      at <unknown source file>:<unknown line>
   8: std[3b892ea2c9c77f31]::rt::lang_start_internal<unknown>
      at <unknown source file>:<unknown line>
   9: main<unknown>
      at <unknown source file>:<unknown line>
  10: __libc_start_main<unknown>
      at <unknown source file>:<unknown line>
  11: _start<unknown>
      at <unknown source file>:<unknown line>

Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.
Run with RUST_BACKTRACE=full to include source snippets.
error: test failed, to rerun pass `--test ui`

Caused by:
  process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/ui-4e882db334023ac9 pass` (exit status: 1)
Bootstrap failed while executing `test --stage 2 src/tools/miri --target x86_64-pc-windows-gnu --test-args pass`
Command `/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo test --target x86_64-unknown-linux-gnu -Zbinary-dep-depinfo -j 4 -Zroot-dir=/checkout --locked --color always --release --manifest-path /checkout/src/tools/miri/Cargo.toml -- pass [workdir=/checkout]` failed with exit code 1
Created at: src/bootstrap/src/core/build_steps/tool.rs:189:21
Executed at: src/bootstrap/src/core/build_steps/test.rs:677:19

Command has failed. Rerun with -v to see more details.
Build completed unsuccessfully in 0:02:03
  local time: Fri Sep  5 14:00:24 UTC 2025
  network time: Fri, 05 Sep 2025 14:00:24 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

Jules-Bertholet · 2025-09-05T14:20:00Z

The new thread-local macro logic is probably just broken on Windows.

Yes, it is, I’ll fix it

Jules-Bertholet · 2025-09-05T17:21:34Z

Yes, it is, I’ll fix it

Here you go: Jules-Bertholet@e0cc8eb

(It’s not complete yet; I still need to add cfg_attr support, and a few more tests.)

folkertdev · 2025-09-05T18:35:30Z

given how complicated the thread_local! stuff is getting, maybe that should be its own PR as a follow-up?

Jules-Bertholet · 2025-09-05T19:58:56Z

cfg_attr support

Done: Jules-Bertholet@ed2f8cd

maybe that should be its own PR as a follow-up?

Sure, if that’s what you prefer.

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

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

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

This comment has been minimized.

Sign in to view

assert alignment of thread_local statics

602e757

folkertdev force-pushed the static-align branch from 389dad1 to 602e757 Compare September 5, 2025 12:46

	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!

Uh oh!

rustbot commented Sep 5, 2025

Uh oh!

This comment has been minimized.

folkertdev commented Sep 5, 2025

Uh oh!

RalfJung commented Sep 5, 2025

Uh oh!

RalfJung commented Sep 5, 2025

Uh oh!

Jules-Bertholet commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Sep 5, 2025

Uh oh!

folkertdev commented Sep 5, 2025

Uh oh!

RalfJung commented Sep 5, 2025

Uh oh!

RalfJung commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-log-analyzer commented Sep 5, 2025

Uh oh!

Jules-Bertholet commented Sep 5, 2025

Uh oh!

Jules-Bertholet commented Sep 5, 2025

Uh oh!

folkertdev commented Sep 5, 2025

Uh oh!

Jules-Bertholet 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

Jules-Bertholet commented Sep 5, 2025 •

edited

Loading

RalfJung commented Sep 5, 2025 •

edited

Loading