NASM RPN Calculator

Question

I've been learning Assembly over the past few days, and I've made a simple RPN calculator.

Here is the main logic of the program, excluding utility functions such as printing:

%include "utilities.s"

SECTION .data
    badOpMsg      db "Unknown character: ", 0h
    badNumMsg     db "Error parsing number: ", 0h
    badExprMsg    db "Invalid expression supplied", 0h

SECTION .bss
    userInput resw 500

SECTION .text
global _start

%macro preOp 0
    cmp    ecx, 2
    jl     .invalidExpr
    ; pop two values off of the "stack" and place them into esi and eax
    dec    ecx
    mov    esi, [userInput + ecx * 4]
    dec    ecx
    mov    eax, [userInput + ecx * 4]
%endmacro

%macro postOp 0
    ; put eax back onto the "stack" and return to the end of the parse loop
    mov    [userInput + ecx * 4], eax
    inc    ecx
    jmp    .endloop
%endmacro

_start:
    mov    ebp, esp
    xor    ecx, ecx      ; parsed stack index
    mov    ebx, 2        ; argument list index

.parseLoop:
    mov    eax, [ebp + ebx * 4]

    ; Hack to allow numbers prefixed with '-'
    ; If current argument has a length of 1, treat it as an op/number
    ; otherwise treat it as a number
    cmp    byte [eax + 1], 0
    jnz    .parseNum


    cmp    byte [eax], 43          ; "+"
    je     .add
    cmp    byte [eax], 45          ; "-" 
    je     .subtract
    cmp    byte [eax], 47          ; "/"
    je     .divide          
    cmp    byte [eax], 120         ; "x"
    je     .multiply        
    cmp    byte [eax], 48          ; "0"
    jl     .fail
    cmp    byte [eax], 57          ; "9"
    jg     .fail

.parseNum:
    ; otherwise, add number to the stack
    ; use edi to indicate atoi error
    xor    edi, edi
    call   atoi
    cmp    edi, 1
    je     .numError

    mov    [userInput + ecx * 4], eax
    inc    ecx

.endloop:
    inc    ebx

    cmp    ebx, [ebp]
    jle    .parseLoop

    jmp    .success

.add:
    preOp
    add    eax, esi
    postOp

.subtract:
    preOp
    sub    eax, esi
    postOp

.multiply:
    preOp
    imul   eax, esi
    postOp

.divide:
    preOp
    div    si
    postOp

.numError:
    mov    eax, badNumMsg
    call   sprint
    mov    eax, [ebp + ebx * 4]
    call   sprintln
    jmp    .end

.fail: 
    ; An invalid character was supplied as input
    push   eax
    mov    eax, badOpMsg
    call   sprint
    ; Infringing character is on the top of the stack
    pop    esi
    mov    eax, [esi]
    call   putchar
    call   newline
    jmp    .end

.invalidExpr:
    mov    eax, badExprMsg
    call   sprintln
    jmp    .end

.success:
    ; If the stack has more than one item in it, 
    ; the supplied expression is invalid
    cmp    ecx, 1
    jne    .invalidExpr

    ; Pop remaining value off the stack and print it
    sub    ecx, 1
    mov    eax, [userInput + ecx * 4]
    call   iprintln

.end:
    call   quit

Here is the content of "utilities.s":

;----------------------------
; int strlen(String message)

strlen:
    push   ebx
    mov    ebx, eax

.nextchar:
    cmp    byte[eax], 0
    jz     .finished
    inc    eax
    jmp    .nextchar

.finished:
    sub    eax, ebx
    pop    ebx
    ret

;----------------------------
; void sprint(String message)

sprint:
    push   edx
    push   ecx
    push   ebx
    push   eax
    call   strlen

    mov    edx, eax ; move string len from eax to edx
    pop    eax      ; restore eax to string pointer

    mov    ecx, eax ; move string pointer to ecx
    mov    ebx, 1   ; 
    mov    eax, 4   ; opcode 4
    int    80h      ; make syscall

    pop    ebx
    pop    ecx
    pop    edx
    ret

;----------------------------
; void sprintln (String message)

sprintln:
    call   sprint    ; print string found at eax
    push   eax       ; preserve eax
    mov    eax, 0Ah
    call   putchar
    pop    eax
    ret

;----------------------------
; void putchar (char)

putchar:
    push   edx 
    push   ebx
    push   ecx
    push   eax     ; eax has character
    mov    edx, 1
    mov    ecx, esp
    mov    ebx, 1
    mov    eax, 4
    int    80h
    pop    eax
    pop    ecx
    pop    ebx
    pop    edx 
    ret

;------------------------------
; newline

newline:
    push   eax
    mov    eax, 0ah
    call   putchar
    pop    eax
    ret

;-----------------------------
; void iprint(int)

iprint:
    push   eax
    push   ecx
    push   edx
    push   esi
    push   edi

    xor    edi, edi
    xor    ecx, ecx
    test   eax, eax
    js     .negate

.divide:
    inc    ecx
    xor    edx, edx
    mov    esi, 10
    idiv   esi
    add    edx, 48
    push   edx
    cmp    eax, 0
    jnz    .divide

    cmp    edi, 1
    je     .printNegative

.print:
    dec    ecx
    mov    eax, esp
    call   sprint
    pop    eax
    cmp    ecx, 0
    jnz    .print
    jmp    .end

.negate:
    mov    edi, 1
    neg    eax
    jmp    .divide

.printNegative:
    push   45d
    inc    ecx
    jmp    .print

.end:
    pop    edi
    pop    esi
    pop    edx
    pop    ecx
    pop    eax
    ret

;-----------------------
; iprintln 

iprintln:
    call   iprint
    call   newline
    ret


;----------------------------
; atoi

atoi:
    push   ecx
    push   esi
    push   ebx

    mov    esi, eax  ; move string pointer to esi as eax will be used for math
    mov    eax, 0
    mov    ecx, 0

    xor    edi, edi  ; negative flag and error flag

    cmp    byte [esi], 45   ; "-"
    je     .negative

.parse:
    xor    ebx, ebx
    mov    bl, [esi + ecx]

    cmp    bl, 10
    je     .terminated
    cmp    bl, 0
    jz     .terminated
    cmp    bl, 48     ; "0"
    jl     .error
    cmp    bl, 57     ; "9"
    jg     .error

    sub    bl, 48
    add    eax, ebx
    mov    ebx, 10
    mul    ebx
    inc    ecx
    jmp    .parse

.negative:
    inc    ecx
    mov    edi, 1     ; make sign negative
    jmp    .parse


.negate:
    neg    eax
    mov    edi, 0     ; because edi is both negative and error, make sure to clear it
    jmp    .end

.error:
    mov    edi, 1
    jmp    .end

.terminated:
    mov    ebx, 10
    div    ebx
    cmp    edi, 1
    je     .negate

.end:
    pop    ebx
    pop    esi
    pop    ecx
    ret


;----------------------------
; void quit()

quit:
    mov    ebx, 0  ; exit code 0
    mov    eax, 1  ; opcode 
    int    80h
    ret

I'd mainly like to know if my code is idiomatic. For instance, are my register choices correct? Is using edi (a register I chose arbitrarily) to indicate a parsing error bad practice? Should I be using the .bss segment this way? How do people typically document their functions? Do they specify input and output registers?

Feel free to point out anyway my program could be improved, including how I could support larger numbers and floating point.

Edit: The code now supports negative numbers.

Answer 1

When I first saw the call atoi, I thought you would call the C function of the same name. That frightened me a bit, because that function should not be used in serious programs, since it doesn't provide proper error checking. So you should choose another name, especially when programming on Linux, where potential readers of your code are familiar with that function.

I like the trick with the self-terminating string in iprint. Also, the idea of pushing the characters in reverse order and then printing them is nice and simple. If you had to fight for every system call, allocating a single 12-byte buffer would be more efficient, both in the memory usage and in the number of system calls.

Do you indent to support negative numbers?

Instead of using null-terminated strings like in C, you could represent a string as (start, length) tuple, which would make the strlen function unnecessary.