Weird behaviour when global variables are added in another file

Question

Here's my simple program:

#include "stdafx.h"
#include<Windows.h>

LRESULT CALLBACK WndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam);
void initBackBuffer(HWND hwnd);

HDC hBackDC = NULL;
HBITMAP hBackBitmap = NULL;

const int WIDTH = 512;
const int HEIGHT = 512;

DWORD screenBuffer[WIDTH * HEIGHT];


void draw(HWND hwnd) {
    HDC hWinDC = GetDC(hwnd);

    SetBitmapBits(hBackBitmap, HEIGHT * WIDTH * sizeof(DWORD), (const void*)(screenBuffer));
    BitBlt(hWinDC, 0, 0, WIDTH, HEIGHT, hBackDC, 0, 0, SRCCOPY);
    ReleaseDC(hwnd, hWinDC);
}

int WINAPI wWinMain(HINSTANCE hInstace, HINSTANCE hPrevInstace, LPWSTR lpCmdLine, int nCmdShow) {
    memset(screenBuffer, 0, sizeof(screenBuffer));
    MSG msg = { 0 };
    WNDCLASS wnd = { 0 };

    wnd.lpfnWndProc = WndProc;
    wnd.hInstance = hInstace;
    wnd.lpszClassName = L"Window";

    if (!RegisterClass(&wnd)) {
        return 0;
    }

    HWND hwnd = CreateWindowEx(NULL, wnd.lpszClassName, L"Window",
        WS_OVERLAPPEDWINDOW, CW_USEDEFAULT, CW_USEDEFAULT, WIDTH, HEIGHT, NULL, NULL, hInstace, NULL);

    if (!hwnd) {
        return 0;
    }

    ShowWindow(hwnd, nCmdShow);
    UpdateWindow(hwnd);

    for (int i = 0; i <= 512; i++) {
        screenBuffer[i * WIDTH + 0] = 0x00FF0000;
    }

    while (true) {
        if(PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) {
            if (msg.message == WM_QUIT) {
                break;
            }

            TranslateMessage(&msg);
            DispatchMessage(&msg);
        }


        draw(hwnd);
    }

    return msg.wParam;
}

LRESULT CALLBACK WndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam){

    switch (msg){
        case WM_CREATE:
            initBackBuffer(hwnd);
            break;
        case WM_DESTROY:
            DeleteDC(hBackDC);
            DeleteObject(hBackBitmap);
            PostQuitMessage(0);
            break;
    }
    return DefWindowProc(hwnd, msg, wParam, lParam);
}

void initBackBuffer(HWND hwnd) {
    HDC hWinDC = GetDC(hwnd);

    hBackDC = CreateCompatibleDC(hWinDC);
    hBackBitmap = CreateCompatibleBitmap(hWinDC, WIDTH, HEIGHT); 
    SetBitmapBits(hBackBitmap, HEIGHT * WIDTH * sizeof(DWORD), (const void*)(screenBuffer));

    SelectObject(hBackDC, hBackBitmap);
    ReleaseDC(hwnd, hWinDC);
}

The output is as expected.

I moved

const int WIDTH = 512;
const int HEIGHT = 512;

DWORD screenBuffer[WIDTH * HEIGHT];

into Global.h and I added #include "Global.h" in my main file.

Main File :

#include "stdafx.h"
#include<Windows.h>
#include "Global.h"

LRESULT CALLBACK WndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam);
void initBackBuffer(HWND hwnd);

HDC hBackDC = NULL;
HBITMAP hBackBitmap = NULL;


void draw(HWND hwnd) {
    HDC hWinDC = GetDC(hwnd);

    SetBitmapBits(hBackBitmap, HEIGHT * WIDTH * sizeof(DWORD), (const void*)(screenBuffer));
    BitBlt(hWinDC, 0, 0, WIDTH, HEIGHT, hBackDC, 0, 0, SRCCOPY);
    ReleaseDC(hwnd, hWinDC);
}

int WINAPI wWinMain(HINSTANCE hInstace, HINSTANCE hPrevInstace, LPWSTR lpCmdLine, int nCmdShow) {
    memset(screenBuffer, 0, sizeof(screenBuffer));
    MSG msg = { 0 };
    WNDCLASS wnd = { 0 };

    wnd.lpfnWndProc = WndProc;
    wnd.hInstance = hInstace;
    wnd.lpszClassName = L"Window";

    if (!RegisterClass(&wnd)) {
        return 0;
    }

    HWND hwnd = CreateWindowEx(NULL, wnd.lpszClassName, L"Window",
        WS_OVERLAPPEDWINDOW, CW_USEDEFAULT, CW_USEDEFAULT, WIDTH, HEIGHT, NULL, NULL, hInstace, NULL);

    if (!hwnd) {
        return 0;
    }

    ShowWindow(hwnd, nCmdShow);
    UpdateWindow(hwnd);

    for (int i = 0; i <= 512; i++) {
        screenBuffer[i * WIDTH + 0] = 0x00FF0000;
    }

    while (true) {
        if(PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) {
            if (msg.message == WM_QUIT) {
                break;
            }

            TranslateMessage(&msg);
            DispatchMessage(&msg);
        }


        draw(hwnd);
    }

    return msg.wParam;
}

LRESULT CALLBACK WndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam){

    switch (msg){
        case WM_CREATE:
            initBackBuffer(hwnd);
            break;
        case WM_DESTROY:
            DeleteDC(hBackDC);
            DeleteObject(hBackBitmap);
            PostQuitMessage(0);
            break;
    }
    return DefWindowProc(hwnd, msg, wParam, lParam);
}

void initBackBuffer(HWND hwnd) {
    HDC hWinDC = GetDC(hwnd);

    hBackDC = CreateCompatibleDC(hWinDC);
    hBackBitmap = CreateCompatibleBitmap(hWinDC, WIDTH, HEIGHT); 
    SetBitmapBits(hBackBitmap, HEIGHT * WIDTH * sizeof(DWORD), (const void*)(screenBuffer));

    SelectObject(hBackDC, hBackBitmap);
    ReleaseDC(hwnd, hWinDC);
}

Global.h

#pragma once


const int WIDTH = 512;
const int HEIGHT = 512;

DWORD screenBuffer[WIDTH * HEIGHT];

I get a erroneous white window.

I don't understand why this is happening because the compiler will anyway copy the contents of Global.h in to main file, so both variants should produce same results.

What is the cause of this problem ?

Putting `#include "stdafx.h"` in a header is dangerous, but I don't know if it would lead to the problem you are having. — user4581301, Apr 28 '18 at 06:31
Drawing outside of `WM_PAINT` is a recipe for disaster. Why are you doing it? — Matteo Italia, Apr 28 '18 at 06:53
@MatteoItalia The topic spotted by you seems to remain the only suspect. Can you elaborate and turn it into an answer, maybe a solution? — Yunnosch, Apr 28 '18 at 06:56
I might have misunderstood, but I thought you also changed the "definition-in-header" to "header-declare-code-define". — Yunnosch, Apr 28 '18 at 07:00
You have a bug here `for(int i = 0; i <= 512; i++)`, it should be `i < 512`. Buffer overruns can create unpredictable problems. Your message loop seems okay. You don't seem to be using precompiled headers. — Barmak Shemirani, Apr 28 '18 at 07:05
@BarmakShemirani I think you have spotted the error, but still I don't understand how that was the problem especially when it was working with one variant of program. — user8277998, Apr 28 '18 at 07:09
@BarmakShemirani Please consider making an answer. I guess it will be acccepted. And it can add a little additional explanation. — Yunnosch, Apr 28 '18 at 07:22
Changing this from a side-issue-related, not directly helpful answer into a comment: The header related to precompiled headers ("stdafx.h") should be included first in a code file and a best practice recommends to do it only in code files. — Yunnosch, Apr 28 '18 at 11:45

Barmak Shemirani · Accepted Answer · 2018-04-28T15:56:49.220

3

There is a bug here:

const int WIDTH = 512;
const int HEIGHT = 512;
DWORD screenBuffer[WIDTH * HEIGHT];
void foo()
{
    for (int i = 0; i <= 512; i++) {
        screenBuffer[i * WIDTH + 0] = 0x00FF0000;
    }
}

This should be should be i < 512. Otherwise it overwrites a random memory location, this can result in an error in a different location, or no error if you are lucky. Debugger may report a nonsensical error, or no error at all. If screenBuffer was created on stack, debugger may give "heap corruption" error.

Consider using std::vector to avoid this problem in future.

vector<int> vec;
vec[vec.size()] = 0;//<- debug error

^{Side note: SetDIBitsToDevice or StretchDIBits will set bits directly:}

void draw(HWND hwnd) 
{
    BITMAPINFO bi;
    bi.bmiHeader.biSize = sizeof(bi.bmiHeader);
    bi.bmiHeader.biBitCount = 32;
    bi.bmiHeader.biWidth = WIDTH;
    bi.bmiHeader.biHeight = HEIGHT;
    bi.bmiHeader.biPlanes = 1;
    bi.bmiHeader.biCompression = BI_RGB;
    HDC hdc = GetDC(hwnd);
    SetDIBitsToDevice(hdc, 0, 0, WIDTH, HEIGHT, 0, 0, 0, HEIGHT, screenBuffer,
        &bi, DIB_RGB_COLORS);
    ReleaseDC(hwnd, hdc);
}

edited Apr 28 '18 at 15:56

answered Apr 28 '18 at 07:27

Barmak Shemirani

30,904
6
40
77

1

`std::vector` is unchecked by default as well. – Matteo Italia Apr 28 '18 at 09:51
@MatteoItalia VS debugger breaks when it encounters `myvector[myvector.size()]` – Barmak Shemirani Apr 28 '18 at 14:04
That's just VC++ with "safe STL" enabled doing you a courtesy; going outside a vector boundaries is plain UB with no diagnostic required (and for example with g++ you have to explicitly enable the debug STL to get an error). – Matteo Italia Apr 28 '18 at 14:47
@MatteoItalia VS debugger doesn't have "safe STL" option. Those error checks are enabled by default in debug mode. You want to disable redundant error checks in optimized code, so this is not 100% reliable. But it's a big improvement. I am not familiar with other debuggers, I don't know why they would disable important error checks by default. – Barmak Shemirani Apr 28 '18 at 15:24
Debuggers aren't really involved, it's a library compilation mode that depends from a define (`_ITERATOR_DEBUG_LEVEL`), whose defaults are governed by the compile mode. It was enabled by default even in release mode at least in some older VC++ version, as I remember that I had to switch it off explicitly because it added a good 35% overhead. The libstdc++-equivalent (`_GLIBCXX_DEBUG`) isn't enabled by default because to implement the most extensive checks (in particular on iterators) the container ABI changes, which is a problem when linking against other static libraries. – Matteo Italia Apr 28 '18 at 16:01

Weird behaviour when global variables are added in another file

1 Answers1