-1

little bit of a problem here, tried to create assembly code which converts strings to integers. Unfortunately, I cannot find the reason why it "breaks", I input 54321 and it converts to 543418. All goes OK till 2 and then it prints random numbers instead of printing the same 54321. I already spent an hour debugging, but cannot find the cause of this problem, maybe I'm blind?

#include <stdio.h>
#include <string.h>
#include "stdlib.h"

int main(int argc, char** argv)
{
    int result;
    char* argv1 = argv[1];

    if (argc < 2)
    {
        printf("Parameter is not provided*/\n");
        return(0);
    }

    __asm
    {
        push eax
        push ebx
        push ecx

        mov ecx, argv1 // 54321 --> 5 *10+4=54 *10+3=543 *10+2=5432 *10+1=54321

        mov ebx, 10 // register in which i put value
        mov al, byte ptr[ecx] // byte of string to al
        sub al, '0'

        loop_begins:
        mov dl, byte ptr [ecx] // byte of string to dl
        cmp dl, 0 // compare to zero (string end)
        je loop_ends // if zero byte (string end)

        // here we make char out of int
        xor edx, edx // zero edx
        mul ebx // edx: eax = eax * ebx
        inc ecx // ecx points to next char in string
        mov dl, byte ptr [ecx] // dl - edx part, other register
        sub dl, '0' // same, but other element
        add eax, edx // addition (here we get 54)
        xor edx, edx // zero edx
        jmp loop_begins // and loop to next char

        loop_ends:
        mov [result], eax // answer to variable result

            pop ecx
            pop ebx
            pop eax
    };
    printf("Answer is %d\n", result);

    system("PAUSE");
}
R3Natas
  • 17
  • 3
  • 2
    One obvious problem is that you check the current character in `loop_begins` but then process the **next** character in the body (meaning you do `inc ecx` too late). – Jester Dec 17 '18 at 20:51
  • Thank you! Yes, indeed, I put it in the wrong place – R3Natas Dec 17 '18 at 20:57
  • IDK why you're loading the character twice. You're writing 32-bit code, so you can use `imul eax, eax,10` if you want to use multiply at all (instead of a couple LEA instructions), and avoid clobbering EDX or needing a constant in another register. (Also, `EDX` is not an input to one-operand `mul`. It does EDX:EAX = EAX*src. Maybe you're thinking of `div`. [When and why do we sign extend and use cdq with mul/div?](https://stackoverflow.com/q/36464879)) – Peter Cordes Dec 17 '18 at 22:30

1 Answers1

0

I made a few modifications (<mod> in comments) to your code keeping the spirit (also according to my weak and unused for a long time asm knowledge ):

__asm
    {
        push eax  // current value / result
        push ebx  // 10
        push ecx  // string

        xor eax, eax // <mod> : reset eax
        mov ebx, 10 // register in which i put value
        mov ecx, argv1 // 54321 --> 5 *10+4=54 *10+3=543 *10+2=5432 *10+1=54321

        // <mod> : unnecessary
        // mov al, byte ptr[ecx] // byte of string to al
        // sub al, '0'

        loop_begins:
            mov dl, byte ptr [ecx] // byte of string to dl
            cmp dl, 0 // compare to zero (string end)
            je loop_ends // if zero byte (string end)

            // here we make char out of int
            // xor edx, edx // zero edx // <mod> : unnecessary
            mul ebx // edx: eax = eax * ebx

            mov dl, byte ptr [ecx] // dl - edx part, other register
            sub dl, '0' // same, but other element
            add eax, edx // addition (here we get 54)
            // xor edx, edx // zero edx // <mod> : unnecessary

            inc ecx // ecx points to next char in string // <mod> moved as suggested in comments
            jmp loop_begins // and loop to next char

        loop_ends:
            mov [result], eax // answer to variable result

        pop ecx
        pop ebx
        pop eax
    };
Liviu
  • 1,859
  • 2
  • 22
  • 48