Fix g_gc_lowest_address updating - #131324
Conversation
There is a rare issue that occurs when virtual memory blocks allocated by GC end up getting addresses that trigger a code path where the `g_gc_lowest_address` .. `g_gc_highest_address` is expanded down and the `g_gc_lowest_address` is exactly 1/2 of the `g_gc_highest_address`. We end up setting the `g_gc_lowest_address` to 0 and that then breaks GC because it starts considering NULL references as being part of the GC heap. The culprit is a check that uses `>` instead of `>=`, which prevents the `g_gc_lowest_address` to be clamped to OS_PAGE_SIZE as its minimal value. Close dotnet#131216
|
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. |
|
Tagging subscribers to this area: @anicka-net, @dotnet/gc |
There was a problem hiding this comment.
Pull request overview
Fixes an edge-case in CoreCLR GC card-table growth logic where expanding the tracked heap address range could incorrectly allow g_gc_lowest_address to become 0, which can cause GC to treat null references as in-heap.
Changes:
- Adjusts a boundary comparison from
>to>=when deciding whether to clamp the new lowest address toOS_PAGE_SIZE, preventing the “exactly half” span case from underflowing to0.
kkokosa
left a comment
There was a problem hiding this comment.
The PR body says the bug triggers when "g_gc_lowest_address is exactly 1/2 of g_gc_highest_address " (i.e. L = H/2 ). But there's ps *= 2 at line 604. The real exact boundary is L = 2/3 * H. Just for the sake of documentation.
|
/backport to release/10.0 |
|
Started backporting to |
|
@janvorli backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix g_gc_lowest_address updating
Using index info to reconstruct a base tree...
A src/coreclr/gc/card_table.cpp
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): src/coreclr/gc/card_table.cpp deleted in HEAD and modified in Fix g_gc_lowest_address updating. Version Fix g_gc_lowest_address updating of src/coreclr/gc/card_table.cpp left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 Fix g_gc_lowest_address updating
Error: The process '/usr/bin/git' failed with exit code 128 |
There is a rare issue that occurs when virtual memory blocks allocated by GC end up getting addresses that trigger a code path where the
g_gc_lowest_address..g_gc_highest_addressis expanded down and theg_gc_lowest_addressis exactly 2/3 of theg_gc_highest_address. We end up setting theg_gc_lowest_addressto 0 and that then breaks GC because it starts considering NULL references as being part of the GC heap.The culprit is a check that uses
>instead of>=, which prevents theg_gc_lowest_addressto be clamped to OS_PAGE_SIZE as its minimal value.Close #131216