Skip to content

Fix negative tmp-buffer size in vxsort align_vectorized - #134881

Open
janvorli wants to merge 1 commit into
dotnet:mainfrom
janvorli:fix-vxsort-align-junk-lane-popcount
Open

janvorli wants to merge 1 commit into
dotnet:mainfrom
janvorli:fix-vxsort-align-junk-lane-popcount

Conversation

@janvorli

Copy link
Copy Markdown
Member

Issue

Server/Workstation GC background threads can crash with an access violation (read) inside coreclr!memcpy, called from vxsort<T,M,Unroll,Shift>::vectorized_partition, itself called from do_vxsort during mark_phase/plan_phase sorting of the GC mark list. This reproduces the crash originally reported in #86548 ("The size calculated to be negative and cause Access Violation when doing vxsort vectorized_partition"), which was closed without a fix in 2023 due to lack of a reliable repro. Analysis of a production Windows crash dump (Second Chance Exception c0000005, AMD EPYC/Zen4 AVX-512) root-caused the same failure independently and pinpointed the exact algorithmic defect below.

Root cause

In vxsort::align_vectorized(), when the left read pointer isn't vector-aligned, the code pre-aligns by reading a full vector starting before left. The low L = -leftAlign lanes of that vector are "junk" elements that lie before the current [left, right] partition range (already settled by an ancestor partition step), while the remaining lanes are real elements of the current range. Both junk and real lanes are compared against the current pivot and compacted together (via compress-store or an order-preserving permute), so junk elements can land on either side of the pivot - they are not guaranteed to all be <= pivot.

The code advanced tmpLeft by the true, data-dependent count of <=pivot lanes (ltPopCountLeftPart), but advanced tmpStartLeft - the base pointer later used to size the copy-back region (leftTmpSize = tmpLeft - tmpStartLeft) - by the constant L, implicitly assuming all L junk lanes are <=pivot. When fewer than L junk lanes are actually <=pivot, tmpStartLeft overshoots tmpLeft, producing a negative leftTmpSize. That negative value, reinterpreted as an unsigned byte count in the subsequent memcpy, causes a massive out-of-bounds read and the access violation.

The formula was validated against both the original #86548 report (L=7, ltMask=0x3f -> ltPopCountLeftPart=2 -> deficit of -5 elements = -20 bytes, exactly matching the reported value) and against the investigated crash dump (1-element/4-byte deficit reproduced from register/stack state captured in the dump).

The right-side counterpart (tmpStartRight -= rightAlign & rai) has the same class of defect for junk elements read beyond right. It was partially papered over by rtPopCountRightPart = max(mask_popcount( rtMask), rightAlign), which avoids the crash but can instead leave an uninitialized "hole" in the tmp buffer that gets silently copied back into the array as if it were valid sorted data.

Fix

Replace the constant junk-lane skip on both sides with the exact, data-dependent count of junk lanes that landed on the "keep" side of the pivot, computed via a popcount masked to just the junk-lane bits (low L bits for the left side, high rightAlign bits for the right side). This removes the incorrect assumption entirely instead of papering over its symptoms, and makes the previous max() band-aid on the right side unnecessary (removed).

Validated by a full CoreCLR Debug x64 native build (build-runtime.cmd), confirming the change compiles cleanly across the WKS/SVR GC, gcsample, and NativeAOT runtime consumers, for both AVX2 and AVX-512 machine traits and both int32/int64 element types.

Fixes #86548

…torized sort)

Issue:
Server/Workstation GC background threads can crash with an access
violation (read) inside coreclr!memcpy, called from
vxsort<T,M,Unroll,Shift>::vectorized_partition, itself called from
do_vxsort during mark_phase/plan_phase sorting of the GC mark list.
This reproduces the crash originally reported in dotnet#86548
("The size calculated to be negative and cause Access Violation when
doing vxsort vectorized_partition"), which was closed without a fix in
2023 due to lack of a reliable repro. Analysis of a production Windows
crash dump (Second Chance Exception c0000005, AMD EPYC/Zen4 AVX-512)
root-caused the same failure independently and pinpointed the exact
algorithmic defect below.

Root cause:
In vxsort::align_vectorized(), when the left read pointer isn't
vector-aligned, the code pre-aligns by reading a full vector starting
before `left`. The low `L = -leftAlign` lanes of that vector are "junk"
elements that lie before the current [left, right] partition range
(already settled by an ancestor partition step), while the remaining
lanes are real elements of the current range. Both junk and real lanes
are compared against the current pivot and compacted together (via
compress-store or an order-preserving permute), so junk elements can
land on either side of the pivot - they are not guaranteed to all be
<= pivot.

The code advanced `tmpLeft` by the true, data-dependent count of
<=pivot lanes (`ltPopCountLeftPart`), but advanced `tmpStartLeft` - the
base pointer later used to size the copy-back region
(`leftTmpSize = tmpLeft - tmpStartLeft`) - by the *constant* `L`,
implicitly assuming all L junk lanes are <=pivot. When fewer than L
junk lanes are actually <=pivot, tmpStartLeft overshoots tmpLeft,
producing a negative leftTmpSize. That negative value, reinterpreted
as an unsigned byte count in the subsequent memcpy, causes a massive
out-of-bounds read and the access violation.

The formula was validated against both the original dotnet#86548 report
(L=7, ltMask=0x3f -> ltPopCountLeftPart=2 -> deficit of -5 elements =
-20 bytes, exactly matching the reported value) and against the
investigated crash dump (1-element/4-byte deficit reproduced from
register/stack state captured in the dump).

The right-side counterpart (`tmpStartRight -= rightAlign & rai`) has
the same class of defect for junk elements read beyond `right`. It was
partially papered over by `rtPopCountRightPart = max(mask_popcount(
rtMask), rightAlign)`, which avoids the crash but can instead leave an
uninitialized "hole" in the tmp buffer that gets silently copied back
into the array as if it were valid sorted data.

Fix:
Replace the constant junk-lane skip on both sides with the exact,
data-dependent count of junk lanes that landed on the "keep" side of
the pivot, computed via a popcount masked to just the junk-lane bits
(low L bits for the left side, high `rightAlign` bits for the right
side). This removes the incorrect assumption entirely instead of
papering over its symptoms, and makes the previous `max()` band-aid on
the right side unnecessary (removed).

Validated by a full CoreCLR Debug x64 native build
(build-runtime.cmd), confirming the change compiles cleanly across the
WKS/SVR GC, gcsample, and NativeAOT runtime consumers, for both AVX2
and AVX-512 machine traits and both int32/int64 element types.

Fixes dotnet#86548

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b029aa4e-2a19-4582-b9e5-4918cb4c3095
@janvorli
janvorli requested a review from kkokosa September 29, 2026 21:10
@janvorli janvorli self-assigned this Sep 29, 2026
@janvorli

Copy link
Copy Markdown
Member Author

cc: @damageboy

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @anicka-net, @dotnet/gc
See info in area-owners.md if you want to be subscribed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The size calculated to be negative and cause Access Violation when doing vxsort vectorized_partition

1 participant