Windows VirtualAlloc() - "Memory leak" error when using MEM_COMMIT?

[cybersquash]

Hello,

I've recently started running cppcheck on an exising project, and I'm a little puzzled about why cppcheck is having issues with some of our VirtualAlloc calls. We're using VirtualAlloc to reserve a range of addresses (using MEM_RESERVE), and then repeatedly allocating the reserved pages using MEM_COMMIT. When we're all done with the buffer, we call VirtualFree on the address returned from the original MEM_RESERVE call.

It looks like when windows.cfg is used, cppcheck will report a memory leak error on every one of our MEM_COMMIT calls, even though the pages are all freed later with a single call to VirtualFree.

This can be observed running cppcheck on the "Reserving and Committing Memory" example in the MSDN library.

With the following code, the MEM_COMMIT line appears to generate a cppcheck error:

// A short program to demonstrate dynamic memory allocation
// using a structured exception handler.

#include <windows.h>
#include <tchar.h>
#include <stdio.h>
#include <stdlib.h>             // For exit

#define PAGELIMIT 80            // Number of pages to ask for

LPTSTR lpNxtPage;               // Address of the next page to ask for
DWORD dwPages = 0;              // Count of pages gotten so far
DWORD dwPageSize;               // Page size on this computer

INT PageFaultExceptionFilter(DWORD dwCode)
{
    LPVOID lpvResult;

    // If the exception is not a page fault, exit.

    if (dwCode != EXCEPTION_ACCESS_VIOLATION)
    {
        _tprintf(TEXT("Exception code = %d.\n"), dwCode);
        return EXCEPTION_EXECUTE_HANDLER;
    }

    _tprintf(TEXT("Exception is a page fault.\n"));

    // If the reserved pages are used up, exit.

    if (dwPages >= PAGELIMIT)
    {
        _tprintf(TEXT("Exception: out of pages.\n"));
        return EXCEPTION_EXECUTE_HANDLER;
    }

    // Otherwise, commit another page.

    lpvResult = VirtualAlloc(
                     (LPVOID) lpNxtPage, // Next page to commit
                     dwPageSize,         // Page size, in bytes
                     MEM_COMMIT,         // Allocate a committed page
                     PAGE_READWRITE);    // Read/write access
    if (lpvResult == NULL )
    {
        _tprintf(TEXT("VirtualAlloc failed.\n"));
        return EXCEPTION_EXECUTE_HANDLER;
    }
    else
    {
        _tprintf(TEXT("Allocating another page.\n"));
    }

    // Increment the page count, and advance lpNxtPage to the next page.

    dwPages++;
    lpNxtPage = (LPTSTR) ((PCHAR) lpNxtPage + dwPageSize);

    // Continue execution where the page fault occurred.

    return EXCEPTION_CONTINUE_EXECUTION;
}

VOID ErrorExit(LPTSTR lpMsg)
{
    _tprintf(TEXT("Error! %s with error code of %ld.\n"),
             lpMsg, GetLastError ());
    exit (0);
}

VOID _tmain(VOID)
{
    LPVOID lpvBase;               // Base address of the test memory
    LPTSTR lpPtr;                 // Generic character pointer
    BOOL bSuccess;                // Flag
    DWORD i;                      // Generic counter
    SYSTEM_INFO sSysInfo;         // Useful information about the system

    GetSystemInfo(&sSysInfo);     // Initialize the structure.

    _tprintf (TEXT("This computer has page size %d.\n"), sSysInfo.dwPageSize);

    dwPageSize = sSysInfo.dwPageSize;

    // Reserve pages in the virtual address space of the process.

    lpvBase = VirtualAlloc(
                     NULL,                 // System selects address
                     PAGELIMIT*dwPageSize, // Size of allocation
                     MEM_RESERVE,          // Allocate reserved pages
                     PAGE_NOACCESS);       // Protection = no access
    if (lpvBase == NULL )
        ErrorExit(TEXT("VirtualAlloc reserve failed."));

    lpPtr = lpNxtPage = (LPTSTR) lpvBase;

    // Use structured exception handling when accessing the pages.
    // If a page fault occurs, the exception filter is executed to
    // commit another page from the reserved block of pages.

    for (i=0; i < PAGELIMIT*dwPageSize; i++)
    {
        __try
        {
            // Write to memory.

            lpPtr[i] = 'a';
        }

        // If there's a page fault, commit another page and try again.

        __except ( PageFaultExceptionFilter( GetExceptionCode() ) )
        {

            // This code is executed only if the filter function
            // is unsuccessful in committing the next page.

            _tprintf (TEXT("Exiting process.\n"));

            ExitProcess( GetLastError() );

        }

    }

    // Release the block of pages when you are finished using them.

    bSuccess = VirtualFree(
                       lpvBase,       // Base address of block
                       0,             // Bytes of committed pages
                       MEM_RELEASE);  // Decommit the pages

    _tprintf (TEXT("Release %s.\n"), bSuccess ? TEXT("succeeded") : TEXT("failed") );

}

Cppcheck version: 1.74
Command line:
"C:\Program Files\cppcheck\cppcheck.exe" --inline-suppr -v --template="{file}|{line}|{severity}|{id}|{message}" --library=windows --library=std --language=c++ .
Resulting error:
"VirtualAlloc.cpp|67|error|memleak|Memory leak: lpvResult"

I'm curious, has anyone been able to work around this, or is this just another limitation in cppcheck's analysis of global variables? It also doesn't make a difference whether I use VirtualAlloc or VirtualAllocEx.

It'd be a shame to have to "--suppress=memleak" to get these errors to go away.

Thanks

[cybersquash]

Since the return value was not stored, I've worked around this by now by enabling inline suppressions, and adding "// cppcheck-suppress leakReturnValNotUsed" before the VirtualAlloc(..., MEM_COMMIT) line.

I'm still somewhat new to cppcheck, so feel free let me know if there's a better way to deal with this sort of thing.

I'm not sure if it would be worth the effort to modify the checker to ignore cases where the flAllocationType equals MEM_COMMIT (since doing a dealloc on the return isn't required to avoid a leak in that case).

[Daniel Marjamäki]

It sounds to me that the configuration for VirtualAlloc in windows.cfg should be changed.

This configuration:

  <memory>
    <alloc>VirtualAlloc</alloc>
    <dealloc>VirtualFree</dealloc>
  </memory>

Means that each VirtualAlloc call allocates memory that must be freed with a VirtualFree.

If that is not entirely true then this should be removed.

Could you send us a fixed windows.cfg?

I'm not sure if it would be worth the effort to modify the checker to ignore cases where the flAllocationType equals MEM_COMMIT (since doing a dealloc on the return isn't required to avoid a leak in that case).

There is no way to configure this directly in the <alloc> currently.

But you can configure this indirectly.

For instance (1.cfg):

<?xml version="1.0"?>
<def format="1">
  <memory>
    <alloc>VirtualAlloc_MEM_COMMIT</alloc>
    <dealloc>VirtualFree</dealloc>
  </memory>
  <define name="VirtualAlloc(A,B,C,D)" value="VirtualAlloc_##C(A,B,C,D)"/>
</def>

Example code (1.c):

void f1() {
    VirtualAlloc(1,2,MEM_RESERVE,4); // <- dont warn
    VirtualAlloc(1,2,MEM_COMMIT,4); // <- warn
}

Output from Cppcheck

daniel@debian:~/cppcheck$ ./cppcheck --library=1.cfg 1.c
Checking 1.c...
[1.c:4]: (error) Return value of allocation function 'VirtualAlloc_MEM_COMMIT' is not stored.

I do think this configuration is a hack. But it works. If we want to do it better then it takes some effort. Feel free to open a ticket in our issue tracker (http://trac.cppcheck.net) if you want it.

[Mr. X]

If that is not entirely true then this should be removed.

Or keep the issue until we can properly fix it. How often do such false positives occur?

[Daniel Marjamäki]

Or keep the issue until we can properly fix it. How often do such false positives occur?

I don't know. Is it normal that you always use MEM_COMMIT to VirtualAlloc? Or is the normal usage that you first use a MEM_RESERVE?

[cybersquash]

Thanks for your input on this.
From what I understand, it's "normal" to call VirtualAlloc with MEM_RESERVE the first time. Then, you call VirtualAlloc repeatedly with MEM_COMMIT, to commit more pages as necessary. You can also use "MEM_COMMIT | MEM_RESERVE", if you want to reserve and commit all of the memory in one step. You only need to call VirtualFree (with MEM_RELEASE) on the address returned from the first MEM_RESERVE call.

Also, I tried to use 1.cfg from your example, and I think windows.cfg somehow automatically included first when cppcheck is run on windows, even when "--library=windows" is missing the command line. As a result, the memleak is still reported. Is there a way to include 1.cfg first, or would I have to modify windows.cfg to work around this?

I guess until the issue can be properly fixed, it's always possible to "// cppcheck-suppress leakReturnValNotUsed" before any false positives.

[Daniel Marjamäki]

Sorry Mr. X that I edited my post after your comment. Maybe you would have responded differently if you saw the workaround.