Fix debug block size overflow checks

[janrysavy]

Summary

This is a second small follow-up PR from the same review pass as #67.

The first PR fixed a debug-mode AllocMem header lookup issue. This PR addresses another debug-mode edge case: calculating the actual debug block size after adding header/footer overhead without first checking whether that size is representable in NativeInt.

The fix is intentionally limited to debug-mode allocation/reallocation paths.

Problem

Debug-mode allocations add metadata around the user block:

• CDebugBlockHeaderSize
• debug footer checksum
• allocation stack trace
• free stack trace

The previous code added that overhead directly to the requested user size:

ASize + CDebugBlockHeaderSize + CalculateDebugBlockFooterSize(...)

and debug ReallocMem used the same unchecked arithmetic when deciding whether the existing block could be resized in place:

ANewSize + CDebugBlockHeaderSize + LDebugFooterSize

If the actual debug block size cannot be represented in NativeInt, the calculation can wrap before the normal allocator or realloc path sees the request.

The most important case is debug ReallocMem: a failed resize must leave the original block valid. With the unchecked in-place size test, an overflowed calculated size can make the code treat the existing block as large enough and mutate debug metadata, including UserSize and the debug footer location, before the request has been rejected.

Fix

Add a small checked helper:

TryCalculateDebugBlockSize(AUserSize, AStackTraceDepth, ADebugBlockSize)

It rejects debug block sizes that cannot be represented after adding header/footer overhead. The helper is used before:

• debug GetMem allocates a debug block
• debug ReallocMem checks whether a debug block can be resized in place
• debug ReallocMem allocates a replacement debug block

The change is intentionally limited to debug-mode paths.

Reproducer

I kept the patch minimal and did not add a new test directory because the repository does not currently appear to have one. I can add this reproducer wherever you prefer.

Standalone reproducer:

program DebugSizeOverflowRegression;

{$APPTYPE CONSOLE}

uses
  FastMM5,
  System.SysUtils;

procedure ExpectNil(APointer: Pointer; const AScenario: string);
begin
  if APointer <> nil then
    raise Exception.Create(AScenario + ' returned a non-nil pointer');
end;

procedure CheckDirectUnrepresentableAllocations;
begin
  {Negative sizes are invalid caller input, but they exercise the same guard:
  debug metadata must not be populated unless the full debug block size can be
  represented.}
  ExpectNil(FastMM_DebugGetMem(-1), 'FastMM_DebugGetMem(-1)');
  ExpectNil(FastMM_DebugAllocMem(-1), 'FastMM_DebugAllocMem(-1)');

  {This is the important positive-size case: the requested user size is a
  NativeInt value, but adding debug header/footer overhead would overflow the
  actual debug block size.}
  ExpectNil(FastMM_DebugGetMem(High(NativeInt)), 'FastMM_DebugGetMem(High(NativeInt))');
  ExpectNil(FastMM_DebugAllocMem(High(NativeInt)), 'FastMM_DebugAllocMem(High(NativeInt))');
end;

procedure CheckFailedReallocPreservesOldBlock(ANewSize: NativeInt; const AScenario: string);
var
  P: Pointer;
  Q: Pointer;
  I: Integer;
begin
  P := GetMemory(32);
  if P = nil then
    raise Exception.Create('Initial allocation failed');

  try
    FillChar(P^, 32, $5A);

    {The key ReallocMem contract is that failure leaves the old block valid.
    The buggy overflowed in-place size check could mutate UserSize and move the
    debug footer before failing.}
    Q := FastMM_DebugReallocMem(P, ANewSize);
    if Q <> nil then
      raise Exception.Create(AScenario + ' unexpectedly returned a non-nil pointer');

    {The old pointer must still be readable and later freeable after the failed
    resize request.}
    for I := 0 to 31 do
      if PByte(P)[I] <> $5A then
        raise Exception.CreateFmt('%s corrupted the old block at offset %d', [AScenario, I]);
  finally
    FreeMemory(P);
  end;
end;

begin
  ExitCode := 1;

  {The failure is specific to debug-mode wrappers that add header/footer
  overhead before calling the normal allocator. The full debug DLL is not
  required.}
  FastMM_EnterDebugMode;
  try
    CheckDirectUnrepresentableAllocations;
    {The negative-size ReallocMem case is an invalid-input boundary check.}
    CheckFailedReallocPreservesOldBlock(-1, 'FastMM_DebugReallocMem(-1)');
    {The positive-size ReallocMem case is the main overflow regression.}
    CheckFailedReallocPreservesOldBlock(High(NativeInt), 'FastMM_DebugReallocMem(High(NativeInt))');
  finally
    FastMM_ExitDebugMode;
  end;

  Writeln('OK: unrepresentable debug allocation and reallocation sizes failed cleanly.');
  ExitCode := 0;
end.

Validation

Tested locally with RAD Studio 37.0:

• Win64 dcc64: build passed, reproducer passed
• Win32 DCC32: build passed, reproducer passed

Expected successful output:

OK: unrepresentable debug allocation and reallocation sizes failed cleanly.

Risk

Low. Normal-mode allocator paths are unchanged. The fix only adds validation before debug-mode code calculates actual debug block sizes or mutates debug metadata for a resize.

[pleriche]

Thank you very much for the report. I have pushed a fix for the overflow when requesting a new size close to Max(NativeInt). I am on the fence whether it is worth adding checks for the negative size case since handling negative sizes gracefully is not in the implicit contract: These functions should all be called through the standard memory manager interface, which already checks for sizes <= 0.

If I add <= 0 checks in the debug calls then I would also need to add it to the non-debug calls (which are also affected). In the latter case performance matters, so I would rather let the memory manager interface deal with the negative values instead and not perform duplicate checks.