Runtime error with member function pointers
Hi all,
I need some expert advice. I had the following code running but after moving it from one file to several files (because the code is being merged in another branch) it somehow broke and I get this runtime error:
Run-Time Check Failure #0 - The value of ESP was not properly saved across a function call. This is usually a result of calling a function declared with one calling convention with a function pointer declared with a different calling convention.
I am quite sure it has something to do with the member function pointers because when I comment out these lines
mainButton->SetOnDownFunction(&CDiagnoseWindow::OnMainButtonDown, this);
infoButton->SetOnDownFunction(&CDiagnoseWindow::OnInfoButtonDown, this);
graphButton->SetOnDownFunction(&CDiagnoseWindow::OnGraphButtonDown, this);
the error disappears.
I read on several places that it might have something to do with using a wrong calling convention. I have no idea if I am doing that nor to what it should be changed. This is only my first code with member function pointers (to get a grip on them) so I can not rely on any experience.
All help is appreciated
regards,
Jef
This is a short version of the code:
diagnosewindow.cpp (partial)
#include "DiagnoseWindow.h"
...
#include "DiagNavButton.h"
CDiagnoseWindow::CDiagnoseWindow(coord_t win_size) : CWindow("diag win", win_size, POPUP_VALUE_FRAME_PRIORITY+1)
{
int i;
this->SetSize(win_size);
SetRoundCorners(true);
SetBorderColor(BLACK);
currentLayer = LAYER_MAIN;
//Main
mainButton = new CDiagNavButton(Coord(240,1), P_TAB_MAIN);
mainButton->SetOnDownFunction(&CDiagnoseWindow::OnMainButtonDown, this);
AddWidget(mainButton);
infoButton = new CDiagNavButton(Coord(240,37), P_TAB_INFO);
infoButton->SetOnDownFunction(&CDiagnoseWindow::OnInfoButtonDown, this);
AddWidget(infoButton);
graphButton = new CDiagNavButton(Coord(240,74), P_TAB_DIAG_GRAPH);
graphButton->SetOnDownFunction(&CDiagnoseWindow::OnGraphButtonDown, this);
AddWidget(graphButton);
...
}
...
void CDiagnoseWindow::OnMainButtonDown(void)
{
...
}
void CDiagnoseWindow::OnInfoButtonDown(void)
{
...
}
void CDiagnoseWindow::OnGraphButtonDown(void)
{
...
}
diagnosewindow.h (partial)
class CDiagnoseWindow : public CWindow
{
public:
CDiagnoseWindow(coord_t win_size);
~CDiagnoseWindow() {}
void InitSPN(int diagItem, int SPN);
void SetActiveItem(uint8 activeItem);
void UpdatePage(bool down);
private:
void EraseDataRegion();
void OnRefresh(void);
void OnOpen(void);
void OnClose(void);
virtual bool OnExternalTouch(uint32 button_id, event_e event_id);
void ChangeTitle(int SPN);
void OnMainButtonDown(void);
void OnInfoButtonDown(void);
void OnGraphButtonDown(void);
...
};
diagnavbutton.h (complete)
#ifndef _diag_nav_button_h
#define _diag_nav_button_h
#include "Widget.h"
class CDiagnoseWindow;
class CIcon;
typedef void (CDiagnoseWindow::*PushCallback)(void);
class CDiagNavButton : public CWidget
{
public:
CDiagNavButton(coord_t loc, const CLang* label_id);
~CDiagNavButton() {}
void OnSelect(void);
void OnDeselect(void);
void OnRefresh(CGraphicFrame *frame_p);
void SetOnDownFunction(PushCallback pushCallback, CDiagnoseWindow * parentWindow);
void Push();
private:
void OnOpen(CGraphicFrame *frame_p);
const CLang* label_id;
void OnDown(void);
text_attrib_t text_attrib;
uint8 text_color;
region_t region;
CIcon * button;
CGraphicFrame *diagNavButtonFrame;
bool redrawText;
PushCallback _PushCallback;
CDiagnoseWindow * parentWindow;
};
#endif
diagnavbutton.cpp(complete)
#include "DiagNavButton.h"
#include "Icon.h"
#include "bitmapharvester.h"
CDiagNavButton::CDiagNavButton(coord_t loc, const CLang* label_id) : CWidget("DiagNavButton", region)
{
this->label_id = label_id;
this->SetTouchable(true);
this->SetFocused(true);
region.loc = loc;
region.size.x = 64;
region.size.y = 35;
}
void CDiagNavButton::OnOpen(CGraphicFrame *frame_p)
{
diagNavButtonFrame = frame_p;
button = new CIcon();
button->SetIcon(&bitmap_button_off);
button->Draw_Icon(frame_p, region);
text_color = WHITE;
text_attrib.text_p = CLanguage::GetPhrase(label_id);
text_attrib.unicode_text_p = NULL;
text_attrib.text_style = TEXT_BOLD | TEXT_VCENTER | TEXT_CENTER;
text_attrib.text_size = 16;
frame_p->DrawTextW(region, &text_attrib, WHITE, 0xE6);
redrawText = false;
}
void CDiagNavButton::OnSelect(void)
{
button->SetIcon(&bitmap_button_on);
button->Draw_Icon(diagNavButtonFrame, region);
redrawText = true;
text_color = BLACK;
Push();
}
void CDiagNavButton::Push()
{
if(_PushCallback != NULL)
(parentWindow->*_PushCallback)();;
}
void CDiagNavButton::OnDeselect(void)
{
button->SetIcon(&bitmap_button_off);
button->Draw_Icon(diagNavButtonFrame, region);
redrawText = true;
text_color = WHITE;
}
void CDiagNavButton::OnRefresh(CGraphicFrame *frame_p)
{
if(redrawText)
{
frame_p->DrawTextW(region, &text_attrib, text_color, 0xE6);
redrawText = false;
}
}
void CDiagNavButton::SetOnDownFunction(PushCallback pushCallback, CDiagnoseWindow * parentWindow)
{
_PushCallback = pushCallback;
this->parentWindow = parentWindow;
}
[6003 byte] By [
Jef Patat] at [2007-11-20 10:14:11]

# 1 Re: Runtime error with member function pointers
Does the error occur when you push the buttons?
I may be wrong, but I think the code below will only work if the functions are inline. If you previously declared the OnXXXXButtonDown functions directly in the class, this could explain your problem.
void CDiagNavButton::Push()
{
if(_PushCallback != NULL)
(parentWindow->*_PushCallback)();
}
zerver at 2007-11-10 22:30:26 >

# 2 Re: Runtime error with member function pointers
Does the error occur when you push the buttons?
No the error occurs when the constructor is executed. It brakes at the final bracket of the CDiagnoseWindow constructor. Removing the lines with SetOnDownFunction makes the error disappear.
I may be wrong, but I think the code below will only work if the functions are inline. If you previously declared the OnXXXXButtonDown functions directly in the class, this could explain your problem.
void CDiagNavButton::Push()
{
if(_PushCallback != NULL)
(parentWindow->*_PushCallback)();
}
I think this is not correct. The functions were declared and used exactly the same, just in other files. Also it is not crashing at this point but in the constructor of CDiagnoseWindow.
thank you
Jef
# 3 Re: Runtime error with member function pointers
Step one line at a time using the debugger to see exactly on which line the error occurs.
zerver at 2007-11-10 22:32:30 >

# 4 Re: Runtime error with member function pointers
That I did before posting :cool: everything went fine until the final bracket of the constructor.
In the meantime I found out that my information posted before was not completely correct. I did change a bit to the code. I moved the function declaration of SetOnDownFunction to the cpp file. Previously both definition and declaration were in the header file.
so in diagnavbutton.cpp the was no SetOnDownFunction
and in diagnavbutton.h there was
...
void OnRefresh(CGraphicFrame *frame_p);
void SetOnDownFunction(PushCallback pushCallback, CDiagnoseWindow * parentWindow)
{
_PushCallback = pushCallback;
this->parentWindow = parentWindow;
}
void Push();
...
instead of
...
void OnRefresh(CGraphicFrame *frame_p);
void SetOnDownFunction(PushCallback pushCallback, CDiagnoseWindow * parentWindow);
void Push();
...
I found out that moving the declaration back to the header file fixes the problem. That's great! But can anybody explain why I won't work with the declaration in the cpp file?
I may be wrong, but I think the code below will only work if the functions are inline. If you previously declared the OnXXXXButtonDown functions directly in the class, this could explain your problem.
I guess this indeed is my problem, but can you explain further as to why?
thanks for the time and effort !
regards,
Jef
# 5 Re: Runtime error with member function pointers
Does the error occur when you push the buttons?
I may be wrong, but I think the code below will only work if the functions are inline. If you previously declared the OnXXXXButtonDown functions directly in the class, this could explain your problem.
void CDiagNavButton::Push()
{
if(_PushCallback != NULL)
(parentWindow->*_PushCallback)();
}
sorry, but now you indeed seem to be correct. It now crashes when the button is pressed. Which now seems to be because parentWindow is lost. I'll look a bit further into this.
# 6 Re: Runtime error with member function pointers
Ok, I got it all fixed, but two questions remain to me:
Why does it not work when the function declaration is not in the header file?
Why do I need to include #include "DiagnoseWindow.h" in navbutton.h? It compiles without but then parentWindow is set to 0xfdfdfdfd and can not be changed in SetOnDownFunction. How can this make a difference?
I hope you professionals can answer these questions.
thanks in advance,
Jef
# 7 Re: Runtime error with member function pointers
In the meantime I found out that my information posted before was not completely correct. I did change a bit to the code. I moved the function declaration of SetOnDownFunction to the cpp file. Previously both definition and declaration were in the header file.
....
I found out that moving the declaration back to the header file fixes the problem. That's great! But can anybody explain why I won't work with the declaration in the cpp file?This is *not* great!
All you're doing when you move code around is move the bug to another part of the code. You are describing the classic scenario of a memory corruption bug -- move the code here, delete a code there, and the bug goes away (where the code has absolutely nothing to do with fixing any bugs).
Seriously, what you should do is move the code back to where the bug reappears and actually fix the problem. Right now, you haven't fixed anything, and you're masking the bug at this moment, but the bug is still there.
Regards,
Paul McKenzie
# 8 Re: Runtime error with member function pointers
Ok, I got it all fixed,No you haven't fixed it. You're questions below proves this.
Why does it not work when the function declaration is not in the header file?
Why do I need to include #include "DiagnoseWindow.h" in navbutton.h? It compiles without but then parentWindow is set to 0xfdfdfdfd and can not be changed in SetOnDownFunction. How can this make a difference?
If you can't explain the fix, then the problem isn't fixed. As my previous post mentioned, the action of moving or deleting lines of code making bugs mysteriously appear/disappear is the classic sign of a memory corruption bug somewhere else in your code. If you don't identify where this is, then you haven't fixed the problem.
Regards,
Paul McKenzie
# 9 Re: Runtime error with member function pointers
Regarding the memory corruption:
The debugger has an excellent feature "break on data".
Type in the memory address of parentWindow and it will break when it gets corrupted.
Why does it not work when the function declaration is not in the header file?
Perhaps I'm wrong, but this is my thinking:
A pointer to a non-static member function is really nothing more than an offset. When you call the function
(parentWindow->*_PushCallback)();
you tell the system to start executing code at
class instance address + function offset
If the function is declared outside the class, it will not be inline.
This means the code will be allocated globally at some random address (one code instance will be used for all instances of the class).
Therefore, there will be no valid code to execute at class address + offset.
zerver at 2007-11-10 22:38:37 >

# 10 Re: Runtime error with member function pointers
No you haven't fixed it. You're questions below proves this.
If you can't explain the fix, then the problem isn't fixed. As my previous post mentioned, the action of moving or deleting lines of code making bugs mysteriously appear/disappear is the classic sign of a memory corruption bug somewhere else in your code. If you don't identify where this is, then you haven't fixed the problem.
Regards,
Paul McKenzie
I partially agree to your view. The fact that I post my questions means that I am interested in trying to understand what is going on but that I am not able to. I can always spends hours and hours or maybe days and days trying to find out something that is above my knowledge.
But try to convince management of that. You have functional software that has no visable bug, you move it to another branch and do some clean up and it doesn't work. While to you and me that indeed is a strong warning, to management this only proves you're an idiot not knowing what he is doing. They just want the same result as previously.
So to me it is great that everything functions again. That means I can in all peace look to get it 'realy' fixed. Hence my further questions.
At the same time I must say that it is easy to give comments to my way of working, but you do not offer any information to my questions. I have a couple of years of programming experience and the two questions I describe make totally no sense to me. I think it would be more constructive to me, and other readers, if you tried to give an answer instead of criticism.
The debugger has an excellent feature "break on data"
If i set a breakpoint in the constructor of CDiagNavButton and add parentWindow to the watch I see that when #include "DiagnoseWindow.h" is not included in diagnavbutton.h the address is set to 0xfdfdfdfd and when it is included it is set to 0xcdcdcdcd. The 0xcdcdcdcd seems correct, this is what is correct for not initialized pointers. The 0xfdfdfdfd however is something I never saw before. According to Microsoft (http://msdn2.microsoft.com/en-us/library/Aa270812(VS.60).aspx) this is something called "NoMansLand". How I get there and how to get out I have no idea.
If the function is declared outside the class, it will not be inline.
Correct, I just forgot about the terminology. Since it is a member function it is inline by default no? And it does not matter if it's declaration is in the header or cpp file. I now found out that when #include "DiagnoseWindow.h" is included in diagnavbutton.h i can move the function to the cpp file.
So apperently after a lot of moving code around, sorry for that Paul McKenzie :p I think the conclusion is that everything goes bananas, and I go to nomansland, if #include "DiagnoseWindow.h" is not included in diagnavbutton.h.
Any ideas?
# 11 Re: Runtime error with member function pointers
So to me it is great that everything functions again. That means I can in all peace look to get it 'realy' fixed. Hence my further questions.It only seems to be fixed. You don't know when or where the problem will show up again. That is the danger of not knowing how you fixed a problem -- the issue may appear again at a customer site, a demo, who knows?
At the same time I must say that it is easy to give comments to my way of working, but you do not offer any information to my questions.I gave you enough information as I can, given what you provided.
This is not me making things up -- a classic case of a memory corruption bug or a bug with a wild pointer is *exactly* what you're describing. You have code that doesn't work, then you add an innocuous line of code (or lines of code), and then all of a sudden, things start to work. Similarly, you have code that works, remove or add lines that do basically nothing related to the problem, and things start not to work.
I have a couple of years of programming experience and the two questions I describe make totally no sense to me.In C++, you have things called memory corruption bugs. If you corrupt memory, that bug will exist forever until you fix that bug. The bug may show itself at anytime, either after you've changed the source code somewhat, or if you run the program on another machine, or at any random time.I think it would be more constructive to me, and other readers, if you tried to give an answer instead of criticism.What criticism? I'm giving you information as to why you would get such behaviour, and why the problem is never fixed until it is really fixed. Add or remove functionality from your program, and you don't know what will happen if bugs such as I've described are not fixed. You're back to square one again. If you want me to pinpoint where in your code this is occuring, we need your entire code base.
When you change the source code, recompile, you are moving the corruption to another part of the running code. Maybe you will never hit this part of your code when you're program is running -- but you never know where it is, so you are running on blind faith that nothing wrong will happen. In short, the program is not in a stable state.
Regards,
Paul McKenzie
# 12 Re: Runtime error with member function pointers
Also, calling convention shouldn't be an issue. By default, all calls in C++ are __cdecl. The only time when you see the ESP error is if for some reason, your calls were declared as __stdcall, and you're calling them using __cdecl convention, or vice-versa. But I don't see how that can be if you're calling member functions through a pointer in the normal sense.
The only other time I've seen the ESP error is due to (as I mentioned) a wild pointer bug, memory corruption, or memory overwrite error.
So maybe you didn't recompile your entire project? Or maybe your project is not one with just C++ source modules? (i.e. you are using static libraries built seperately or something similar).
Regards,
Paul McKenzie
# 13 Re: Runtime error with member function pointers
I agree to the memory corruption. I have enough experience to know what that is. As you can understand I can not give you proprietary code. To me this is not a wild pointer, it is set correctly to 0xcdcdcdcd or incorrectly to 0xfdfdfdfd depending on the inclusion of a header file. To me that means something is going wrong at compile time, well before the memory is allocated. How can it be corrupted in the first line of the constructor? It is not being overwritten by something else, it is not yet used, it is not even initialized. Equally, at that point the memory can't be corrupted by anything else since it is just allocated. And all that because of an inclusion of a header file?
It doesn't make any sense to me. Including or not including should not change anything to the functionality of the code, should it?
regards,
Jef
# 14 Re: Runtime error with member function pointers
As you can understand I can not give you proprietary code.Then you should create a very simple program, maybe two or three C++ modules, that duplicates this error with member function pointers. Then post it here.
You should also mention what version of Visual C++ you're using.
Regards,
Paul McKenzie
# 15 Re: Runtime error with member function pointers
I was a little afraid that when I was going to isolate the problem it was going to disappear, but at the same time it does not make sense to me so I gave it a try, you never know, right?
Guess what, even with a totally stripped down version, which is not proprietary for sure, it is still reproducable. I've discussed it with several collegues but nobody has any idea.
So here it goes. Commenting the line
#include "DiagnoseWindow.h" // do not remove
in diagnavbutton.h brings up the problem. I'm using visual studio 2005.
Thanks for all help and ideas so far! Even if I judged them as criticism :blush:
regards,
Jef
# 16 Re: Runtime error with member function pointers
I've reproduced the problem with your code.
Here is where I see a difference in when it works and when it doesn't:
Here is the assembly breakdown of the working CDiagNavButton::SetOnDownFunction:
void CDiagNavButton::SetOnDownFunction(PushCallback pushCallback, CDiagnoseWindow * parentWindow)
{
004116F0 push ebp
004116F1 mov ebp,esp
004116F3 sub esp,0D8h
004116F9 push ebx
004116FA push esi
004116FB push edi
004116FC push ecx
004116FD lea edi,[ebp-0D8h]
00411703 mov ecx,36h
00411708 mov eax,0CCCCCCCCh
0041170D rep stos dword ptr es:[edi]
0041170F pop ecx
00411710 mov dword ptr [ebp-8],ecx
_PushCallback = pushCallback;
00411713 mov eax,dword ptr [this]
00411716 mov ecx,dword ptr [pushCallback]
00411719 mov dword ptr [eax],ecx
this->parentWindow = parentWindow;
0041171B mov eax,dword ptr [this]
0041171E mov ecx,dword ptr [parentWindow]
00411721 mov dword ptr [eax+4],ecx
int a=0;
00411724 mov dword ptr [a],0
a++;
0041172B mov eax,dword ptr [a]
0041172E add eax,1
00411731 mov dword ptr [a],eax
}
00411734 pop edi
00411735 pop esi
00411736 pop ebx
00411737 mov esp,ebp
00411739 pop ebp
0041173A ret 8
Here is the assembly language breakdown of the non-working version of the same function:
void CDiagNavButton::SetOnDownFunction(PushCallback pushCallback, CDiagnoseWindow * parentWindow)
{
004116F0 push ebp
004116F1 mov ebp,esp
004116F3 sub esp,0D8h
004116F9 push ebx
004116FA push esi
004116FB push edi
004116FC push ecx
004116FD lea edi,[ebp-0D8h]
00411703 mov ecx,36h
00411708 mov eax,0CCCCCCCCh
0041170D rep stos dword ptr es:[edi]
0041170F pop ecx
00411710 mov dword ptr [ebp-8],ecx
_PushCallback = pushCallback;
00411713 mov eax,dword ptr [this]
00411716 mov ecx,dword ptr [pushCallback]
00411719 mov dword ptr [eax],ecx
0041171B mov edx,dword ptr [ebp+0Ch]
0041171E mov dword ptr [eax+4],edx
00411721 mov ecx,dword ptr [ebp+10h]
00411724 mov dword ptr [eax+8],ecx
00411727 mov edx,dword ptr [ebp+14h]
0041172A mov dword ptr [eax+0Ch],edx
this->parentWindow = parentWindow;
0041172D mov eax,dword ptr [this]
00411730 mov ecx,dword ptr [parentWindow]
00411733 mov dword ptr [eax+10h],ecx
int a=0;
00411736 mov dword ptr [a],0
a++;
0041173D mov eax,dword ptr [a]
00411740 add eax,1
00411743 mov dword ptr [a],eax
}
00411746 pop edi
00411747 pop esi
00411748 pop ebx
00411749 mov esp,ebp
0041174B pop ebp
0041174C ret 14h
Doing a "diff" on these two code samples shows the following differences:
0041171B mov edx,dword ptr [ebp+0Ch]
0041171E mov dword ptr [eax+4],edx
00411721 mov ecx,dword ptr [ebp+10h]
00411724 mov dword ptr [eax+8],ecx
00411727 mov edx,dword ptr [ebp+14h]
0041172A mov dword ptr [eax+0Ch],edx
//...
0041174C ret 14h
The above lines only appear in the bad version. Interestingly, the last line, the line with the "ret" is where things break down. For the good version, the "ret" retuns 8 bytes to the stack, while the bad version returns 14h (20 bytes) to the stack. This is where the corruption occurs.
The function that calls SetOnDownFunction() pushed 8 bytes onto the stack, but the SetOnDownFunction() pops of 20 bytes in the bad version, when it should pop only 8 bytes, as the good version does.
One thing that I was always wary of when using a typedef to specify a non-static class member function is that I always make sure that the class is fully defined before typedefing. In your case, when you removed the header, you had this:
// #include "DiagnoseWindow.h" // do not remove
class CDiagnoseWindow;
typedef void (CDiagnoseWindow::*PushCallback)(void);
Since CDiagnoseWindow is only forward declared, out of habit, I would hesitate using it as part of a typedef. I am not sure if I'm paranoid or if there is a rule that I am not aware of in C++, but I only use forward declared classes for simple pointer and reference parameters and members later on in the header. I never use it for typedeffing member function pointers.
Removing the comment fully defines the CDiagnoseWindow class, and it is now safe to create the typedef for it in the PushCallback function.
So whether the compiler has a bug, or whether using forward declared-only classes in typedefs is safe, I'm not sure at this point. That's the analysis I have of your error.
I think what I've given you in this post, plus your small example, is enough to give to the Microsoft engineers and submit a bug report, and have them figure out whether this is really a bug or not (unless someone here in the interim can confirm whether using forward-declared classes within typedefs of member function pointers is ill-formed, according to ANSI/ISO C++).
Regards,
Paul McKenzie
# 17 Re: Runtime error with member function pointers
Thanks for all the effort Paul!
This is the conclusion I came up on together with a collegue of mine even before I read your comment:
The reason appears to be that the typedef lets the compiler know how the function pointer looks, the forward declaration lets the compiler know parentWindow is a class. In the code parentWindow is only used as a pointer to point to the pushback function. The compiler knows where to point for the class parentWindow and knows what the memory offset is for the function. BUT if the header file inclusion is ommited the compiler has no idea about how big the class parentWindow is in memory and apperently doesn't care, resulting in pointing to an offset in a 'no memory allocated class' which of course results in a crash.
What you are mentioning all completes the picture! Thank you, I really appreciate that!
So whether the compiler has a bug, or whether using forward declared-only classes in typedefs is safe, I'm not sure at this point. That's the analysis I have of your error.
I think what I've given you in this post, plus your small example, is enough to give to the Microsoft engineers and submit a bug report, and have them figure out whether this is really a bug or not (unless someone here in the interim can confirm whether using forward-declared classes within typedefs of member function pointers is ill-formed, according to ANSI/ISO C++).
I'm not sure neither. Does my explanation makes sense? Because then it looks just like bad coding practice. To the compiler everything looks correct I think. It looks like bad dereferencing of a pointer to me. I think we both can think of many other similar examples which are all not compiler related.
regards,
Jef
# 18 Re: Runtime error with member function pointers
I'm not sure neither. Does my explanation makes sense? Because then it looks just like bad coding practice. To the compiler everything looks correct I think. It looks like bad dereferencing of a pointer to me. I think we both can think of many other similar examples which are all not compiler related.Maybe our good moderators can move this thread to the non-Visual C++ forum. Maybe some others there can shed some light on this in terms of what C++ says about incomplete types and typedefs (I can't find the pertinent line in the standard that mentions your scenario).
Regards,
Paul McKenzie