Memory problems: heap allocation

[NoCforMe]

It's time to have fun and games with memory allocation.

So I'm trying to allocate a buffer from the system heap in order to read a file into it.

I've got the following (abridged version):

Code Select Expand



; Get handle to heap memory to allocate later:
INVOKE GetProcessHeap
MOV HeapHandle, EAX

...
; (fileSize comes from opening a file and calling GetFileSize() )
INVOKE HeapAlloc, HeapHandle, 0,  fileSize
MOV heapPtr, EAX ;Save pointer to allocated heap

...
; attempt to read file into allocated memory:
INVOKE ReadFile, FileHandle, heapPtr, fileSize, NULL, NULL



According to the return values, the memory allocation itself (HeapAlloc() ) succeeds. But the program throws an exception when I try to read the file into the allocated memory.

I suppose it's possible the exception comes from some other problem with ReadFile(), but I strongly suspect the memory allocation as the cause.

Any clues to what I'm doing wrong here?

Heap memory allocation seemed the right way to go here. I know there are other methods: should I be using these instead?

When I get a pointer to heap memory, is it just a flat 32-bit pointer I'm free to use? No dicking around with segment registers?

This fails even for small file sizes.

[jj2007]

It shouldn't fail. The problem is elsewhere. Post your complete code, and maybe we can fix it.

[NoCforMe]

OK, here it is in all its glory: this is my WM_COMMAND handler for File--> Open:

Code Select Expand



do_open:
MOV heapPtr, NULL
INVOKE OpenFileDlg, hWin, ADDR fileOpenName
; Look at result from GetOpenFileName() to see if user selected a file
; (result will be zero if user cancels, doesn't select a file or an error occurs):
OR EAX, EAX
JZ open80
INVOKE CreateFile, ADDR fileOpenName, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,
FILE_ATTRIBUTE_NORMAL OR FILE_FLAG_RANDOM_ACCESS, NULL
; See if file open succeeded:
CMP EAX, INVALID_HANDLE_VALUE
JNE open20
INVOKE MessageBox, hWin, ADDR FileOpenErrMsg, ADDR ErrorCaption, MB_OK
JMP open80

open20: MOV FileHandle, EAX
INVOKE GetFileSize, FileHandle, NULL
MOV fileSize, EAX
add eax, 1000 ;desperate attempt to fix ...
INVOKE HeapAlloc, HeapHandle, 0,  eax
; Check that memory allocation succeeded:
OR EAX, EAX
JNZ open30
invoke MessageBox, hWin, memfailmsg, addr ErrorCaption, MB_OK
jmp short closeit

open30:
MOV heapPtr, EAX ;Save pointer to allocated heap
INVOKE ReadFile, FileHandle, heapPtr, fileSize, NULL, NULL
OR EAX, EAX
JNZ open40
invoke MessageBox, hWin, ADDR readfailmsg, ADDR ErrorCaption, MB_OK
jmp short doclose
open40:
if 0
push esi
push edi
mov esi, heapPtr
mov edi, offset fileshowarea
mov ecx, 10
rep movsb
pop edi
pop esi
endif
invoke MessageBox, hWin, fileshowmsg, addr InfoCaption, MB_OK

closeit:
INVOKE CloseHandle, FileHandle
open80: CMP heapPtr, NULL
JNZ open90
INVOKE HeapFree, HeapHandle, 0, heapPtr
open90: XOR EAX, EAX
RET



Note about my coding style: All variables that start with a capital letter (e.g., FileHandle) are global, otherwise local (heapPtr).

It blows up on the call to ReadFile().

I tried padding the memory allocation by 1000 bytes, but no joy.

[MichaelW]

For ReadFile, lpOverlapped and lpNumberOfBytesRead cannot both be NULL.

http://msdn.microsoft.com/en-us/library/aa365467(VS.85).aspx

[NoCforMe]

Ah, yes, it's right there (I missed it earlier):

This parameter can be NULL only when the lpOverlapped parameter is not NULL.

I changed it:

Code Select Expand



INVOKE ReadFile, FileHandle, heapPtr, fileSize, ADDR numBytesRead, NULL



Still blows up. Nothing wrong with the memory; I can copy stuff to and from it.



+++++++++++++++++++++++++++++++++++++++++++++++++++

(Later, in another part of the forest ...)

Interesting developments: if I do this after the read

Code Select Expand



INVOKE ReadFile, FileHandle, heapPtr, 20, ADDR numBytesRead, NULL
push eax
invoke MessageBox, hWin, addr heremsg, NULL, MB_OK
pop eax
OR EAX, EAX
JNZ open40
invoke MessageBox, hWin, ADDR readfailmsg, ADDR ErrorCaption, MB_OK



everything is fine (at least I don't get an exception), unless I try to access the memory I've read the file into; then it blows up. It's as if ReadFile() was doing something to the memory that makes it off-limits to me. ?????

Further investigation: I don't even have to do the MessageBox(); all I have to do is not touch that memory and everything's OK.

[dedndave]

i usually use CreateFile to open files

first, i use the following equates
they combine the dwDesiredAccess and dwCreationDisposition flags into one dword

Code Select Expand

OPF_FILECREATE EQU GENERIC_WRITE or CREATE_ALWAYS                  (40000002h)
OPF_FILEWRITE  EQU GENERIC_WRITE or OPEN_EXISTING                  (40000003h)
OPF_FILEREAD   EQU GENERIC_READ or OPEN_EXISTING                   (80000003h)
OPF_FILERANDOM EQU GENERIC_READ or GENERIC_WRITE or OPEN_EXISTING  (C0000003h)

then, i use this little routine to open files

Code Select Expand

;******************************************************************************************

        OPTION  PROLOGUE:None
        OPTION  EPILOGUE:None

OpnFile PROC    lpFileName:LPSTR,dwOpenFlags:DWORD

;file open function

        mov     eax,[esp+8]
        xor     ecx,ecx
        movzx   edx,al
        and     eax,0C0000000h
        and     dl,3
        INVOKE  CreateFile,[esp+28],eax,FILE_SHARE_READ,ecx,edx,FILE_ATTRIBUTE_NORMAL,ecx
        ret     8

OpnFile ENDP

        OPTION  PROLOGUE:PrologueDef
        OPTION  EPILOGUE:EpilogueDef

;******************************************************************************************

that makes the open INVOKE much simpler   :bg

Code Select Expand

        INVOKE  OpnFile,lpFileName,OPF_FILEREAD

of course, the file should exist for read   :P
make sure you get a non-zero handle after CreateFile

as far as your allocation and ReadFile code, it looks like it ought to be ok

Code Select Expand

INVOKE ReadFile, FileHandle, heapPtr, fileSize, ADDR numBytesRead, NULL

i see no error checking at all
what i mean to say is, when you call functions like HeapAlloc, CreateFile, and so on,
check the contents of EAX, against the documented "Return Value" for that function

if HeapAlloc returns a 0, any function that tries to access that memory is going to fail
if you try to read from a file with a handle of 0, it will fail
you can stick MessageBox or Beep or something in there to let you know where the error is

Code Select Expand

        INVOKE  OpnFile,lpFileName,OPF_FILEREAD
        or      eax,eax
        jnz     Continue

        INVOKE  GetLastError
        xor     ecx,ecx
        INVOKE  FormatMessage,FORMAT_MESSAGE_FROM_SYSTEM,ecx,eax
                ecx,offset ErrorBuffer,sizeof ErrorBuffer,ecx
        INVOKE  MessageBox,hWin,offset ErrorBuffer,offset szAppName,MB_OK or MB_ICONERROR
        INVOKE  ExitProcess,0

Continue:
        mov     hFile,eax
of course, rather than repeating the error code 100 times throughout your program, you can put it in a routine and call it
with a little work, you can put the fail address or some other unique identifier string in the message box title

[NoCforMe]

Did you even look at my code?

I'm checking the return values for GetOpenFileName() (through my subroutine OpenFileDlg() ), CreateFile() (used to open the file), HeapAlloc() and ReadFile(). The only one that fails is the latter (confirmed by debugging code).

Please try to address the problem I'm having, and not the stylistic aspects of my code.

[dedndave]

yes - and i read your post, too - lol

but, if you want to learn how to debug your code, you will want to know how to check for errors
if your ReadFile call fails, use GetLastError to find out why

in the future, i'll try not to give you any additional info   :U

[NoCforMe]

but, if you want to learn how to debug your code, you will want to know how to check for errors
if your ReadFile call fails, use GetLastError to find out why

Maybe I didn't make myself clear, in which case I apologize.

First of all, ReadFile() is NOT failing. I'm checking the return value; it's right there in my code. If it fails, I show a MessageBox and exit. (That has never happened in my code.)

The problem is not ReadFile() itself; it's the fact that the memory I've allocated somehow becomes "contaminated" or off-limits to me AFTER the call. In other words, if I call ReadFile() but don't do anything with the allocated memory, everything's cool. As soon as I try to access one byte of that memory, BOOM! The program crashes (and I can't debug that, as Windows shuts me down).

Does that make sense?

[dedndave]

well - we can't see the entire program
so, we have to make some educated guesses

assuming that you got a non-zero value from HeapAlloc....

perhaps the ReadFile call overwrites the heap allocation pointer ?
that would cause something like what you are describing

maybe the NumberOfBytesRead dword overwrites the heap pointer ?

[jj2007]

There is a little problem in your code. And there is a little problem with the Forum members:

1. Most are no geniuses, and therefore unable to spot where your error happens; we are used to run a program, adding little debug macros here and there, and once we have an idea where the error sits, we insert int 3 and launch Olly.

2. All of us, without exception, are too lazy to build a full app around your WM_xx handler. Either you post your complete code, zipped and with all necessary resource files, or you will get hardly any feedback.

Don't take it personal, please. These are simply habits that are based on a lot of experience here in the Forum.
:thumbu

[dedndave]

Code Select Expand

INVOKE ReadFile, FileHandle, heapPtr, fileSize, NULL, NULL

the second-to-last parameter, lpNumberOfBytesRead, should point to a dword variable that receives the number of bytes read
i generally make a temp spot on the stack....

Code Select Expand

        push    eax
        mov     edx,esp
        INVOKE  ReadFile, FileHandle, heapPtr, fileSize, edx, NULL
        pop     ecx     ;ECX = number of bytes read

[NoCforMe]

No offense taken, really.

The reason I didn't attach the whole program is that I wasn't quite at the point where I wanted to plead with someone to please! debug my code!

I'm quite willing to post what I've got. (I'll even edit the INCLUDEs  and INCLUDELIBs so that they play nice with the default MASM32 installation.)

I'm quite open to the possibility that there's something else in my code that's screwing things up. After all, I'm obviously doing something wrong; the object of the game is to hunt down that error and kill it.

So I'll post it. Please be advised that I do some things in an idiosyncratic way. I do not want to be talked out of every way of doing things that happens to differ from the way you do things (whoever you may be). However, I would like to know if one of these things is causing my program to bomb!

Thanks in advance to whoever takes the time to look at this.

(If you want to look at this, the program needs the attached resource file. I've included the .obj file as well as the .rc file.)

[NoCforMe]

With all due respect, Dave, how is that any different than simply allocating a LOCAL variable and using it (which is what I did)? The difference is that your code is more obscure (and actually uses more space, as stack variables are automatically allocated on entry to the routine). Do you really have an objection to using named variables in your code?

[qWord]

simply allocating a LOCAL variable [...]  The difference is that your code is more obscure

If you think so, why are you producing spaghetti code instead of using  MASM's highlevel constructs?  :U