Message Box Question

[RKennedy9064]

I have been messing around with assembly the past few days, and got a program up and running. My program will real a semi colon deliminated file, place it into an array that is allocated, make an index for each string, and then print them out one by one. I first made this program as a console program, and it printed everything fine. I seem to be having an issue now that I have moved it to a windowed program. When I modified the code to create a message box for each name instead of an StdOut, nothing seems to happen. If I put a hardcoded string in it works, but using my array it doesn't. Is there anything special I need to know about message boxes? I'm fairly new when it comes to assembly, so I assumed you could print it the same way with an StdOut or a MessageBox. This is the code I have, I commented out the original working console print. Any help would be greatly appreciated.

Code Select Expand

PrintArray proc array:DWORD
        mov esi, array
        mov ecx, 0
       
        CheckBytes:
            mov edi, [esi + ecx * 4]
            cmp edi, 0
            jz Done
            push esi
            push edi
            push ecx
            invoke MessageBox, NULL, [esi + ecx * 4], addr szSuccess, MB_OK
            ;invoke StdOut, [esi + ecx * 4]
            ;invoke StdOut, OFFSET szNewLine
            pop ecx
            pop edi
            pop esi
            inc ecx
            jmp CheckBytes
           
            Done:
                ret
               
    PrintArray endp


[jj2007]

It looks innocuous, no reason why it should fail. Can you post the complete code?

P.S. pushing esi+edi is not necessary. Windows APIs trash only eax ecx edx - see my Tips below for more.

[raymond]

It should not fail as long as (always assuming the code is according to MASM syntax):
1 - The variable array contains the address of the allocated memory which contains the addresses of each string stored in another section of allocated memory.
2 - Each string is null terminated in memory (either by replacing the original deliminating semi colon by a 0 byte, or adding that 0 byte after the semi colon).

Ass jj mentioned, the complete code could shed light on what may be at fault.

[RKennedy9064]

Here is the full source code. I went back to my original console program and tried to make a messagebox and it worked, so I retyped it out for my windowed program and it seems to be working now. I'm not really sure why it didn't before, I believe I typed it all in the exact same way. Maybe I had a typo somewhere before. I appreciate the tip about which registers are trashed by the Windows API, I'll have to look at the other tips on your link. If you don't mind, does it look like I'm doing things correctly. I'm new to assembly and not sure if I'm using proper techniques.

Code Select Expand

include \masm32\include\masm32rt.inc
   
.DATA
    szCharacterFile db "Character List.txt", 0      ; Name of file that stores character list
    szErrOpen       db "Error Opening File", 0      ; String used for debugging
    szErrRead       db "Error Reading File", 0      ; String used for debugging
    szSuccess       db "Success", 0                 ; String used for debugging
    szNewLine       db 13, 10                       ; Newline feed
    szClassName     db "lolcounter", 0
    szAppName       db "lolcounter", 0
   
.DATA?
    hMem dd ?                                       ; Pointer to the character list in memory
    hArray dd ?                                     ; Pointer to the index array of characters
    hArraySize dd ?                                 ; Number of indexes in the array
    hInstance HINSTANCE ?
   
ReadCharacterFile Proto                             ; Reads the character list into memory (hMem)
ProcessString Proto :DWORD                          ; Prototype : exchanges ; with 0 to null terminate each character
CreateArray Proto :DWORD, :DWORD                    ; Creates the index array so each character can be accessed
PrintArray Proto :DWORD                             ; Prints out every character stored in hMem

.CODE

    WinMain proc hInst:HINSTANCE,hPrevInst:HINSTANCE,CmdLine:LPSTR,CmdShow:DWORD
        LOCAL wc:WNDCLASSEX                                            ; create local variables on stack
        LOCAL msg:MSG
        LOCAL hwnd:HWND
   
        mov   wc.cbSize,SIZEOF WNDCLASSEX                   ; fill values in members of wc
        mov   wc.style, CS_HREDRAW or CS_VREDRAW
        mov   wc.lpfnWndProc, OFFSET WndProc
        mov   wc.cbClsExtra,NULL
        mov   wc.cbWndExtra,NULL
        push  hInstance
        pop   wc.hInstance
        mov   wc.hbrBackground,COLOR_WINDOW+1
        mov   wc.lpszMenuName,NULL
        mov   wc.lpszClassName,OFFSET szClassName
        invoke LoadIcon,NULL,IDI_APPLICATION
        mov   wc.hIcon,eax
        mov   wc.hIconSm,eax
        invoke LoadCursor,NULL,IDC_ARROW
        mov   wc.hCursor,eax
        invoke RegisterClassEx, addr wc                       ; register our window class
        invoke CreateWindowEx,NULL,\
                    ADDR szClassName,\
                    ADDR szAppName,\
                    WS_OVERLAPPEDWINDOW,\
                    CW_USEDEFAULT,\
                    CW_USEDEFAULT,\
                    CW_USEDEFAULT,\
                    CW_USEDEFAULT,\
                    NULL,\
                    NULL,\
                    hInst,\
                    NULL
        mov   hwnd,eax
        invoke ShowWindow, hwnd,CmdShow               ; display our window on desktop
        invoke UpdateWindow, hwnd                                 ; refresh the client area
   
        .WHILE TRUE                                                         ; Enter message loop
                    invoke GetMessage, ADDR msg,NULL,0,0
                    .BREAK .IF (!eax)
                    invoke TranslateMessage, ADDR msg
                    invoke DispatchMessage, ADDR msg
       .ENDW
        mov     eax,msg.wParam                                            ; return exit code in eax
        ret
    WinMain endp
       
    WndProc proc hWnd:HWND, uMsg:UINT, wParam:WPARAM, lParam:LPARAM
        .IF uMsg==WM_DESTROY                           ; if the user closes our window
            invoke GlobalFree, hMem
            invoke GlobalFree, hArray
            invoke PostQuitMessage,NULL             ; quit our application
        .ELSEIF uMsg == WM_CREATE
            invoke ReadCharacterFile
            invoke ProcessString, hMem
            invoke CreateArray, hMem, hArraySize
            invoke PrintArray, ADDR hArray
        .ELSE
            invoke DefWindowProc,hWnd,uMsg,wParam,lParam     ; Default message processing
            ret
        .ENDIF
        xor eax,eax
        ret
    WndProc endp

    ReadCharacterFile proc                          ; Start of ReadCharacterFile function

        LOCAL hFile:HANDLE                          ; Handle to the file being opened
        LOCAL fileSize:DWORD                        ; Stores the size of the file
        LOCAL bytesRead:DWORD                       ; Stores how many bytes have been read
       
        invoke CreateFile, ADDR szCharacterFile,    ; Creates a file handle pointing to the character file
                           GENERIC_READ, NULL,
                           NULL, OPEN_EXISTING,
                           FILE_ATTRIBUTE_NORMAL,
                           NULL
        or eax, eax                                 ; Sets eax to itself
        jz OpenFail                                 ; If eax == 0, jump to OpenFail
        mov hFile, eax                              ; Store the file pointer in hFile

        invoke GetFileSize, eax, 0                  ; Get the size of the file
        mov fileSize, eax                           ; Store the file size in fileSize
        inc eax                                     ; Add 1 to eax (extra byte for null terminator)

        invoke GlobalAlloc, GMEM_FIXED, eax         ; Allocate enough memory to store the file
        mov hMem, eax                               ; Put the pointer to that memory in hMem
        add eax, fileSize                           ; Make eax point to the end of the buffer
        mov BYTE PTR [eax], 0                       ; Set last byte to 0 (Null terminator)
       
        invoke ReadFile, hFile, hMem,               ; Reads the file into the hMem buffer
               fileSize, ADDR bytesRead, NULL   
        or eax, eax                                 ; Set eax to itself
        jz ReadFail                                 ; If eax == 0, jump to ReadFail
       
        ReadFail:
            invoke CloseHandle, hFile               ; Closes the file handle

        OpenFail:
            ret                                     ; Return from function
               
    ReadCharacterFile endp                          ; End of ReadCharacterFile function

    ProcessString proc mem:DWORD
        mov esi, mem
        xor ecx, ecx
        CheckByte:
            mov al, [esi]
            inc esi
            cmp BYTE PTR al, ';'
            jz ReplaceByte
            cmp BYTE PTR al, 0
            jnz CheckByte
            inc ecx
            ret

            ReplaceByte:
                mov BYTE PTR [esi - 1], 0
                inc ecx
                mov [hArraySize], ecx
                jmp CheckByte
               
    ProcessString endp

    CreateArray proc string:DWORD, dwSize:DWORD
        mov eax, dwSize
        inc eax
        invoke GlobalAlloc, GMEM_FIXED, eax
        mov hArray, eax
        add eax, dwSize
        mov DWORD PTR [eax], 0
        mov esi, string
        mov ecx, dwSize
        mov edx, 4
        mov [hArray], esi
       
        CheckBytes:
            cmp DWORD PTR ecx, 0
            jz Done
            mov BYTE PTR al, [esi]
            inc esi
            cmp BYTE PTR al, 0
            jnz CheckBytes
            mov [hArray + edx], esi
            add edx, 4
            dec ecx
            jmp CheckBytes
           
            Done:
                ret
               
    CreateArray endp

    PrintArray proc array:DWORD
        mov esi, array
        mov ecx, 0
       
        CheckBytes:
            mov edi, [esi + ecx * 4]
            cmp edi, 0
            jz Done
            push esi
            push edi
            push ecx
            invoke MessageBox, NULL, [esi + ecx * 4], offset szSuccess, MB_OK
            ;invoke StdOut, [esi + ecx * 4]
            ;invoke StdOut, OFFSET szNewLine
            pop ecx
            pop edi
            pop esi
            inc ecx
            jmp CheckBytes
           
            Done:
                ret
               
    PrintArray endp
   
    start:
        invoke GetModuleHandle, NULL
        mov hInstance,eax
        invoke WinMain, hInstance, NULL, NULL, SW_SHOWDEFAULT
        invoke ExitProcess, eax                           
    end start
       

[dedndave]

Code Select Expand

            invoke PrintArray, ADDR hArray
you pass the address of a single dword variable

Code Select Expand

    PrintArray proc array:DWORD
        mov esi, array
you attempt to access several dwords from that address   :P

that may not be the only problem

[donkey]

Hi Dave,

PrintArray looks like it is addressing OK on a first check, he uses Base+Index*Scale (esi+ecx*4) for addressing so it should walk the array properly unless I'm missing something. I would think that the problem is that he is attempting to scan string a dword at a time checking for 0x00000000 not 0x00. When the string is hard coded it is probably filled to an aligned area with 0 so it would detect the end of string properly however, when it is in an array only a single 0 is present and the function fails. He should be trying to scan a single character at a time by removing the Scale (*4) and masking out the high order 3 bytes of EDI (and edi,0ffh). Anyway that's my take on a first look.

[raymond]

The main error I can see is in the following part of the code:

Code Select Expand

    CreateArray proc string:DWORD, dwSize:DWORD
        mov eax, dwSize
        inc eax
        invoke GlobalAlloc, GMEM_FIXED, eax
        mov hArray, eax
        add eax, dwSize
        mov DWORD PTR [eax], 0


The dwSize seems to be the count of strings. The hArray will be the allocated memory to store the starting address of the strings. The size of that array must thus be at least 4* the count. The "inc eax" is for allocating an extra address space where a 0000 address will indicate the end of the addresses. That  "inc eax" must therefore be followed by a "shl eax,2" to allocate sufficient space.

Then, the terminating 0000 address should be inserted correctly as follows:

Code Select Expand

   mov eax,dwSize
   shl eax,2
   add eax,hArray
   mov DWORD PTR [eax], 0

Even if the allocated memory had been sufficient, the program would fail if the number of strings is not a multiple of 4. That ending 0000 address would then never be found.

[RKennedy9064]

Does it need to be a multiple of 4 since a DWORD is 4 bytes, but I'm only allocating 1/4th of that with my count? As for the 0 never being found unless it was a multiple of 4, wouldn't it always be found? I thought that if I stored the starting position of the DWORDS, and waled through it with esi + ecx * 4, it would jump DWORD to DWORD checking each memory location since it starts at the first byte and every 4 additional bytes is the start of the next DWORD. Any idea why I got the names to print out even though I didn't allocate enough space when I ran my console program? When I called PrintArray using the console, it printed every name in the list. Shouldn't that not work if I didn't allocate enough space?

[Gunner]

Shouldn' t this:
Esi+ ecx*4

Be this?
Esi+ 4*ecx

[jj2007]

it printed every name in the list. Shouldn't that not work if I didn't allocate enough space?

A string in the .data? section will typically choke if the section is full, i.e. at 4096 bytes total. If the string is the last entry, you will hardly notice. A good recipe for getting bugs that are difficult to chase :toothy

@Gunner: Esi+ ecx*4 = Esi+ 4*ecx

[qWord]

RKennedy9064,
If this program has ever worked with the console, I would say this was simply 'luck'. See Attachment.

qWord

[raymond]

Any idea why I got the names to print out even though I didn't allocate enough space when I ran my console program?

If you look at the description of the GlobalAlloc function, you would find this at the end:

If this function succeeds, it allocates at least the amount of memory requested. If the actual amount allocated is greater than the amount requested, the process can use the entire amount. To determine the actual number of bytes allocated, use the GlobalSize function.

Depending on your OS, if you were to request 16 bytes for example, you may actually be allocated 256 bytes or even more. The memory is allocated in manageable blocks by the OS. If the number of strings in your file is relatively small, you could have had enough space to store ALL of their addresses even though your requested space would initially be too small.

And, my bad, you would NEVER have found the terminating 0000 address simply because you would have overwritten it while parsing the file and storing the starting addresses of the strings. Again assuming 16 strings, you would have inserted the 0000 at offset 16 in the array; and, when processing the 5th string, you would have inserted its address over the 0000.

[RKennedy9064]

That makes sense, so it was just luck that it actually worked. I was looking at your code qWord and it seemed to have a lot of changes. It seems there are a few things I should learn before continuing on this. Would you guys have any recommendations of where I could do some research? I see that you used labels called @@: and a jump back to @b. I'm not familliar with that or the uses declaration at the top of the functions.

[raymond]

The use of the @@: label is to avoid the creation of distinct labels for "short" jumps in your code. Then jxx @B (or @b) means to jump at the previous occurence of the @@: while the jxx @F (or @f) means to jump at the very next occurence or the @@: You only have to be careful not to use those @@: in between where you actually want the jump to occur.

As for the purpose of USES, if you are using the QEditor in the MASM32 package, in the Help menu => MASM32 Help => Full Listing => USES, it is described as follows:

An optional keyword used with PROC. Generates code to push the
     value of registers that should be preserved (and that will be
     altered by your procedure) on the stack and pop them off when the
     procedure returns.

     The <reglist> parameter is a list of one or more registers. Separate
     multiple registers with spaces.

A few more hints to save you coding time (and very possibly code size) and prevent your program to crash unexpectedly.

Read file

You included code to check if opening and reading the file were successful but, on return from the procedure, you simply continue processing the file without verifying the success or failure.

Process file

Here's a modified version to count the number of strings and replace the ";" delimiter with a 0.

Code Select Expand

;move bytesRead dd ? to your global .DATA? section instead of a LOCAL in the Readfile proc

   mov edi,mem
   mov ecx,bytesRead
   mov eax,";"
@@:
   repnz scasb
   .if ZERO?
      mov [edi-1],ah
      inc hArraySize
      jmp @B
   .endif
   inc hArraySize
   ret

BTW, the count of strings with your code would have been short by 1; when reaching the end of the file, you were incrementing ecx by 1 but never got it into the memory variable.

CreateArray

invoke GlobalAlloc, GPTR, eax
would rezero all the allocated memory; thus, no need to add code for inserting the 0000 to terminate the array.