First Masm32 program, can't figure out unhandled exception

[henry.chris]

Coding this in vis studio 12 using MASM in 32 bit.  Just included one procedure in a program that will convert an input string to a table that shows hex, decimal and ascii conversions of the input.  This procedure is supposed to convert the input string to a hex number. 
The exception occurs during either the flag/register pops or the return.  All my registers are set to 0, except ebx gets set to a random value when returning.
Also, the unhandled exception seems to be at EIP.

Code Select Expand


inputStr  db 16 DUP(0)
.code
main proc

   xor eax, eax
   xor ebx, ebx
   xor ecx, ecx
   xor edx, edx

   lea esi, outputStr1
   call PrintString
   lea esi, inputStr
   call GetString
   call StrtoNum

   
   invoke ExitProcess, 0     ;**This is next line when it crashes**
main endp

StrtoNum proc  ;going to hex first
pushad
pushfd
mov bl, 1        ;mov 1 into bl for later multiplying
whilemorechar:
mov al,byte ptr [esi]
cmp al, 0       ;stuff
je ConvertDone ;if null then we are done here
   ;esi is pointing to the string to be converted
;cmp bl,0
;jnz decrement
cmp al, 0h
je ConvertDec
sub al, 30h     ;get first string byte to dec number 0-9
push ax         ;push last digit to stack
inc esi         ;gets to next string byte
inc cl          ;make note of decimal position in string
jmp whilemorechar      ;jump for next place in string
ConvertDec: ;reverse is done, now turn into decimal number
cmp cl, 0           ;compare counter to 0
jz ConvertDone ;if counter is 0, start comparing numbers
pop ax                  ;pop last on stack
mul bl ;multiply place value by input byte
add dx, ax ;add decimal value into dl, sum of loop is input in decimal
mov al, 10d ;move 10 into al
mul bx ;multiply 10 and bl value to get next decimal place
mov bx, ax ;mov decimal place into bl for next loop through
dec cl ;decrement counter
jmp ConvertDec ;loop through again for next decimal place
ConvertDone:
mov ebx, 0
popfd ;pop flags
popad ;pop registers
ret ;return to caller
StrtoNum endp

[dedndave]

there are a few problems, here

first, you should not push or pop word registers in 32-bit code
strictly speaking, it may work ok
but, it is a bad habit, as the stack should always be 4-aligned

so - instead of PUSH AX and POP AX, use PUSH EAX and POP EAX

second, take a close look at your loop
inside the loop, you push, push, push.....
one push per loop pass
then, you pop one time
your pushes should balance with the pops in this case
when it gets to the end of the routine, the stack is not balanced,
so POPFD, POPAD, and RET are all getting incorrect values off the stack
there is another way to balance the stack, but that's a little bit of an advanced technique, for now

another one, it isn't usually necessary to preserve all registers and the flags across a call
i think you are using Randy Hyde's include files, and he does preserve most registers, most of the time
but - no need to preserve the flags, for sure

finally, if you preserve all registers across the call, what does MOV EBX,0 do ?   :P
you might POPAD, then zero EBX

[henry.chris]

Sorry, ya the pop was getting bad values...ebx to be exact.  I was messing around with what to do and out of desperation set ebx to 0 before the pop to see what would happen. 
I will start pushing and popping the dword registers only.  Instructor did not go into detail about what and what not to pop, jsut said you cannot go under a word.

The loop definitely pushes ax three times and pops it 3 times.  Since the program will only deal with original ascii, the number of characters in the decimal will never be above 127, so I only need to loop through 3 times max. This may not be the most efficient way, but is the way I thought of.  When I debug my program, this definitely loops through and pushes, pops the correct number of times. 
If I input '45' into my inputStr I will have 0x00003534 stored in it.  It then sets ecx to 0 and loops 3 times to get each byte to 0h-9h and pushes it onto the stack. I do this so that I can pull it off in reverse order and output it as '45' instead of '54'. 

The correct value will be put into a variable, but I cut out all unneeded code to try to debug this.

I'll try the changes you outlined so far.   Thank you so much for responding, I'll be right here if you look at it again.

Edit:  It's definitely the EIP.  The exception says it cannot read whatever address EIP is at.

[dedndave]

Code Select Expand

whilemorechar:
mov al,byte ptr [esi]
cmp al, 0       ;stuff
je ConvertDone ;if null then we are done here
   ;esi is pointing to the string to be converted
;cmp bl,0
;jnz decrement
cmp al, 0h
je ConvertDec
sub al, 30h     ;get first string byte to dec number 0-9
push ax         ;push last digit to stack
inc esi         ;gets to next string byte
inc cl          ;make note of decimal position in string
jmp whilemorechar      ;jump for next place in string

from what i can see, this portion of code loops until you get to a null char
then, it branches to ConvertDone

Code Select Expand

mov al,byte ptr [esi]
cmp al, 0       ;stuff
je ConvertDone ;if null then we are done here
   ;esi is pointing to the string to be converted
;cmp bl,0
;jnz decrement
cmp al, 0h
je ConvertDec

notice that you test AL for 0 - if it is, you go to ConvertDone
then, you test AL for 0 again - this second test will never branch to ConvertDec because AL will never be 0   

the code at ConvertDone does not balance the stack

[henry.chris]

Ok, you are definitely right. I must have changed something.

Fixing that portion now, will repost this procedure when Its looping properly again if I'm still getting the same exception.

I figured i was doing something stupid, I dunno how I didn't see that.

[henry.chris]

Fixed!   Thank you so much.  It was definitely the loop messing it up like you said...then all I had to do was change the ax, bx, dx values to the al, bl, dl values. I forgot that multiplying in the word values uses the dx register and just kept setting it to 0 each loop through.  After doing that it loops and pops fine. I'll ask my professor why he wants us to push and pop the flags each proc...maybe just as a precation for us newbies.

Anyways, thanks for taking time out of your day/night...best feeling now,  that was the last part of this program to finish.

[Gunther]

Hi henry.chris,

first things first: welcome to the forum. I think you've the answer to your question. Have fun and enjoy.  :t

Gunther

[dedndave]

now that you have it working....

there are better ways to do what you want
i generally start at the end of the buffer and process numbers in reverse
then, the routine returns an address to the first non-zero char in the buffer
there are other methods, as well

[henry.chris]

Gunther:  Yes, I do.  :p   

dedndave:  Yes, that would be more ideal.  I wasn't sure the best way to go about
that, and seeing as my project time limit was approaching I was mainly just trying to
"get it done"...albeit not in the best way.
I find myself, in assembly, not trying to figure outhow to do it theoretically, but only
computationally if that makes sense.  It's different than thinking
in OO methods even though you kind of structure procedures like functions etc...  So, most of my
time has been just sitting there, thinking, about how to condense my code or take off iterations as well
as messing up with syntax obviously.
Anyways, I will continue to look at this first program and see what I can do better.  I'm sure there is
a lot.  Thank you again for your time, I'm sure I might have another question sooner than later.

[dedndave]

your prof probably doesn't care, as long as it works
the fact that it isn't ideal is a good indicator that you did the work yourself
nice job   :t