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):
; 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.
It shouldn't fail. The problem is elsewhere. Post your complete code, and maybe we can fix it.
OK, here it is in all its glory: this is my WM_COMMAND handler for File--> Open:
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.
For ReadFile, lpOverlapped and lpNumberOfBytesRead cannot both be NULL.
http://msdn.microsoft.com/en-us/library/aa365467(VS.85).aspx
Ah, yes, it's right there (I missed it earlier):
QuoteThis parameter can be NULL only when the lpOverlapped parameter is not NULL.
I changed it:
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
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.
i usually use CreateFile to open files
first, i use the following equates
they combine the dwDesiredAccess and dwCreationDisposition flags into one dword
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
;******************************************************************************************
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
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
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
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
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.
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
Quote from: dedndave on October 12, 2011, 05:36:13 PMbut, 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?
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 ?
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
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....
push eax
mov edx,esp
INVOKE ReadFile, FileHandle, heapPtr, fileSize, edx, NULL
pop ecx ;ECX = number of bytes read
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.)
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?
Quote from: NoCforMe on October 12, 2011, 06:17:17 PMsimply 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
invoke MessageBox, hWin, fileshowmsg, addr InfoCaption, MB_OK
try ths instead...
invoke MessageBox, hWin, addr fileshowmsg, addr InfoCaption, MB_OK
btw...
my code takes 4 bytes
when you are done, the value is in ECX where it can be tested for error conditions
after that, the value is of little use
i don't care how you do it, it will take more bytes to accomplish the same by any other means
but, that's not an important issue
I don't like using them because they generate code that I can't see. (Well, I can see it in the listing, but not in the source.)
I want to write assembly language, not a higher-level-looking code.
It's a matter of personal style. I strive to make my code as understandable as possible (to me, at any rate).
Pure assembly language will always be somewhat "spaghetti"-like by virtue of the fact that we HAVE to use "gotos" to get anywhere.
Short answer: I don't like using IF-ELSEIF in my code. I may learn to like it later.
Quote from: dedndave on October 12, 2011, 06:36:46 PM
invoke MessageBox, hWin, fileshowmsg, addr InfoCaption, MB_OK
try ths instead...
invoke MessageBox, hWin, addr fileshowmsg, addr InfoCaption, MB_OK
Doh! Doh! Doh! (Bangs head on keyboard)
Thank you! I owe you one, buddy.
not only that - it displays the right stuff :U
OK, to maybe wrap this thread up:
The reason for my posting turned out to be a really STUPID mistake on my part. But to address the topic, I've found that heap allocation works.
However, looking through the MASM32 examples, I see other methods used. For instance, the "dibfile" example uses GlobalAlloc() instead.
The MSDN memory-management explanation of heap functions (http://msdn.microsoft.com/en-us/library/aa366711.aspx) says "New applications should use the heap functions instead of the global and local functions for this purpose."
I guess I'll use the heap for now. (This is for reasonably small allocations; no 4GB database files.)
Quote from: NoCforMeThis is for reasonably small allocations; no 4GB database files.
Any process that has to load an entire file into memory to work doesn't scale well. I might have 16GB in my desktop, my customers almost certainly don't, and a bunch will likely have less than a GB of physical memory.
also, some functions specifically state which type of memory allocation should be used
QuoteFor instance, the "dibfile" example uses GlobalAlloc() instead.
not able to see that code, but i know, for example, that the clipboard functions want you to use GlobalAlloc
probably a similar situation for the dibfile example
GlobalAlloc isn't going to go away, as much as ms may like to do so
HeapAlloc is my favorite because it is fast and makes for small code
but GlobalAlloc, LocalAlloc, and VirtualAlloc all have their uses
Or Map the file and then it won't take up more physical memory than you touch, and won't consume page file space because it's already pinned/associated with a file. Presuming of course you only want to read the data, and not modify it in-place.
Dave is really good at spotting such goodies. Which, by the way, become clear only if you see the full code because...
fileshowmsgTXT DB "File contents: "
fileshowmsg dd fileshowmsgTXT
... this is a legal option not requiring the ADDR keyword before fileshowmsg.
The headers from .386 to msvcrt.lib can be replaced with a single line:
include \masm32\include\masm32rt.inc
(and certainly, that is a matter of taste)
that would be ideal
if he were to modify his masm32rt.inc file, we could assemble his programs without changing anything