Call to GetDIBits() succeeds, but program terminates

Question

When I call the following function in a Windows program, the program abruptly terminates.

The purpose of ScanRect() is to copy a rectangle at specified coordinates on the screen and load the pixel values into a memory buffer.

Every function call within ScanRect() succeeds, including both calls to GetDIBits(). The first call, with lpvBits set to NULL, causes it to fill the BITMAPINFOHEADER of bmInfo with information about the pixel data, reporting a value of 32 bits per pixel. The second call to GetDIBits() copies 80 lines of the rectangle into memory buffer pMem, returning the value 80 for the number of lines copied.

Everything seems to succeed, but then the program suddenly terminates. I inserted the line Sleep(8192) after the second call to GetDIBits(), and the program terminates after the 8 seconds have elapsed.

What is causing the program to terminate?

EDIT: the original code is revised per suggestions in this thread. No errors are detected when the function is run, but the program still terminates unexpectedly. I realize the memory buffer size is hard coded, but it is way bigger than needed for the rectangle used in the testing. That should not cause an error. Of course I will have the program compute the necessary buffer size after I find out why the program is terminating.

VOID ScanRect(int x, int y, int iWidth, int iHeight) // 992, 96, 64, 80
{   HDC hDC = GetDC(NULL);
    if (!hDC)
    {
      cout << "!hDC" << endl;        // error handling ...
    }
    else
    {   HBITMAP hBitmap = CreateCompatibleBitmap(hDC, iWidth, iHeight);
        if (!hBitmap)
        {
           cout << "!hBitmap" << endl;        // error handling ...
        }
        else
        {   HDC hCDC = CreateCompatibleDC(hDC); // compatible with screen DC
            if (!hCDC)
            {
              cout << "!hCDC" << endl;        // error handling ...
            }
            else
            {   HBITMAP hOldBitmap = (HBITMAP) SelectObject(hCDC, hBitmap);
                BitBlt(hCDC, 0, 0, iWidth, iHeight, hDC, x, y, SRCCOPY);
                BITMAPINFO bmInfo = {0};
                bmInfo.bmiHeader.biSize = sizeof(bmInfo.bmiHeader);
                if (!GetDIBits(hCDC, hBitmap, 0, iHeight, NULL, &bmInfo, DIB_RGB_COLORS))
                {
                  cout << "!GetDIBits" << endl; // error handling ...
                }
                else
                {   HANDLE hHeap = GetProcessHeap();
                    LPVOID pMem = HeapAlloc(hHeap, HEAP_ZERO_MEMORY, 65536); // TODO: calculate a proper size based on bmInfo's pixel information ...
                    if (!pMem)
                    {
                      cout << "!pMem" << endl;
                    }
                    else
                    {   int i = GetDIBits(hCDC, hBitmap, 0, iHeight, pMem, &bmInfo, DIB_RGB_COLORS);
                        cout << "i returned by GetDIBits() " << i << endl;
                        HeapFree(hHeap, NULL, pMem);
                    }
                }
                SelectObject(hCDC, hOldBitmap);
                DeleteDC(hCDC);
            }
            DeleteObject(hBitmap);
        }
        ReleaseDC(NULL, hDC);
    }
}

Use a debugger rather than sleep for figuring out where the problem is. — SoronelHaetir, May 10 '21 at 21:09
You should always undo your `SelectObject` before deleting either the HDC or the HGDIOBJ. — Ben Voigt, May 10 '21 at 21:27
You don't know whether any function succeeds or not because you do no error checking — David Heffernan, May 10 '21 at 21:28
David Heffernan, I did error checking by using cout to a console created by AllocConsole, but I omitted that from the code example for brevity. Trust me, I checked the return values of all the functions called, and they succeed. If you copy and paste the function into a Windows program, you can check the return values. The value of i is 80 after the second call to GetDIBits(). Per the suggestion by Ben Voight, I will attempt to undo SelectObject() after I figure out how to do that. But note that when I comment out the last four lines of the function, the program still terminates. — Steven Brown, May 10 '21 at 22:01
Always include error checking code. Also, we really need a [mcve]. I mean, for all we know your main function calls the function in the Q and terminates. — David Heffernan, May 10 '21 at 22:05
You didn't reserve space for the color table, so this probably overflowed the stack buffer, which is bad news. — Raymond Chen, May 11 '21 at 00:14
I'll investigate that possibility. Debug said the stack around "BITMAPINFO bmInfo" was corrupted. I moved the declaration "BITMAPINFO bmInfo = {0};" out to the global scope, and that fixed the problem. — Steven Brown, May 11 '21 at 00:38

score 1 · Answer 1 · answered May 11 '21 at 09:59

1

The biCompression value is returned by first GetDIBits is BI_BITFIELDS and before you call second GetDIBits, you need to call bmInfo.bmiHeader.biCompression = BI_RGB;. According to c++ read pixels with GetDIBits(), Setting it to BI_RGB is essential in order to avoid extra 3 DWORDs to be written at the end of structure.
More details

answered May 11 '21 at 09:59

YangXiaoPo-MSFT

1,589
1
4
22

Thank you to YangXiaPo for posting the definitive solution to the problem. After the biCompression field of the BITMAPINOHEADER member of the BITMAPINFO structure is set to BI_RGB, the ScanRect() function works as expected, and the program does not unexpectedly terminate. The "solutions" I posted got around the problem and hinted at the cause, a buffer overrun causing a stack overflow. – Steven Brown May 11 '21 at 20:02

Remy Lebeau · Answer 2 · 2021-05-11T01:08:25.903

Like @BenVoigt said in comments, you need to restore the old HBITMAP that you replaced with SelectObject() before you destroy the HDC that owns it. You are selecting hBitmap into hCDC, and then destroying hCDC before destroying hBitmap.

https://learn.microsoft.com/en-us/windows/win32/gdi/operations-on-graphic-objects

Each of these functions returns a handle identifying a new object. After an application retrieves a handle, it must call the SelectObject() function to replace the default object. However, the application should save the handle identifying the default object and use this handle to replace the new object when it is no longer needed. When the application finishes drawing with the new object, it must restore the default object by calling the SelectObject() function and then delete the new object by calling the DeleteObject() function. Failing to delete objects causes serious performance problems.

Also, you should free the GDI objects in the reverse order that you create them.

And, don't forget error handling.

Try something more like this instead:

VOID ScanRect(int x, int y, int iWidth, int iHeight) // 992, 96, 64, 80
{ 
    HDC hDC = GetDC(NULL);
    if (!hDC)
    {
        // error handling ...
    }
    else
    {
        HBITMAP hBitmap = CreateCompatibleBitmap(hDC, iWidth, iHeight);
        if (!hBitmap)
        {
            // error handling ...
        }
        else
        {
            HDC hCDC = CreateCompatibleDC(hDC); // compatible with screen DC
            if (!hCDC)
            {
                // error handling ...
            }
            else
            {
                HBITMAP hOldBitmap = (HBITMAP) SelectObject(hCDC, hBitmap);

                BitBlt(hCDC, 0, 0, iWidth, iHeight, hDC, x, y, SRCCOPY);

                SelectObject(hCDC, hOldBitmap);

                BITMAPINFO bmInfo = {0};
                bmInfo.bmiHeader.biSize = sizeof(bmInfo.bmiHeader);

                if (!GetDIBits(hCDC, hBitmap, 0, iHeight, NULL, &bmInfo, DIB_RGB_COLORS))
                {
                    // error handling ...
                }
                else
                {
                    HANDLE hHeap = GetProcessHeap();
                    LPVOID pMem = HeapAlloc(hHeap, HEAP_ZERO_MEMORY, 65536); // TODO: calculate a proper size based on bmInfo's pixel information ...
                    if (!pMem)
                    {
                        // error handling ...
                    }
                    else
                    {
                        int i = GetDIBits(hCDC, hBitmap, 0, iHeight, pMem, &bmInfo, DIB_RGB_COLORS);
                        HeapFree(hHeap, NULL, pMem);
                    }
                }

                DeleteDC(hCDC);
            }

            DeleteObject(hBitmap);
        }

        ReleaseDC(NULL, hDC);
    }
}

Note the TODO on the call to HeapAlloc(). You really should be calculating the buffer size based on the bitmap's actual width, height, pixel depth, scanline padding size, etc. Don't use a hard-coded buffer size. I will leave this as an exercise for you to figure out. Although, in this particular example, 64K should be large enough for a 64x80 32bpp bitmap, it will just waste 45K of unused memory.

I copied and pasted your code example into my program. It complies but when I ran it, the program still terminates unexpectedly. And now I will check the return values. — Steven Brown, May 10 '21 at 22:17
I checked the return values and there are no errors. However, the program still terminates. — Steven Brown, May 10 '21 at 22:32
Then the problem is likely not in this code. Run your code under a debugger and see exactly what is crashing and where. — Remy Lebeau, May 10 '21 at 22:37
Debug found the problem. It said the stack around "BITMAPINFO bmInfo" was corrupted. I moved the declaration "BITMAPINFO bmInfo = {0};" out to the global scope, and that fixed the problem. — Steven Brown, May 11 '21 at 00:36
Something I just noticed in the [`GetDIBits()` doc](https://learn.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-getdibits): "*The bitmap identified by the `hbmp` parameter **must not be selected into a device context** when the application calls this function*", so I moved the 2nd `SelectObject()` before the 1st `GetDIBits()`. Also, [this example](https://learn.microsoft.com/en-us/windows/win32/gdi/capturing-an-image) uses `GetObject()` to obtain the `BITMAPINFO` to copy from, rather than using a manual `BITMAPINFO`, so see if that makes a difference, too. — Remy Lebeau, May 11 '21 at 01:13
Remy Lebau, I will check all that out, but I commented a while ago that debug found the problem and now it is fixed. Yes, I realize the buffer size is hard coded but that is just for testing. It's way bigger than needed for the rectangle. lol — Steven Brown, May 11 '21 at 01:18
You haven't really fixed it. You should not need to use a global variable. — David Heffernan, May 11 '21 at 05:47
Agreed. You likely are still corrupting memory, you just moved it to different memory. Whether or not it is doing any real damage is unknown. — Remy Lebeau, May 11 '21 at 05:54
By "fixed the problem" I meant the program stopped terminating, and that was a clue to the nature of the problem. The eventual solution was to set the biCompression field of the BITMAPINFOHEADER to BI_RGB, preventing the BITMPAPINFO structure exceeding bounds. — Steven Brown, May 11 '21 at 23:37

Call to GetDIBits() succeeds, but program terminates

2 Answers2