Check that nested statics in thread locals are duplicated per thread. #123362
Conversation
rustbot
added
S-waiting-on-review
Why only for nested mutable statics? I don't think mutability should make a difference here. |
funny you ask. The first thing I tried was to just outlaw any nested statics, and it seems to pass all tests |
|
Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri |
| #![feature(const_refs_to_cell)] | ||
| #![feature(thread_local)] | ||
|
|
||
| #[thread_local] |
There was a problem hiding this comment.
Does this not also affect thread_local!() macro invokations? It should, since those just desugar to #[thread_local] internally?
There was a problem hiding this comment.
yes, but those always desugar to a Key<StaticsType>, which is always initialized with None internally, and then set to Some by computing a value at runtime at thread start.
There was a problem hiding this comment.
I'm looking at the implementation of thread_local_inner in library/std/src/sys/thread_local/fast_local.rs, which initializes:
#[thread_local]
// We use `UnsafeCell` here instead of `static mut` to ensure any generated TLS shims
// have a nonnull attribute on their return value.
static VAL: $crate::cell::UnsafeCell<$t> = $crate::cell::UnsafeCell::new(INIT_EXPR);
There was a problem hiding this comment.
Notice INIT_EXPR, not $init_expr. It is first stored in a const and then assigned here.
| error: #[thread_local] does not support implicit nested statics, please create explicit static items and refer to them instead | ||
| --> $DIR/nested_thread_local.rs:7:1 | ||
| | | ||
| LL | static mut FOO: &u32 = { |
There was a problem hiding this comment.
Is there a way to extract a more exact span from the allocation? Specifically, I'd like a label on the &{ x } part that says "this is promoted" or something.
There was a problem hiding this comment.
Miri extends the interpreter with more detailed tracking of where each allocation comes from, but so far the compile-time interpreter does not have that information. Maybe we should add a Span to the core allocation type rather than just doing this as a Miri extension.
There was a problem hiding this comment.
I was just wondering about that when I saw this error. We can see how perf is affected, but it would be really nice for many diagnostics
There was a problem hiding this comment.
Yeah... but in a separate PR, please. :)
|
Are there tests to ensure that EDIT: Ah, yes, there's a test like that. |
|
@bors r+ |
bors
added
S-waiting-on-bors
…s, r=estebank Check that nested statics in thread locals are duplicated per thread. follow-up to rust-lang#123310 cc `@compiler-errors` `@RalfJung` fwiw: I have no idea how thread local statics make that work under LLVM, and miri fails on this example, which I would have expected to be the correct behaviour. Since the `#[thread_local]` attribute is just an internal implementation detail, I'm just going to start hard erroring on nested mutable statics in thread locals.
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#123198 (Add fn const BuildHasherDefault::new) - rust-lang#123226 (De-LLVM the unchecked shifts [MCP#693]) - rust-lang#123302 (Make sure to insert `Sized` bound first into clauses list) - rust-lang#123348 (rustdoc: add a couple of regression tests) - rust-lang#123362 (Check that nested statics in thread locals are duplicated per thread.) - rust-lang#123368 (CFI: Support non-general coroutines) - rust-lang#123375 (rustdoc: synthetic auto trait impls: accept unresolved region vars for now) - rust-lang#123378 (Update sysinfo to 0.30.8) Failed merges: - rust-lang#123349 (Fix capture analysis for by-move closure bodies) r? `@ghost` `@rustbot` modify labels: rollup
That is not what everyone imagines to be the case: I realize |
|
Yea, I noticed when I tried to make thr feature gate internal 😅 Still, the hard error, while low quality, prevents any issues from cropping up. Working on improving it via giving Allocations a Span as discussed above |
Rollup merge of rust-lang#123362 - oli-obk:thread_local_nested_statics, r=estebank Check that nested statics in thread locals are duplicated per thread. follow-up to rust-lang#123310 cc ``@compiler-errors`` ``@RalfJung`` fwiw: I have no idea how thread local statics make that work under LLVM, and miri fails on this example, which I would have expected to be the correct behaviour. Since the `#[thread_local]` attribute is just an internal implementation detail, I'm just going to start hard erroring on nested mutable statics in thread locals.
follow-up to #123310
cc @compiler-errors @RalfJung
fwiw: I have no idea how thread local statics make that work under LLVM, and miri fails on this example, which I would have expected to be the correct behaviour.
Since the
#[thread_local]attribute is just an internal implementation detail, I'm just going to start hard erroring on nested mutable statics in thread locals.