HiveBrain v1.2.0
Get Started
← Back to all entries
patternModerate

Countdown program in x86 NASM

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
countdownprogramx86nasm

Problem

I am fairly new to Assembly language programming and, for practice, I gave myself a problem: count down from 10 and right after 1, say "Blast off!".

section .text
    global _start
_start:
    mov eax, '10' ; asci 10
    mov [num], eax ; num now equals ascii 10

    mov edx, 2
    mov ecx, num
    mov ebx, 1
    mov eax, 4
    int 80h ; prints out '10' in the console

    mov eax, 9 ; numerical 9
    call subtract
subtract:
    call log ; logs the current number

    sub eax, 1 ; subract 1

    cmp eax, 0
    jne subtract ; if it doesn't equal 0, jump to subtract
    je blast_off ; if it does equal 0, jump to blast off
log:
    call to_ascii ; converts eax's value to ascii
    mov [num], eax ; num = eax
    mov edx, 1
    mov ecx, num
    mov ebx, 1
    mov eax, 4
    int 80h ; output the current number
    call to_number ; converts eax back to numerical
    ret
to_number:
    mov eax, [num] ; is this necessary? Moves num back into eax
    sub eax, '0' ; converts to numerical
    ret
to_ascii:
    add eax, '0' ; converts to ascii
    ret
blast_off:
    mov edx, len
    mov ecx, msg
    mov ebx, 1
    mov eax, 4
    int 80h ; outputs "Blast off!"

    mov eax, 1
    int 80h ; exits program
section .data
    msg db "Blast off!", 0xA ; message
    len equ $-msg ; length of message
section .bss
    num resb 1 ; this is for outputting the current number


Output:


1098654321Blast off!

Assembled: nasm -f elf blast_off.s

Linked: ld -m elf_i386 -s -o blast_off blast_off.o

My main concerns are:

  • My code seems functional - is this bad, or does it not really matter, being assembly language?



  • Is there repetition? This was a very difficult challenge for me, and at times I lost my head - sometime during those times I might have typed something completely "off topic" in regards to the subject of the code



  • Aside from the first concern, does the organization need re-organizing? As in, should I reorder the different sections(.test, .data, .bss)? Or, does it no

Solution

Section sequence

That's rather a matter of taste. Programmers used to 68k usually use the sequence .text, .data, .bss, programmers used to Java usually use the sequence .data, .bss, .text. In the assemblers known to me it doesn't make any difference, so it's rather a matter of taste or corporate policy.

Actually, strictly speaking it's not the assembler but the linker who might have a word to say about the sequence, and the linkers that I know don't mind the sequence (even the Keil Linker for 8051, L51, which is really weird stuff).

Functional code

Breaking up bigger chunks of code into smaller parts very often makes sense because very often it increases maintainability.

Basically in Assembler you do a lot of trade-off between maintainability, speed and size.

Your functions are not universally reusable, for example your function to_ascii performs no sanity checks.
In that case, it doesn't always make sense, especially when the function body is just on instruction.

However, as far as I'm aware, on a modern member of the 80x86 family, instructions like call and ret basically come free of cost, thanks to the pipeline and branch prediction, so maybe there is no loss of speed, just of space.

Redundancy

The sequence

mov ecx, num
mov ebx, 1
mov eax, 4
int 80h ; output the current number


is redundant.
I don't think it's entirely bad.
Compared to having a single-instruction function to_ascii it looks, however, inconsistent.

I'd extract this into something like a print_number function.

Exit status

(You fixed this meanwhile.)

Your exit code looks like this:

mov ebx, 1
mov eax, 4
int 80h ; outputs "Blast off!"

mov eax, 1
int 80h ; exits program


The contents of register ebx are still 1 when you call exit.
This is probably wrong.
I do not see any indication why you would want to indicate that your program exits unsuccessfully.
You probably want to insert a mov ebx, 0 like this:

mov ebx, 1
mov eax, 4
int 80h ; outputs "Blast off!"

mov ebx, 0
mov eax, 1
int 80h ; exits program


Asymmetry in to_number vs. to_ascii

to_number accesses [num] itself, whereas in the case of to_ascii, [num] is accessed by the caller.
This is asymmetric, a form of inconsistent.
In bigger programs this might lead to confusion with respect to how APIs work.

Name log

Nowadays when you say log people think of generic logging along the lines of Logging APIs like log4j or sclog4c.

I'd rather call this function something else, in order to prevent confusion.

Formatting

I'm not sure whether this is a convention, but I'd do this:

  • blank line before every new function (i.e. after a ret that is not followed by code which could be reached by a conditional jmp before the ret).



  • Use local labels for loop and branch labels inside a function.



  • Write a small function header on top of each function which describes input, output and access to globals (especially side-effects).



Function header

Here's a sample function header for to_ascii

; Returns the ASCII value of the input digit.
; @param eax digit (value in the range of [0..9]) which is converted into an ASCII digit.
; @return eax ASCII value (in the range of ['0'..'9']).
; @warning If eax is out of the supported range, the behavior is undefined.
to_ascii:
    add eax, '0'
    ret


Local labels vs Global labels

AFAIK this is also true for NASM:

  • Local labels start with '.' and are valid until the next global label.



  • Global labels do not start with '.'.



Looking at your code, subtract should be a local label and thus renamed to .subtract.

This is not mandatory, and there is a valid interpretation of your code which includes tail recursion and last call optimization in which your code and label structure are valid.

It's just a bit surprising (maybe even positively) to see such constructs in assembler.

Conditional jumps

At the end of subtract you have two conditional jumps:

jne subtract
je blast_off


While this code is correct, I think the following would be more clear:

je blast_off
jmp subtract


Or in case you want to optimize for speed

jne subtract
jmp blast_off


The point is maintainability.
I found it less error-prone to have the last conditional jmp, which semantically is identical to an unconditional jmp as unconditional jmp indeed.
When you change the conditions, there is less necessity of changing the last jmp, and therefore less risk of getting code which would just go haywire for running unintentionally into the next function, in case the current function doesn't end with ret itself.

Structured Programming regarding _start / subtract?

I personally would almost always follow structured programming even in Assembly language.

At _start / subtract, this is not done because you call subtract.
Maybe that call even is unintentional, as I cannot see where subtract would ever return.

Independently of that, a construct like

```
call

Code Snippets

mov ecx, num
mov ebx, 1
mov eax, 4
int 80h ; output the current number
mov ebx, 1
mov eax, 4
int 80h ; outputs "Blast off!"

mov eax, 1
int 80h ; exits program
mov ebx, 1
mov eax, 4
int 80h ; outputs "Blast off!"

mov ebx, 0
mov eax, 1
int 80h ; exits program
; Returns the ASCII value of the input digit.
; @param eax digit (value in the range of [0..9]) which is converted into an ASCII digit.
; @return eax ASCII value (in the range of ['0'..'9']).
; @warning If eax is out of the supported range, the behavior is undefined.
to_ascii:
    add eax, '0'
    ret
jne subtract
je blast_off

Context

StackExchange Code Review Q#75361, answer score: 13

Revisions (0)

No revisions yet.