Once again, values that get mixed up. Cant figure out why/where

Started by Ani_Skywalker, November 02, 2011, 12:16:35 AM

Previous topic - Next topic

Ani_Skywalker

Bellow code is a solution to one of these programming competitions. I read a 1000 byte large file, then multiply five consecutive numbers and save the product. Then another five consecutive numbers are multiplied and if the product is larger then the previous product, that product is stored. I store the products in a variable called "sum", pointing this out so it wont be any confusion. When the hole file is iterated, I print out the largest number, which is stored in sum.

Now, here is the problem: Depending on from where in the program I print out "sum" in the end, I get different values. If I print it directly after the multiplications, I get the correct value. If I move the printing past a compariso.n, "sum" is zeroed and if I print it out after a jump that wont be executed at the last instance, I'll get an inappropriate value. I commented out in the code so it should be easy to find. Thank you in advance!

main proc
local hFile :HANDLE
local bytesRead :DWORD
local buffer[1001] :BYTE
local sum :DWORD

local first :DWORD
local second :DWORD
local third :DWORD
local fourth :DWORD
local fifth :DWORD

mov hFile,fopen("008.dat")
push 0
lea eax,bytesRead
push eax
push SIZEOF buffer
lea eax,buffer
push eax
push hFile
call ReadFile

push edi
push esi

xor edi, edi
xor esi, esi
mov sum, 0

@@loopStart:
cmp sum, esi
jae @@next
mov sum, esi

@@next:
inc edi
mov al, buffer[edi]
mov first, eax
mov al, buffer[edi+1]
mov second, eax
mov al, buffer[edi+2]
mov third, eax
mov al, buffer[edi+3]
mov fourth, eax
mov al, buffer[edi+4]
mov fifth, eax

sub first, 48
sub second, 48
sub third, 48
sub fourth, 48
sub fifth, 48

mov eax, 1

mul first
mul second
mul third
mul fourth
mul fifth

mov esi, eax

print ustr$(sum),13,10 ; If this is commented out, the variable sum gets an inappropriate value.
; But if is there, both this print and the print "HEY AGAIN" have an appropriate value

cmp edi, 994
;print ustr$(sum)," hey",13,10,13,10 ; if this is not commented out, the variable sum is 0 in all 3 cases in the end.
jle @@loopStart

print ustr$(sum)," hey again",13,10,13,10 ; this is the print "HEY AGAIN"

pop esi
pop edi

ret
main endp

clive

mov al, buffer[edi]
mov first, eax


Surely you need to make sure the high order bits are clear?

movzx eax,byte ptr buffer[edi]
mov first,eax


Without that, the math is going to get messed up by assorted values left in EAX by the printing code, prior iterations, etc.
It could be a random act of randomness. Those happen a lot as well.

Ani_Skywalker

Quote from: clive on November 02, 2011, 12:52:50 AM
movzx eax,byte ptr buffer[edi]
mov first,eax


Without that, the math is going to get messed up by assorted values left in EAX by the printing code, prior iterations, etc.

That solved it. Not sure why, even though I understand both the instruction and the necessity of it. But according to the previous outputs, the right value was stored to sum, otherwise it wouldn't work to print it out which was possible if the print was in this section:

mul fourth
mul fifth

mov esi, eax

print ustr$(sum),13,10

cmp edi, 994
jle @@loopStart

Ani_Skywalker

I could also follow the value, since each storing to sum was recorded (the output changed). I hate this part of assembly, I'm never gonna understand the math of it :'(

dedndave

i post this, just to give you some ideas   :P
@@loopStart:
                cmp sum, esi
                jae @@next

                mov sum, esi

@@next:
                inc edi

                mov al, byte ptr buffer[edi]
                and eax,0Fh
                mov ecx,4

MulLoop:
                inc edi
                mov dl,[edi]
                and dl,0Fh
                mul dl
                dec ecx
                jnz MulLoop

                sub edi,4

                mov esi, eax

                print ustr$(sum),13,10      ; If this is commented out, the variable sum gets an inappropriate value.
                                            ; But if is there, both this print and the print "HEY AGAIN" have an appropriate value

                cmp edi, 994
                print ustr$(sum)," hey",13,10,13,10
                jle @@loopStart


there are probably many ways to go about this
for example, you could take the previous product, divide out the first number, then multiply by the next
divide is slow, however - there are ways around that, too

dedndave

i noticed that bitrake was the second forum poster on this problem   :P

Ani_Skywalker

Quote from: dedndave on November 02, 2011, 10:28:18 AM
i post this, just to give you some ideas   :P


@@next:
                inc edi

                mov al, byte ptr buffer[edi]
                and eax,0Fh
                mov ecx,4

MulLoop:
                inc edi
                mov dl,[edi]
                and dl,0Fh
                mul dl
                dec ecx
                jnz MulLoop
                sub edi,4

                mov esi, eax
                print ustr$(sum),13,10

                cmp edi, 994
                print ustr$(sum)," hey",13,10,13,10
                jle @@loopStart


there are probably many ways to go about this
for example, you could take the previous product, divide out the first number, then multiply by the next
divide is slow, however - there are ways around that, too

I understand the method you describe but not how you execute it. However, I have another question, I could store the values on the stack instead of using local memory, which should end up on the heap, right? Which is less time consuming, storing values in heap or stack?

dedndave

there are different types of stacks
the one we know and love is called a FILO (first in, last out) stack or LIFO if you prefer (last in, first out)
in this case, a FILO stack wouldn't be very helpful

a FIFO (first in, first out) stack might be more useful
a similar mechanism called a "circular buffer" can be made that could also be used
this is a buffer where we keep track of the beginning and end pointers
the highest address wraps around to the lowest address in physical memory
rather than moving each data element in the buffer when items are added or removed, we simply change the pointer
the BIOS keyboard buffer is an example

the heap is just more memory that you haven't declared globally   :P

at any rate, to solve the problem, we do not need to retain the values
keeping them like that is inefficient, itself

my code above has a flaw - lol
i wound up using something similar, though...
EBX = largest product (initially 0)
EBP = 0Fh
EDI = loop count
COUNT_DIGITS = 4

loop00: mov     al,[esi]
        mov     ecx,COUNT_DIGITS                ;ECX = inner loop count
        and     eax,ebp

loop01: inc     esi
        mov     dl,[esi]
        and     edx,ebp
        mul     edx
        dec     ecx
        jnz     loop01

        cmp     eax,ebx
        jbe     loop02

        xchg    eax,ebx

loop02: sub     esi,COUNT_DIGITS-1
        dec     edi
        jnz     loop00