Back to problems: weird data corruption

[NoCforMe]

Working on my EdAsm editor, came across a weird data-corruption problem, and solved it, but still have no idea why it happened in the first place.

Here's the very opening code of that program, the first instructions that get executed:

Code Select Expand


;============================================
; CODE LIVES HERE
;============================================
.code

start: INVOKE GetModuleHandle, NULL
MOV InstanceHandle, EAX

; INVOKE InitmcLB, InstanceHandle

CALL WinMain
INVOKE ExitProcess, EAX

;====================================================================
; Mainline proc
;====================================================================

WinMain PROC
LOCAL msg:MSG, brush:HBRUSH, wX:DWORD, wY:DWORD, gpRect:RECT, accel:ACCEL
LOCAL xPos:DWORD, ncm:NONCLIENTMETRICS, editTabDist:DWORD, buffer[184]:byte

INVOKE InitmcLB, InstanceHandle

INVOKE LoadLibrary, OFFSET RichEditDLLname


Pretty ordinary, right? So here's what's going on: InitmcLB (hinst) is the subroutine that initializes my mcListbox custom control, which is use in the "goodies" dialog in this program. The main thing it does is to register the class for those controls, using RegisterClassEx(). One thing that function needs is the instance handle of the executable, which I pass in as you can see.

Well, that dialog was failing because it couldn't load the custom control because InitmcLB (hinst) failed because RegisterClassEx() failed. (GetLastError() told me "The parameter is incorrect." Gee, thanks a lot, Windows ... what parameter?) After a lot of frustration I traced the problem to 1) where I put the InitmcLB() statement and 2) the size of the buffer I was allocating there at the end of the LOCAL statement.  WTF?!?!?

It turned out that if I had that statement inside WinMain() as shown AND the buffer was any larger than 184 bytes the initialization routine would fail. So I assumed that the instance handle I was passing in (InstanceHandle, a global), was getting corrupted somehow. But it wasn't! Now it was really WTF!?!?!?!?

Something in InitmcLB() is getting corrupted, but what? This function is very far away from this starting code. Is the stack getting messed up somehow? And why does it happen when the buffer>184 bytes? I have functions in my code that have much, much more local data than here and they work fine; no stack corruption.

I though maybe there was a limit to the amount of LOCAL data one can have, but I don't think so: the standard prologue to a function just takes however much local data you've allocated and adds it to (subtracts it from) ESP:

Code Select Expand


00401017  Ú$ 55             PUSH EBP                                             ; EdAsm.00401017(guessed void)
00401018  ³. 8BEC           MOV EBP,ESP
0040101A  ³. 81C4 ACFDFFFF  ADD ESP,-254


This is one of the weirdest problems I've run into in a long time. I could post the code that's getting corrupted, but it's really nothing special: just does a RegisterClassEx() and a few other things. Frustrating ...

Anybody got a clue?

[zedd151]

I may not have a clue sometimes    but I do have some ideas.

Increase the stack size in link options? That is if you think that inadequate stack space is the issue. (doubtful unless there are many recursive calls, stretching the stack to its limits) Otherwise look for other places stack might get corrupted...Seems it is getting overwritten somewhere. Did you miss a pop for a push anywhere?

I would keep an eye on that local in ollydbg and step the code to find where it's getting overwritten..could take some time though

Another idea temporarily make that buffer global, and see if the stack (other locals) get corrupted..
Also are you certain that you are not exceeding the buffer size when writing to it?
In other words check all the bases regarding ebp & esp


As another test, get the instance handle inside the procedure and check if problem remains... other than all of that I wouldn't know where else to look... without seeing more code.

[jj2007]

Post the code, please - I am curious!

[NoCforMe]

I would keep an eye on that local in ollydbg and step the code to find where it's getting overwritten..could take some time though

Maybe you missed the part where I wrote that that variable isn't being overwritten; that's the weird thing. I know this because I displayed it inside the init routine, and it was just what it was supposed to be. And yet the RegisterClassEx() call failed.

Regarding that buffer, it isn't even being used. At all. (Leftover from debugging.) Besides, I pointed out that the code I posted above is the first code that's executed anywhere in this program, so no buffer writes or reads.

OK, for those who want the gory details, here's the code:

Code Select Expand


;====================================================================
; InitmcLB (hInst)
;
; Needs to be called before creating MLB controls.
; Registers control class, does other startup stuff.
;====================================================================

InitmcLB PROC hInst:HINSTANCE
LOCAL wc:WNDCLASSEX, ncm:NONCLIENTMETRICS
; local buffer[64]:byte

; See if we're already initialized--if so, exit:
CMP InitializedFlag, TRUE
JE exit99

; invoke wsprintf, addr buffer, offset hinstfmt, hInst
; invoke MessageBox, NULL, addr buffer, NULL, MB_OK

; Allocate heap memory for this control class:
INVOKE GetProcessHeap
MOV HeapHandle, EAX
; Error checking???????

INVOKE HeapAlloc, EAX, 0, $masterlistSize
MOV MasterListHeap, EAX

; Clear WNDCLASSEX structure:
INVOKE FillMem, ADDR wc, SIZEOF WNDCLASS, 0

; Register class for all mcLB controls:
MOV wc.cbSize, SIZEOF WNDCLASSEX
MOV wc.style, CS_HREDRAW or CS_VREDRAW or CS_BYTEALIGNWINDOW or CS_DBLCLKS
MOV EAX, hInst
MOV wc.hInstance, EAX
MOV wc.lpfnWndProc, OFFSET mcLBproc
INVOKE GetStockObject, WHITE_BRUSH
MOV wc.hbrBackground, EAX
MOV wc.lpszClassName, OFFSET mcLBclassName
INVOKE LoadCursor, NULL, IDC_ARROW
MOV wc.hCursor, EAX
INVOKE RegisterClassExA, ADDR wc

;***** BOOM! This (above) is what fails. *****

; call ShowLastError

; Initialize control-finder structure:
MOV ECX, $maxCtrls
LEA EDX, CtrlFinder
fmcs: MOV [EDX].$ctrlFinder.mcsPtr, -1
ADD EDX, SIZEOF $ctrlFinder
LOOP fmcs

MOV NumCtrls, 0

; Create font for control (using SystemParametersInfo(SPI_GETNONCLIENTMETRICS) ):
MOV ncm.cbSize, SIZEOF NONCLIENTMETRICS
INVOKE SystemParametersInfo, SPI_GETNONCLIENTMETRICS, SIZEOF NONCLIENTMETRICS,
ADDR ncm, 0
INVOKE CreateFontIndirect, ADDR ncm.lfStatusFont ;Grab status font.
MOV DefMLBfont, EAX

; Create pen for drawing grid:
INVOKE CreatePen, PS_SOLID, $gridPenWidth, $defGridClr
MOV GridPenHandle, EAX

; Indicate we're ready to go:
MOV InitializedFlag, TRUE

exit99: RET

InitmcLB ENDP


As you can see, not much to see here. I wouldn't worry at all about the heap memory allocation stuff; this happens after the RegisterClassEx() call, and it never fails. The other stuff is trivial.

Keep in mind that this code is not in the editor program; it's in the custom control module, and is called from the editor.

[TimoVJL]

change

Code Select Expand

INVOKE FillMem, ADDR wc, SIZEOF WNDCLASSEX, 0so rest of structure is cleared too, where wc.hIconSm is.

[NoCforMe]

Yes, that's true. It's also OK. From MSDN:

hIcon:
A handle to the class icon. This member must be a handle to an icon resource. If this member is NULL, the system provides a default icon.

Thanks for playing, though.

I see you edited your reply (perfectly OK). That call does clear the entire structure, since it starts at ADDR wc. That's the whole idea. Then I fill in the stuff I need.

[jj2007]

I see you edited your reply (perfectly OK). That call does clear the entire structure, since it starts at ADDR wc.

Code Select Expand

INVOKE FillMem, ADDR wc, SIZEOF WNDCLASS, 0

I could be wrong, but it seems that you are clearing WNDCLASS bytes of a WNDCLASSEX structure (which is 8 bytes longer)

lpszClassName is set later, so can't be responsible. But, as Timo spotted already, hIconSm may contain garbage.

[NoCforMe]

Bingo!

I propose that both you & Timo share the generous cash prize to be awarded by NoCforMe, GmbH, for solving this.

Seems obvious now, but I sure missed that. I'm guessing that having that extra stuff allocated on the stack must have leaked over to the uninitialized part of WNDCLASSEX.

[jj2007]

Did you know that SIZEOF is redundant here?

INVOKE FillMem, ADDR wc, WNDCLASSEX, 0 is perfectly valid code.

[NoCforMe]

No, I didn't know that; so a structure name is equivalent to its size in code, eh? Who knew?

Thanks but no thanks; I know it would save a little typing (7 characters, exactly), but I'll stick with using SIZEOF for the sake of clarity and understanding. Especially when I return to my code a year or two later and wonder if I didn't actually mean ADDR <structure>*. Sometimes a little less brevity is a good thing.

* Which of course wouldn't make sense, as you'd only use ADDR with an instance of a structure, not its defined name, but still.

[jj2007]

It's a matter of taste, of course. I discovered that some years ago, and since then I use it, knowing that the name is just a number.

[NoCforMe]

Heh, reminds me of a 60s theme song:

Secret Agent man, Secret Agent man,
They're givin' you a number, and taking 'way your name.

[hutch--]

The win with SIZEOF is that it is calculated at assembly time, not runtime. Effectively you get the calculation for nothing, just the end result.

[NoCforMe]

Using the name of the structure without SIZEOF is also calculated by the assembler, not at runtime.