TFixedAllocator<N> will not complain when you add an N+1 entry

Development Platform & Builds
, ,
1

The below code does not trigger any asserts/checks. It will start overwriting adjacent memory. (In this case, it probably will actually change the memory for the variable that contains “arr.ArrayNum” to “105”.)

I think the fix is for TFixedAllocator::CalculateSlackGrow() to “check(NewMax <= NumInlineElements)”. It currently looks at “CurrentMax” -- copy & paste error? CalculateSlackShrink() may also need to be adjusted to check the same -- althought that might want to check both values.

NOTE: TFixedAllocator::ResizeAllocation() has a proper check, but CalculateSlackGrow() essentially clamps the request for N+1 back down to N, which then avoids actually calling TFixedAllocator::ResizeAllocation().

NOTE 2: TArray::AddUninitialized() assumes the call to TArray::ResizeGrow() succeeds implicitly. It doesn’t verify that the new ArrayMax is big enough. I presume this is “by design”, as it expects the allocators to freakout if they can’t do the resize.

2

Steps to Reproduce

Throw this code somewhere that’ll run.

`TArray<int32, TFixedAllocator<4>> arr;

for (int32 c = 0; c < 5; ++c) // <— 5 is too many
{
arr.Add(c+100);
}` It should assert during the loop, but does not.

3

Added a Github PR: https://github.com/EpicGames/UnrealEngine/pull/13216

`— a/Engine/Source/Runtime/Core/Public/Containers/ContainerAllocationPolicies.h
+++ b/Engine/Source/Runtime/Core/Public/Containers/ContainerAllocationPolicies.h
[Content removed]14 @@ public:
FORCEINLINE SizeType CalculateSlackShrink(SizeType NewMax, SizeType CurrentMax, SIZE_T NumBytesPerElement) const
{
// Ensure the requested allocation will fit in the inline data area.

  • check(NewMax <= NumInlineElements);
    check(CurrentMax <= NumInlineElements);
    return NumInlineElements;
    }
    FORCEINLINE SizeType CalculateSlackGrow(SizeType NewMax, SizeType CurrentMax, SIZE_T NumBytesPerElement) const
    {
    // Ensure the requested allocation will fit in the inline data area.
  • check(CurrentMax <= NumInlineElements);
  • check(NewMax <= NumInlineElements);
    return NumInlineElements;
    }`
4

Hi,

Thanks for reporting. CalculateSlackGrow() was fixed on the 1st of November 2024, which didn’t make it into UE5.5. Not sure we need to change CalculateSlackShrink() as NewMax should be less than CurrentMax, or there is a bug in the calling container.

Cheers, Johan