GetWindowLongPtr returns garbage

Question

I am developing a Win32 dialog box wrapper.

.h file

class dlg {
    static INT_PTR CALLBACK DlgProcTmp(HWND hwnd, 
                                       UINT wm, WPARAM wp, LPARAM lp);
    INT_PTR CALLBACK DlgProc(HWND hwnd, UINT wm, WPARAM wp, LPARAM lp);

    bool ismodal;
protected:
    HWND hwndDlg;
    int id;
public:
    virtual void oncreate(const widget &w) { }
    virtual void oncmd(const widget &w, int code, int idCntrl, HWND hwnd) { }
    virtual void onclose(const widget &w) { }
    dlg() { }
    dlg(int id);
    INT_PTR domodal(HWND hwndOwner = nullptr);
    widget domodeless(HWND hwndOwner = nullptr, int cmdshow = SW_SHOW);
};

.cpp file

INT_PTR dlg::DlgProcTmp(HWND hwnd, UINT wm, WPARAM wp, LPARAM lp)
{
    dlg *This;

    if (wm == WM_INITDIALOG) {
        This = (dlg *) lp;
        setwinlong(hwnd, DWLP_USER, This);
        This->oncreate(widget(hwnd));

        return TRUE;
    }
    if ((This = getwinlong<dlg *>(hwnd, DWLP_USER)) != nullptr)
        return This->DlgProc(hwnd, wm, wp, lp);
    return FALSE;
}
INT_PTR dlg::DlgProc(HWND hwnd, UINT wm, WPARAM wp, LPARAM lp)
{
    this->hwndDlg = hwnd;
    switch (wm) {
        case WM_COMMAND:
            this->oncmd(widget(hwnd), HIWORD(wp), LOWORD(wp), (HWND) lp);
            if (this->ismodal)
                EndDialog(hwnd, LOWORD(wp));
            else
                DestroyWindow(hwnd);
            return TRUE;
        case WM_DESTROY:
            this->onclose(widget(hwnd));
            return TRUE;
    }
    return FALSE;
}

The dialog is created in domodal with a DialogBoxParam call. The last argument is the this pointer, which I then retrieve from the lparam of the WM_INITDIALOG message. To allow this to be used between different messages, I save this with the HWND in WM_INITDIALOG. However, whenever a message not WM_INITDIALOG arrives, and I get the this pointer with GetWindowLongPtr, it returns a garbage dlg class, whose vtable is corrupt. I use the vtable to call the correct handler function. As a result, my code crashes on the first line of the WM_COMMAND handler. Here is what the debugger shows as the value of this:

Why is GetWindowLongPtr returning garbage?

BTW, getwinlong is a wrapper for GetWindowLongPtr, and setwinlong is a wrapper for SetWindowLongPtr. Here are their implementations:

template <class TYPE> static TYPE getwinlong(HWND hwnd, int idx)
{
    return (TYPE) GetWindowLongPtr(hwnd, idx);
}
template <class TYPE> static void setwinlong(HWND hwnd, int idx, TYPE val)
{
    SetWindowLongPtr(hwnd, idx, (LONG_PTR) val);
}

I have seen the numerous posts about how GetWindowLongPtr will fail on Win64, if you cast to LONG instead of LONG_PTR, like https://blogs.msdn.microsoft.com/oldnewthing/20131226-00/?p=2263. I believe that my problem is different.

Edit: @andlabs wanted the code that creates the dialog:

INT_PTR dlg::domodal(HWND hwndOwner)
{
    this->ismodal = true;
    return DialogBoxParam(gethinst(hwndOwner), 
        MAKEINTRESOURCE(id), hwndOwner, dlg::DlgProcTmp, 
        (LPARAM) this);
}

Does the dialog template actually specify extra space for user data? Is the dialog itself using that data for its own purposes? There's always: [`SetProp`](https://msdn.microsoft.com/en-us/library/windows/desktop/ms633568%28v=vs.85%29.aspx) — theB, Jun 11 '16 at 15:18
@theB You mean this? https://msdn.microsoft.com/en-us/library/windows/desktop/ms645398(v=vs.85).aspx — , Jun 11 '16 at 15:20
Can you post the code you use to create an instance of this dialog? — andlabs, Jun 11 '16 at 15:28
Possible for naming convetions - class need start with uppercase - "Dlg" — oklas, Jun 11 '16 at 15:42
What's `DLGPROC(dlg::DlgProcTmp)` for? If this doesn't compile without a cast/conversion, the you are possibly passing a function pointer with the wrong signature. — IInspectable, Jun 11 '16 at 15:43
@oklas No, different programmers have different styles. Some choose to use camelCase, some PascalCase, some ALLCAPS. As long as it is readable, it should not matter — , Jun 11 '16 at 15:47
Are you shure that you set window long with good value? May be you set garbage, and than read that garbage back? — oklas, Jun 11 '16 at 15:49
@oklas I am sure, because in WM_INITDIALOG, `This` (note the uppercase T) is dereferenced to call `oncreate`, but that works. — , Jun 11 '16 at 15:52
Do not say me abou styles - It is not my devote. You ask reason - I suppose for styling. May be not. I am here for help to you. Do not like comment say - I remove them. — oklas, Jun 11 '16 at 15:52
Try to add log output to yours setwinlong<> to sure. Errors is hide there - where we shure. — oklas, Jun 11 '16 at 15:55
Nothing technically, it's just bad style to use a side effect. Browser posted before ready, sorry. :) You can write as `if( Bah* p = whatever )` using initialization instead of assignment. — Cheers and hth. - Alf, Jun 11 '16 at 15:56
Do some debugging. Observe or log the values that are used as args, are returned. — David Heffernan, Jun 11 '16 at 16:00
Yes, and I (and many others) write condition in reverse order to minificate ocurrance of assignment instead of comparision. `if( fun() == var )` instad of `if( var == fun() )` which will error whe one `=` is skipped. — oklas, Jun 11 '16 at 16:01
Yes, SetWindowLongPtr is returning 0, but GetLastError() also gives 0. I also cleared the error number as per MSDN(https://msdn.microsoft.com/en-us/library/windows/desktop/ms644898(v=vs.85).aspx) — , Jun 11 '16 at 16:12
I wonder if the default dialog class reserves space for you, or if you're using a custom window class that does that. Otherwise, oops. By the way, why not use `SetProp` and `GetProp`. — Cheers and hth. - Alf, Jun 11 '16 at 16:13
I use the default dialog window class @Cheersandhth.-Alf. Sorry for replying late, I was trying to research more information about custom dialog classes but could not find anything — , Jun 11 '16 at 16:27
Why are we arguing about style? Is this really constructive to figuring out this weird crash? Anyway I don't see anything immediately wrong, apart from your `WM_COMMAND` handler ending the dialog, but that probably isn't the problem. The vtable pointer is `0xCCCCCCCC`, which indicates an uninitialized variable. I'm not sure how this is even happening. — andlabs, Jun 11 '16 at 18:22
Shot in the dark: what is your `this` pointer and what pointer are you getting back instead? — andlabs, Jun 11 '16 at 18:26
I don't have my Petzold book available, but I think you'd need to use a custom window class for your approach. Instead, just use `SetProp` and `GetProp`. Or you can probably use the shell subclassing function, but I don't know how smart that function is wrt. dialog boxes.z — Cheers and hth. - Alf, Jun 11 '16 at 21:22
Uh... `DWLP_USER` is documented as being the same as `GWLP_USERDATA` except for dialog boxes. I'm using it just fine in my own code. The problem is elsewhere... — andlabs, Jun 11 '16 at 22:48
There's nothing obviously wrong with your code. My guess is the problem is elsewhere - something is randomly trashing memory. Try running your program with full pageheap enabled and see if it shows up anything. — Jonathan Potter, Jun 12 '16 at 03:11

score 2 · Accepted Answer · edited May 23 '17 at 12:24

I pieced together the code that you've shown us, and was able to create a working sample:

dlg.h

#pragma once
#include <Windows.h>

class dlg {
    static INT_PTR CALLBACK DlgProcTmp(HWND hwnd, 
                                       UINT wm, WPARAM wp, LPARAM lp);
    INT_PTR CALLBACK DlgProc(HWND hwnd, UINT wm, WPARAM wp, LPARAM lp);

    bool ismodal;
protected:
    HWND hwndDlg;
    int id;
public:
    //virtual void oncreate(const widget &w) { }
    //virtual void oncmd(const widget &w, int code, int idCntrl, HWND hwnd) { }
    //virtual void onclose(const widget &w) { }
    dlg() { }
    dlg(int id) : id(id) { }
    INT_PTR domodal(HWND hwndOwner = nullptr);
    //widget domodeless(HWND hwndOwner = nullptr, int cmdshow = SW_SHOW);
};

dlg.cpp

#include "dlg.h"


template <class TYPE> static TYPE getwinlong(HWND hwnd, int idx)
{
    return (TYPE) GetWindowLongPtr(hwnd, idx);
}
template <class TYPE> static void setwinlong(HWND hwnd, int idx, TYPE val)
{
    SetWindowLongPtr(hwnd, idx, (LONG_PTR) val);
}


INT_PTR dlg::DlgProcTmp(HWND hwnd, UINT wm, WPARAM wp, LPARAM lp)
{
    dlg *This;

    if (wm == WM_INITDIALOG) {
        This = (dlg *) lp;
        setwinlong(hwnd, DWLP_USER, This);
        //This->oncreate(widget(hwnd));

        return TRUE;
    }
    if ((This = getwinlong<dlg *>(hwnd, DWLP_USER)) != nullptr)
        return This->DlgProc(hwnd, wm, wp, lp);
    return FALSE;
}
INT_PTR dlg::DlgProc(HWND hwnd, UINT wm, WPARAM wp, LPARAM lp)
{
    this->hwndDlg = hwnd;
    switch (wm) {
        case WM_COMMAND:
            //this->oncmd(widget(hwnd), HIWORD(wp), LOWORD(wp), (HWND) lp);
            if (this->ismodal)
                EndDialog(hwnd, LOWORD(wp));
            else
                DestroyWindow(hwnd);
            return TRUE;
        case WM_DESTROY:
            return TRUE;
    }
    return FALSE;
}

INT_PTR dlg::domodal(HWND hwndOwner)
{
    this->ismodal = true;
    return DialogBoxParam(GetModuleHandle(NULL), 
        MAKEINTRESOURCE(id), hwndOwner, dlg::DlgProcTmp, 
        (LPARAM) this);
}

main.cpp

int APIENTRY _tWinMain(HINSTANCE, HINSTANCE, LPTSTR, int)
{
    dlg myDialog(IDD_DIALOG);
    myDialog.domodal();

    return 0;
}

You'll notice that there's a fair bit of the code that has been commented out—in particular, the declarations for and calls to the virtual message-handling functions (onXXX) have been omitted. I did this because I didn't have the definition for the widget type, and I didn't think it was germane to your actual problem.

Turns out it must be, since the code works fine so long as those virtual message-handling functions are not called.

I then added in a stub class for widget as follows:

class widget
{
public:
   widget(HWND hWnd)
      : m_hWnd(hWnd)
   { }

   HWND m_hWnd;
};

and uncommented the oncmd function as well as the call to it in the WM_COMMAND handler. Again, the code compiles and works correctly. So I have no idea what problem you're experiencing. It must be in the widget class or some other code you have not shown to us.

There is at least one tweak I would make to the code. Assign the hwndDlg member variable inside of the DlgProcTemp function, rather than waiting until the DlgProc function. So, do this:

INT_PTR dlg::DlgProcTmp(HWND hwnd, UINT wm, WPARAM wp, LPARAM lp)
{
    dlg *This;

    if (wm == WM_INITDIALOG) {
        This = (dlg *) lp;
        setwinlong(hwnd, DWLP_USER, This);
        This->hwndDlg = hwnd;
        This->oncreate(widget(hwnd));

        return TRUE;
    }
    if ((This = getwinlong<dlg *>(hwnd, DWLP_USER)) != nullptr)
        return This->DlgProc(hwnd, wm, wp, lp);
    return FALSE;
}
INT_PTR dlg::DlgProc(HWND hwnd, UINT wm, WPARAM wp, LPARAM lp)
{
    switch (wm) {
        case WM_COMMAND:
            this->oncmd(widget(hwnd), HIWORD(wp), LOWORD(wp), (HWND) lp);
            if (this->ismodal)
                EndDialog(hwnd, LOWORD(wp));
            else
                DestroyWindow(hwnd);
            return TRUE;
        case WM_DESTROY:
            return TRUE;
    }
    return FALSE;
}

Also strongly consider using C++-style casts in preference to C-style casts.

If you are still having a problem getting the code to work, you will have to edit into your question a Minimal, Complete, and Verifiable example, just like I have done here.

GetWindowLongPtr returns garbage

1 Answers1