X Tutup
Skip to content

Fix corruption of D3D12 CPU descriptor heap free blocks#113000

Merged
Repiteo merged 1 commit intogodotengine:masterfrom
brycehutchings:d3d12_descriptor_heap_pool_corruption_fix
Nov 21, 2025
Merged

Fix corruption of D3D12 CPU descriptor heap free blocks#113000
Repiteo merged 1 commit intogodotengine:masterfrom
brycehutchings:d3d12_descriptor_heap_pool_corruption_fix

Conversation

@brycehutchings
Copy link
Contributor

@brycehutchings brycehutchings commented Nov 20, 2025

While running my own build of Godot 4.5.1 I was seeing heap corruption in D3D12 code. To investigate I made a dev build and found I was hitting a dev assert in the D3D12 CPU descriptor heap management logic:

image

I believe the problem due to the order of deletion and update, specifically:

free_blocks_by_offset.erase(next->value.global_offset);

next->value.global_offset always matches the key of the pair in this RBMap, so this is erasing "next" itself. So the next iterator now points to deleted memory, but then next is updated and put back into free_blocks_by_offset. Since next points to deleted memory, this is undefined behavior. In my debug builds, you can see in the screenshot that some kind of sentinel values get set to make this "use-after-free" issue more obvious.

To fix this I create a block separate from the one that next points to so I can mutate it before deleting the old block and inserting in the new merged block. I don't know if this fixes the original heap corruption but it does fix the DEV_ASSERT (on line 497 in my screenshot) and corrupted block data.

@brycehutchings brycehutchings requested a review from a team as a code owner November 20, 2025 23:58
Copy link
Contributor

@blueskythlikesclouds blueskythlikesclouds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, that's a very obvious "use after free" situation. It should fix the heap corruption.

@clayjohn clayjohn added this to the 4.6 milestone Nov 21, 2025
@Repiteo Repiteo merged commit 8480b62 into godotengine:master Nov 21, 2025
36 of 40 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 21, 2025

Thanks!

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.

4 participants

X Tutup