These two functions seem to be secured. I would like to have your advice. What could I improve?
.686p ; create 32 bit code
.MMX ; enable MMX instructions
.XMM ; enable SSE instructions
.NOCREF
OPTION casemap :none ; case sensitive
; -------------------------------------------------------------------------------------------------------------
.Code
; -------------------------------------------------------------------------------------------------------------
; ==================================================================================
ALIGN 16
; ==================================================================================
ReadWholeFile PROC __lpszFileName:LPSTR
LOCAL _lpMemory:LPBYTE
LOCAL _FileHandle:HANDLE
LOCAL _dwFileSize:DWORD
LOCAL _dwTmp:DWORD
INVOKE PathFileExists,__lpszFileName
test eax,eax
jz @Error_01
INVOKE CreateFile,__lpszFileName,GENERIC_READ,0,NULL,OPEN_EXISTING,FILE_ATTRIBUTE_NORMAL,NULL
cmp eax,INVALID_HANDLE_VALUE
je @Error_01
mov _FileHandle,eax
INVOKE GetFileSize,_FileHandle,ADDR _dwTmp
cmp eax,INVALID_FILE_SIZE
je @Error_02
cmp _dwTmp,0
jne @Error_02 ; File is too big
or eax,eax
jz @Error_02 ; File is empty
mov _dwFileSize,eax
INVOKE VirtualAlloc,NULL,eax,MEM_COMMIT + MEM_RESERVE,PAGE_EXECUTE_READWRITE
test eax,eax
je @Error_02
mov _lpMemory,eax
lea edx,_dwTmp
INVOKE ReadFile,_FileHandle,eax,_dwFileSize,edx,NULL
test eax,eax
jz @Error_03
mov eax,_dwFileSize
sub eax,_dwTmp
jnz @Error_03
INVOKE CloseHandle,_FileHandle
mov eax,_lpMemory
ret
; **********************************************************************************
ALIGN 16
; **********************************************************************************
@Error_03 : ; if eax = 0 we have reached the end of the file
push MEM_RELEASE
push 0
push _lpMemory
push OFFSET @Error_02
jmp VirtualFree
; **********************************************************************************
ALIGN 16
; **********************************************************************************
@Error_02 :
push _FileHandle
push OFFSET @Error_01
jmp CloseHandle
; **********************************************************************************
ALIGN 16
; **********************************************************************************
@Error_01 :
xor eax,eax
ret
ReadWholeFile ENDP
; ==================================================================================
ALIGN 16
; ==================================================================================
File_Read PROC __FileHandle:HANDLE,__lpBuffer:LPBYTE,__dwNumberOfBytesToRead:DWord,__lpdwFlag:PTR DWord
LOCAL _dwTmp:DWORD
lea edx,_dwTmp
INVOKE ReadFile,__FileHandle,__lpBuffer,__dwNumberOfBytesToRead,edx,NULL
test eax,eax
jz @Error_01
mov eax,__dwNumberOfBytesToRead
sub eax,_dwTmp
jnz @Error_01
mov eax,__lpdwFlag
mov DWord Ptr [eax],TRUE ; NO ERROR
mov eax,__lpBuffer
ret
; **********************************************************************************
ALIGN 16
; **********************************************************************************
@Error_01 :
cmp _dwTmp,0
jne @Error
mov eax,__lpdwFlag
mov DWord Ptr [eax],FALSE ; END OF FILE
mov eax,__lpBuffer ; End of file
ret ; That is not an error
@Error :
mov eax,__lpdwFlag
mov DWord Ptr [eax],-1 ; ERROR WHILE READING
xor eax,eax
ret
File_Read ENDP
:dazzled:
You use the exact size to allow memory,not very good (risk to read out of the memory ) .
Better is to round at least to the 16 bytes upper.
This avoid hole of memory (useless memory of a few bytes and win time)
You can use the fasted instructions using dword,qword,16 bytes registers.
I use VirtualAlloc, so the size is rounded to a multiple of 4096.
The case you describe could appened, that's true.
Avoiding this case would obliged to add 4096. I could spend a lot of memory.
I found a bug in eof detection.
The EOF is detected when ReadFile returns TRUE and the number of bytes read is 0.
New version (http://www.phrio.biz/download/$File_Read.zip)
To avoid the need of 4096 bytes,globalalloc or heapalloc are perfect.
QuoteNote : The global functions have greater overhead and provide fewer features than other memory management functions. New applications should use the heap functions (https://msdn.microsoft.com/en-us/library/windows/desktop/aa366711(v=vs.85).aspx) unless documentation states that a global function should be used.
But I prefer Virtual Functions.
I have added the SetErrorMode function with the parameter SEM_FAILCRITICALERRORS.
The attachment below is updated. Downloading this file always gives the last version.
You can use VirtualAlloc, no problem. Just use filesize+1 to care for the zero delimiter. It will not waste memory.
Note, though, that
a) for small allocations VA is very slow compared to HeapAlloc
b) for big allocations (>0.5MB) HeapAlloc uses VirtualAlloc internally.
Therefore, to get always the best possible performance, you can use HeapAlloc. Microsoft says that, not me - I just explain why they are saying that ;-)
I try to use HeapAlloc :
add eax,256
push eax
push HEAP_ZERO_MEMORY + HEAP_NO_SERIALIZE
call GetProcessHeap
push eax
call HeapAlloc
Works well, but HeapFree returns 0!
push _lpMemory
push HEAP_NO_SERIALIZE
call GetProcessHeap
push eax
push OFFSET @ReadWholeFile_Error_02
jmp HeapFree
QuoteHEAP_NO_SERIALIZE
This value should not be specified when accessing the process's default heap.
https://msdn.microsoft.com/en-us/library/windows/desktop/aa366597%28v=vs.85%29.aspx (https://msdn.microsoft.com/en-us/library/windows/desktop/aa366597%28v=vs.85%29.aspx)
and, this is crazy code :biggrin:
push OFFSET @ReadWholeFile_Error_02
jmp HeapFree
try this....
INCLUDE \Masm32\Include\Masm32rt.inc
;###############################################################################################
.DATA?
hHeap HANDLE ?
lpBlock LPVOID ?
;###############################################################################################
.CODE
;***********************************************************************************************
main PROC
INVOKE GetProcessHeap
mov hHeap,eax
INVOKE HeapAlloc,eax,HEAP_ZERO_MEMORY,120000
mov lpBlock,eax
print uhex$(eax),13,10,13,10
inkey
INVOKE HeapFree,hHeap,NULL,lpBlock
print uhex$(eax),13,10,13,10
inkey
exit
main ENDP
;###############################################################################################
END main
Quotemov lpBlock,eax[/size]
[/size][/size][size=78%]Rather than making this I made : MOV EAX,lpBlock[/size][/size][size=78%]So when I wanted to free the block the address was FALSE.[/size]
I am pleased to have finished this function!
The last and documented version is available here (http://www.phrio.biz/download/$File_Read.7z)
Thanks everyone for your help. (Dedndae, Jj2007, ToutEnAsm)
aligning the return addresses isn't really necessary...
push OFFSET @Error_02
jmp HeapFree
; **********************************************************************************
ALIGN 4
; **********************************************************************************
@Error_02 :
push _FileHandle
push OFFSET @Error_01
jmp CloseHandle
; **********************************************************************************
ALIGN 4
; **********************************************************************************
@Error_01 :
while it may speed up the RET (slightly), it is probably slower than CALL
In the last test JJ2007 did this code is quicker on AMD CPU
Quote
push _FileHandle
push OFFSET @Error_01
jmp CloseHandle
Agner Frog tells that if a
call is followed by a
jump, it can be
replaced by a jump.
The is the answer to the "
Crazy Code".
yah - i don't really think that's valid
it's difficult to time an API function - let alone the return method
and - that's a lot of work to code each call that way
better to dereference the calls - lol
but, whatever floats your boat
interesting and improve is this one:
http://masm32.com/board/index.php?topic=3483.msg36676#msg36676
accept any types and any size of datas
wrong is the assertion:
Quote
If it is data that can be fitted into fixed length areas of memory, the task is simple, you allocate an array of pointers, then a block of memory and aim each pointer to each section of memory.
I use it more often with lines of text (words) of various sizes (just add a zero at end before save them)
to empty the buffer is also easy:
invoke WriteToBlocHeap,addr BLOCHEAP,0,0 ;empty the block
a format can be also be used to save datas:
FORMAT STRUCT
datalen DWORD
thedata db datalen dup (?) ;pseudo code
FORMAT ENDS
this give:
mov edx,FORMAT.datalen
add edx,sizeof FORMAT.datalen (4)
invoke WriteToBlocHeap,addr BLOCHEAP,addr FORMAT,edx
...and....
there are roughly 5 times as many intel cpu's running windows as there are amd :P
There was something important that was missing. You read a file, ok, but how long is the buffer?
Now you pass the address of a DWORD into which the file size (which also is the buffer size - 256) is returned.
The pointer is initialized to zero at the function entry.
New version (7zip file) (http://phrio.biz/download/$File_Read.7z)
NO VIRUS FOUND INTO THIS FILE
Analyze link (https://www.virustotal.com/fr/file/226248ca9c8a113118f8640d4f559c595f247220fc80f5bbc9080cc39fb17846/analysis/1450374686/)
ReadFile PROTO hFile:HANDLE ,lpBuffer:XMASM ,nNumberOfBytesToRead:DWORD ,lpNumberOfBytesRead:XMASM ,lpOverlapped:XMASM
use the lpNumberOfBytesRead to get the answer and made a verify all is well done
substract this number to the size of the buffer and you have the rest of memory
INVOKE ReadFile,_FileHandle,eax,_dwFileSize,edx,NULL
test eax,eax
jz @Error_03
mov eax,_dwFileSize
sub eax,_dwTmp ; If the number of bytes read is not equal
jnz @Error_03 ; to the number of bytes to read => ERROR
mov eax,__lpdwFileSize
mov edx,_dwFileSize
mov DWord Ptr [eax],edx
INVOKE CloseHandle,_FileHandle
mov eax,_lpMemory
ret
That's what I do
Do you mean allocated buffer?
HeapSize()?
It's the size of the file.
The size of the allocated buffer is equal to the size of the file plus 256 bytes.
It's unclear.
Wich size do you search ??????????
The file size
Philippe,
The normal sequence is you open a file, get its length, allocate a buffer that is large enough then read the file into the buffer.
Just look up the foillowing in the "macros.asm" file in MASM32 and you will find all of the useful API functions for doing file IO. Look for this in the macros file,
; **************
; File IO Macros
; **************
I use this one since a certain time:
comments are in french
; le batch pour batir une librairie
;batch,sources et .inc (contenant les proto)
FichMem struc
Hmem dd ?
Mpoint dd ?
Taille dd ?
FichMem ends
; #########################################################################
.386 ; force 32 bit code
.model flat, stdcall ; memory model & calling convention
option casemap :none ; case sensitive
; --------------------------------
; Les fichiers inclus içi
; --------------------------------
include \masm32\include\windows.inc
include \masm32\include\kernel32.inc
include \masm32\include\user32.inc
include macros.txt
; ---------------------------------------------------------
; Les PROTO sont tous regroupés dans un fichier .inc du même
; nom que la librairie .lib
; ---------------------------------------------------------
.data
ercharge db "Taille ",0
texthou db " Handle fichier ",0
textHmem db "Handle memoire ",0
textPm db "Pointeur memoire ",0
textlu db " Nb octets en lecture ",0
InfosFiles WIN32_FIND_DATA <>
.code
;################################################################
;----------- pushad popad -----------------
LoadFile proc uses ebx NomAediter:DWORD,PileMem:DWORD
LOCAL retour:DWORD
LOCAL fichier:DWORD
local hMemory:DWORD
local pMemory:DWORD
local hFile:DWORD
local NbOctetsaLire:DWORD
local NbOctetsLus:DWORD
;pilemem adresse d'une structure FichMem
;rajoute un zero en fin de fichier
;nécessaire dans les copies avec clipboard par exemple
;utilise WIN32_FIND_DATA et NomComplet
;FichMem struc
; Hmem dd ?
; Mpoint dd ?
; Taille dd ?
;FichMem ends
mov ebx,PileMem
invoke RtlZeroMemory,addr InfosFiles,sizeof InfosFiles
invoke FindFirstFile,NomAediter,addr InfosFiles
;----------------------------------------------------------
; taille du fichier nFileSizeHigh; DWORD nFileSizeLow WIN32_FIND_DATA
; nom court ou nom long sans chemin ?
;-----------------------------------------------------------
.if eax==INVALID_HANDLE_VALUE
mov retour,0
jmp FindeLoadFile
.else
mov edx,eax
invoke FindClose,edx
mov retour,1
.endif
;------------------------------------------------------
; on connait maintenant toutes les infos sur le fichier
;-------------------------------------------------------
;----------------------------
; ouvrir le fichier
;----------------------------
invoke CreateFile,NomAediter,GENERIC_READ,NULL,\
NULL,OPEN_EXISTING,FILE_ATTRIBUTE_NORMAL,0
.if eax==INVALID_HANDLE_VALUE
;invoke MessageBox,NULL,SADD(" Erreur d'ouverture Fichier "),\
; SADD("LoadFile"),MB_OK
mov retour,0
jmp FindeLoadFile
.endif
mov hFile,eax
;-------------------------
;obtenir le handle de mémoire, taille du fichier dw
;-------------------------
mov ecx,InfosFiles.nFileSizeLow
mov dword ptr (FichMem ptr [ebx]).Taille,ecx
inc ecx ;pour rajouter le zero en fin de fichier
add ecx,32 ;mmx utilise des registres de 16 bytes
and ecx,0FFFFFFF0h ;rounded 16 bytes upper
invoke GlobalAlloc,GMEM_MOVEABLE or GMEM_ZEROINIT,ecx
.if eax == 0
;invoke MessageBox,NULL,SADD("Pas assez de place mémoire "),\
; SADD("GlobalAlloc"),MB_OK
mov retour,0
jmp FindeLoadFile
.endif
mov hMemory,eax
mov dword ptr (FichMem ptr [ebx]).Hmem,eax
;obtenir le pointeur de memoire
invoke GlobalLock,hMemory
mov pMemory,eax
mov dword ptr (FichMem ptr [ebx]).Mpoint,eax
mov eax,InfosFiles.nFileSizeLow
mov NbOctetsaLire,eax
invoke ReadFile,hFile,pMemory,NbOctetsaLire,addr NbOctetsLus,NULL
;-------------------------
;libéré la memoire ,fermé le fichier ouvert
;-------------------------
; invoke GlobalUnlock,pMemory
; invoke GlobalFree,hMemory
invoke CloseHandle,hFile
mov retour,1
FindeLoadFile:
mov eax,retour
ret
LoadFile endp
;#########################################################################
end
I think there is a mistake. Into one of my post I told that I had forgotten the file size. If the buffer needed to be written I did know its size. But I had the information because I call "GetFileSize" function.
I put the entire source code. You can download it here (http://www.phrio.biz/download/$File_Read.zip)
I appreciate any comment and speed tests. I don't know how to measure how fast/slow the functions are.
.686p ; create 32 bit code
.MMX ; enable MMX instructions
.XMM ; enable SSE instructions
.NOCREF
OPTION casemap :none ; case sensitive
; -------------------------------------------------------------------------------------------------------------
.Code
; -------------------------------------------------------------------------------------------------------------
; ==================================================================================
ALIGN 4
; ==================================================================================
; ReadWholeFile : INPUT PARAMETER : A pointer on the file name to be read.
; RETURNED VALUE : A pointer on the allocated memory block
; Or NULL if there was an error.
; The memory block must be free by the caller usif HeapFree
; The input file is opened for reading.
; During the read operation all access to the file will fail because there is no share.
; The file must not be empty and its size must be in the range 0...FFFFFFFFh
; Memory allocated size : File size + 256 bytes
; When the fuction returns the input file is closed even if there was an error
; Modified registers : ALL except EBX, EDI and ESI which are not used.
ReadWholeFile PROC __lpszFileName:LPSTR
LOCAL _lpMemory:LPBYTE ; The adress of the returned memory block
LOCAL _FileHandle:HANDLE ; The file handle
LOCAL _dwFileSize:DWORD ; Low part of the file size
LOCAL _dwTmp:DWORD ; Temporary variable
INVOKE PathFileExists,__lpszFileName
test eax,eax ; File does not exist
jz @Error_01
INVOKE CreateFile,__lpszFileName,GENERIC_READ,0,NULL,OPEN_EXISTING,FILE_ATTRIBUTE_NORMAL,NULL
cmp eax,INVALID_HANDLE_VALUE ; Can't open the file
je @Error_01
lea edx,_dwTmp
mov _FileHandle,eax
INVOKE GetFileSize,eax,edx
cmp eax,INVALID_FILE_SIZE
je @Error_02
cmp _dwTmp,0
jne @Error_02 ; File is too big
or eax,eax
jz @Error_02 ; File is empty
mov _dwFileSize,eax
add eax,256 ; File size + 256
push eax
push HEAP_ZERO_MEMORY ; Memory initialyzed with '0'
call GetProcessHeap
push eax
call HeapAlloc
test eax,eax ; Memory allocation failed?
je @Error_02
mov _lpMemory,eax
lea edx,_dwTmp
INVOKE ReadFile,_FileHandle,eax,_dwFileSize,edx,NULL
test eax,eax
jz @Error_03
mov eax,_dwFileSize
sub eax,_dwTmp ; If the number of bytes read is not equal
jnz @Error_03 ; to the number of bytes to read => ERROR
@EndOfFile :
INVOKE CloseHandle,_FileHandle
mov eax,_lpMemory
ret
; **********************************************************************************
ALIGN 4
; **********************************************************************************
@Error_03 :
call GetProcessHeap
push _lpMemory
push 0
push eax
push OFFSET @Error_02
jmp HeapFree
; **********************************************************************************
ALIGN 4
; **********************************************************************************
@Error_02 :
push _FileHandle
push OFFSET @Error_01
jmp CloseHandle
; **********************************************************************************
ALIGN 4
; **********************************************************************************
@Error_01 :
xor eax,eax
ret
ReadWholeFile ENDP
; ==================================================================================
ALIGN 4
; ==================================================================================
File_Read PROC __FileHandle:HANDLE,__lpBuffer:LPBYTE,__dwNumberOfBytesToRead:DWord,__lpdwFlag:PTR DWord
LOCAL _dwTmp:DWORD
lea edx,_dwTmp
INVOKE ReadFile,__FileHandle,__lpBuffer,__dwNumberOfBytesToRead,edx,NULL
test eax,eax
jz @Error_01
cmp _dwTmp,0
je @EndOfFile
mov eax,__dwNumberOfBytesToRead
sub eax,_dwTmp
jnz @Error_01
@EndOfFile : ; EOF detected
mov eax,__lpdwFlag
mov edx,_dwTmp
mov DWord Ptr [eax],edx ; NO ERROR - Returns the number of bytes read
mov eax,__lpBuffer
ret
; **********************************************************************************
ALIGN 4
; **********************************************************************************
@Error_01 :
mov eax,__lpdwFlag
mov DWord Ptr [eax],-1 ; ERROR WHILE READING
xor eax,eax
ret
File_Read ENDP