Skip to content

Fix debug AllocMem zero-fill header lookup - #67

Closed
janrysavy wants to merge 1 commit into
pleriche:masterfrom
janrysavy:codex/fix-debug-allocmem-zero-fill
Closed

Fix debug AllocMem zero-fill header lookup#67
janrysavy wants to merge 1 commit into
pleriche:masterfrom
janrysavy:codex/fix-debug-allocmem-zero-fill

Conversation

@janrysavy

@janrysavy janrysavy commented Apr 28, 2026

Copy link
Copy Markdown

Summary

We have been long-time users of FastMM and rely on it in our Delphi codebases. Recently we started using GPT-5.5 PRO as an additional code review tool, and it has been very effective at finding subtle bugs in our own code. Based on that experience, we tried the same style of review on FastMM5.

For the issues that look actionable, we would like to send small, reproducible PRs with focused fixes, so you can decide whether they make sense for upstream.

This PR fixes one debug-mode AllocMem issue where the debug header is read from the wrong address.

Problem

FastMM_DebugGetMem_GetDebugBlock returns a pointer to user data:

Inc(PByte(Result), CDebugBlockHeaderSize);

However, FastMM_DebugAllocMem used that returned pointer as if it still pointed to TFastMM_DebugBlockHeader when reading StackTraceEntryCount.

For medium debug allocations near the medium/large threshold, stale bytes from the user area can affect the calculated debug footer size. That can make FastMM_DebugAllocMem skip the required FillChar, causing debug-mode AllocMem to return non-zeroed memory.

Fix

Read StackTraceEntryCount from the real debug header:

PFastMM_DebugBlockHeader(PByte(Result) - CDebugBlockHeaderSize)

The change is intentionally minimal and only affects debug-mode AllocMem.

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 DebugAllocMemZeroRegression;

{$APPTYPE CONSOLE}

uses
  FastMM5,
  System.SysUtils;

const
  {This size is intentionally close to the debug medium/large threshold. With
  the bug present, a stale byte from the user area can be misread as the debug
  header's StackTraceEntryCount, making FastMM_DebugAllocMem skip zero-filling.}
  TestSize = 557500;
  IterationCount = 10;

procedure CheckAllocMemReturnsZeroedMemory;
var
  P: Pointer;
  Q: Pointer;
  I: NativeInt;
begin
  P := nil;
  Q := nil;
  try
    {Step 1: Allocate a reusable debug medium block and fill the user area with
    non-zero data.}
    P := GetMemory(TestSize);
    if P = nil then
      raise Exception.Create('Initial allocation failed');

    FillChar(P^, TestSize, $CC);

    {Step 2: Free the block so a later same-size allocation is likely to reuse
    storage whose user area still contains a non-zero/debug pattern.}
    FreeMemory(P);
    P := nil;

    {Step 3: AllocMem must return zero-filled memory. The original bug could
    read StackTraceEntryCount from Q itself instead of from the real debug
    header at Q - CDebugBlockHeaderSize, then skip this required zero-fill.}
    Q := AllocMem(TestSize);
    if Q = nil then
      raise Exception.Create('AllocMem failed');

    {Step 4: Scan every returned byte. Any non-zero byte is a violation of the
    AllocMem contract and reproduces the bug.}
    for I := 0 to TestSize - 1 do
      if PByte(Q)[I] <> 0 then
        raise Exception.CreateFmt(
          'AllocMem returned non-zero memory at offset %d: $%s',
          [I, IntToHex(PByte(Q)[I], 2)]);
  finally
    if Q <> nil then
      FreeMemory(Q);
    if P <> nil then
      FreeMemory(P);
  end;
end;

var
  I: Integer;

begin
  ExitCode := 1;

  {The failure is specific to the debug memory manager path. The full debug DLL
  is not required; FastMM_EnterDebugMode is enough to install debug GetMem,
  FreeMem, ReallocMem and AllocMem handlers.}
  FastMM_EnterDebugMode;
  try
    {Repeat a few times to avoid depending on one exact free-list reuse event.}
    for I := 1 to IterationCount do
      CheckAllocMemReturnsZeroedMemory;
  finally
    FastMM_ExitDebugMode;
  end;

  Writeln('OK: debug AllocMem returned zeroed memory for ',
    IterationCount, ' iterations.');
  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: debug AllocMem returned zeroed memory for 10 iterations.

Risk

Low. Normal-mode allocator paths are unchanged. The fix only changes where debug-mode AllocMem reads the existing stack trace count before deciding whether explicit zero-fill is required.

FastMM_DebugGetMem_GetDebugBlock returns a pointer to user data, but FastMM_DebugAllocMem used that pointer as if it were the debug block header when reading StackTraceEntryCount.

For medium debug allocations near the medium/large threshold, stale user data could affect the calculated footer size and make AllocMem skip the required zero-fill.

Read StackTraceEntryCount from the real debug header at PByte(Result) - CDebugBlockHeaderSize.

A standalone reproducer was validated locally with RAD Studio 37.0 for Win32 and Win64 and is kept outside the upstream PR tree.
@pleriche

Copy link
Copy Markdown
Owner

Hi Jan, thank you very much for the report. I don't know how I missed that. The RTL makes very little use of AllocMem (mostly calling GetMem followed by FillChar), which is probably why it sailed under the radar for so long.

I have pushed a fix for it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants