Fix alignment of 8-byte thread statics on direct TLS path

[EgorBo]

Fixes#129733

Problem

Interlocked.Increment(ref threadStaticLong) on a [ThreadStatic] static long throws System.DataMisalignedException on osx-arm64.

Root cause

In GetTLSIndexForThreadStatic (src/coreclr/vm/threadstatics.cpp), small non-collectible thread statics of already-initialized classes are placed via a top-down bump allocator on the direct-on-thread-local-data path. The per-field alignment was computed with a missing else:

uint32_t alignment;
if (bytesNeeded >= 8)
alignment = 8;
if (bytesNeeded >= 4) // <-- missing 'else'
alignment = 4;
elseif (bytesNeeded >= 2)
alignment = 2;
else
alignment = 1;

For an 8-byte field (long/double), the first if sets alignment = 8, but the second if (bytesNeeded >= 4) is also true and overwrites it with alignment = 4. The offset is then chosen via AlignDown(indexOffsetWithoutAlignment, alignment), so with alignment == 4 it can land on a 4-mod-8 boundary. Since ThreadLocalData (and thus ExtendedDirectThreadLocalTLSData) is only 8-byte aligned, the resulting absolute address of the long can be 4-byte but not 8-byte aligned.

Why it only reproduces on arm64 (and intermittently)

• x64 tolerates misaligned atomic operations; arm64 Interlocked.Increment lowers to ldaxr/stlxr, which fault on a misaligned address → DataMisalignedException.
• It is layout-dependent: whether the 4-aligned offset also happens to be 8-aligned depends on how many bytes earlier thread statics already consumed in the bump allocator. This is why a real app (e.g. ASP.NET/Kestrel, which registers other thread statics first) trips it while a trivial repro may not.

Fix

Add the missing else so an 8-byte field keeps alignment = 8.

Testing

I was unable to build/run locally for this change (development box is linux-x64 while the failure manifests on osx-arm64, where misaligned atomics fault). The fix is a one-line correction to the alignment selection logic and was verified by source inspection. CI will exercise the relevant platforms.

Note

This pull request was authored with the assistance of GitHub Copilot.

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @VSadov
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

Fixes a correctness bug in CoreCLR’s thread statics TLS-index allocation logic where 8-byte non-GC thread static blocks on the direct-on-thread-local-data path could be aligned only to 4 bytes, leading to misaligned long/double storage on some platforms.

Changes:

• Correct the alignment selection logic so bytesNeeded >= 8 preserves 8-byte alignment (via else if chaining).
• Prevent potential misalignment of 8-byte thread statics allocated from ThreadLocalData::ExtendedDirectThreadLocalTLSData.

[EgorBo]

PTAL @jkotas@janvorli

Do we need to backport it? Non-atomic access to TLS sounds like a not a big deal, but it might be problematic for e.g. double field on arm32

[janvorli]

LGTM, thank you!

[jkotas]

Do we need to backport it?

Customer reported regression. It makes sense to backport to me.

This bug may lead to performance issues as well.

[EgorBo]

/backport to release/10.0

[github-actions]

Started backporting to release/10.0 (link to workflow run)