5
\$\begingroup\$

I am a beginner in assembly and would like to know ways I could improve my code. I am talking more about assembly specific things and less the logic e.g. what I could've written better, like loops

SECTION .data           ; Section containing initialised data
    Base64Table: db "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"
    OutBuf: db "xxxx"
    OutBufLen: equ $ - OutBuf

SECTION .bss            ; Section containing uninitialized data
    InBufLen: equ 3
    InBuf:    resb InBufLen

SECTION .text           ; Section containing code

global _start           ; Linker needs this to find the entry point!

_start:

read:
    ; read chunk from stdin to InBuf
    mov rax, 0                      ; sys_read
    mov rdi, 0                      ; file descriptor: stdin
    mov rsi, InBuf                  ; destination buffer
    mov rdx, InBufLen               ; maximum # of bytes to read
    syscall

    ; check number of bytes read
    cmp rax, 0                      ; did we receive any bytes?
    je exit; if not: exit the program

    xor r10, r10                    ; clear r10
    mov r10, rax                    ; save # of bytes read

    cmp rax, 2                      ; If only two bytes were read call specific subroutine
    je twobyte

    cmp rax, 1
    je onebyte

    ; clear r10, r11 and rax for future usage
    xor r11, r11
    xor rax, rax

process:
    mov eax, [InBuf]

    ; convert little endian to big endian and shifting right by 8 to remove 0x00
    ; which we only got because we saved a 24 bit value into a 32 bit register
    bswap eax                       ; example: 0x004e5089 -> 0x89504e00
    shr eax, 8                      ; example: 0x89504e00 -> 0x0089504e

    ; clean rdx and rbx to store values there and then copy eax into edx to use edx to shift the values around
    xor rdx, rdx
    xor rbx, rbx
    mov edx, eax

    ; Get second char in 3 byte chunk
    shr edx, 6                      ; move first char out of the way
    and edx, 0x3F                   ; mask 3 and 4 char
    mov bl, [Base64Table + rdx]     ; Lookup and add to ebx on the 4 place xxxx -> xxxV
    xor rdx, rdx                    ; clear rdx
    mov edx, eax                    ; fill edx with eax

    and edx, 0x3F                   ; get first char by masking all others
    mov bh, [Base64Table + rdx]     ; lookup and add to ebx on the 3 place xxxV -> xxiV
    xor rdx, rdx                    ; clear rdx
    mov edx, eax                    ; fill edx with eax

    shl ebx, 16                     ; move 3 and 4th place of ebx to first and second xxiV -> iVxx

    shr edx, 18                     ; move away 1st, 2nd and 3d char out of edx
    mov bl, [Base64Table + rdx]     ; look up remaining 4th char and add to ebx iVxx -> iVxB
    xor rdx, rdx                    ; clear rdx
    mov edx, eax                    ; fill edx with eax

    shr edx, 12                     ; move 1st and 2nd char out of edx
    and edx, 0x3F                   ; mask 4 char
    mov bh, [Base64Table + rdx]     ; look up remaining 3th char and add to 3th place in ebx iVxB -> iVOB

    mov [OutBuf + r11], ebx         ; move content of ebx to the output at the correct place (r11)
    push r11                        ; save r11
    push r10                        ; save r10
    call write                      ; write output
    pop r10                         ; get r10 back
    pop r11                         ; get r11 back

    add r11, 4                      ; add 4 to output index
    cmp r11, r10                    ; check if all data read in was processed yet by comparing the numbers of read in bytes to the index in the output
    jl process                      ; if r11 is lower, go to process

    jmp read                        ; else read new data

onebyte:
    ; Clear registers
    xor rdx, rdx
    xor rbx, rbx
    xor rax, rax

    ;Add two = and move them up 16 to make room for the last two chars
    mov bh, 0x3D
    mov bl, 0x3D
    shl ebx, 16

    mov eax, [InBuf]

    and eax, 0xFF                   ; Mask all other bytes except last one

    shl eax, 4                      ; Add 4 zeroes at the end

    mov edx, eax

    shr edx, 6                      ; move first char out of edx
    and edx, 0x3F                   ; mask everything except the remaining (second) 6 bites
    mov bl, [Base64Table + rdx]     ; Look up and write to bh
    xor rdx, rdx
    mov edx, eax

    and edx, 0x3F                   ; Mask second char away
    mov bh, [Base64Table + rdx]     ; Lookup first char and add to output

    ; Write and exit the programm
    mov [OutBuf], ebx
    call write
    jmp exit

twobyte: ;TODO Document
    ; Clear registers
    xor rdx, rdx
    xor rbx, rbx
    xor rax, rax


    mov eax, [InBuf]

    and eax, 0xFFFF                 ; Mask all other bytes except the two last ones

    bswap eax
    shr eax, 16                     ; bswap ax doesn't work so we have to do this manually

    shl eax, 2                      ; Add 2 zeroes at the end

    ; Add one =
    mov bh, 0x3D
    mov edx, eax

    and edx, 0x3F
    mov bl, [Base64Table + rdx]
    xor rdx, rdx
    mov edx, eax

    shl ebx, 16

    shr edx, 6
    and edx, 0x3F
    mov bh, [Base64Table + rdx]
    xor rdx, rdx
    mov edx, eax

    shr edx, 12
    and edx, 0x3F
    mov bl, [Base64Table + rdx]

    ; Write and exit the programm
    mov [OutBuf], ebx
    call write
    jmp exit

write: ;TODO Document
   mov rax, 1
   mov rdi, 1
   mov rsi, OutBuf
   mov rdx, OutBufLen
   syscall
   ret

exit: ;TODO Document
    mov rdi, 0
    mov rax, 60
    syscall
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

A few thoughts, without examining everything:

  1. A classic Unix and/or C programming error: Assuming that read() will fill the buffer except at end of file. This might (usually) be true when reading from a file, but standard input might easily be a terminal, and return 1 character because the user pressed return.
  2. Error handling is also missing.
  3. You really shouldn't assume you can write mov eax, [InBuf] unless you have allocated InBuf large enough. If it winds up at address fffffffffffffffd then you would get an exception. (OK, that address is unlikely, but it could also cross a page boundary into an invalid page.)
  4. OutBuf doesn't need to be initialized either. (as in, you can move it to BSS.)
  5. You compare the number of bytes read with 0, 1, and 2. Compare it with the most expected value (3) first, and make sure the branch prediction works for it being 3. Also consider making several conditional jumps based on 1 comparison. For instance, if you have already eliminated the possibility of 3, you can distinguish 0, 1, and 2 by comparing with 1, using je to the 1 case, jl for the 0, and jg for 2. Of course, don't do all three conditionally. Also, do remember you could also get other values in the error case.
\$\endgroup\$
0
\$\begingroup\$

... ways I could improve my code.
... assembly specific things

Here's a list of improvements that you can make. Many occur multiple times throughout your code.

mov rax, 0                      ; sys_read
mov rdi, 0                      ; file descriptor: stdin

The best way to zero a register is to 'exclusive or' the register with itself: xor rax, rax. And because writing to the low dword of a 64-bit register automatically clears the high dword, we can write this one byte shorter using: xor eax, eax.

cmp rax, 0                      ; did we receive any bytes?
je exit                         ; if not: exit the program

The preferred way to check whether a register is zero, is to 'test' the register with itself: test rax, rax jz exit. A nice thing to remember is that it is more idiomatic to use jz (JumpIfZero) after test, just like it is more idiomatic to use je (JumpIfEqual) after cmp.

xor r10, r10                    ; clear r10
mov r10, rax                    ; save # of bytes read

There's no point in zeroing a register right before loading it with a value that is going to fill the entire register anyway.

xor rdx, rdx                    ; clear rdx
mov edx, eax                    ; fill edx with eax

Similar to the previous point. The mov edx, eax instruction effectively loads the whole 64-bit register (automatically zeroing the high dword), so a separate clearing is redundant.

cmp r11, r10                    ; check if all data ...
jl process                      ; if r11 is lower ...

Both R11 and R10 contain unsigned data. It would be best to use the matching unsigned conditional jump: jb (JumpIfBelow).
jl stands for JumpIfLess and pertains to signed numbers.

mov bh, 0x3D
mov bl, 0x3D
shl ebx, 16

You can write this in a single instruction: mov ebx, 0x3D3D0000.

mov eax, [InBuf]
and eax, 0xFF                   ; Mask all other bytes ...

For cases like this one, the instruction set has the movzx (MoveWithZeroExtension) instruction. With movzx eax, byte [InBuf], you will load 1 byte from memory and have the other 7 bytes zeroed.

mov eax, [InBuf]
and eax, 0xFFFF                 ; Mask all other bytes ...

Similar to the previous point, but using word tag.
movzx eax, word [InBuf] will load 2 bytes from memory and zero the other 6 bytes.

bswap eax
shr eax, 16 ; bswap ax doesn't work so we have to do this manually

You can exchange byte-sized registers, so replace this pair of instructions by xchg al, ah. The other parts of RAX were already zeroed.

Beware of this (future) buffer overflow

You have defined a 4-byte output buffer via OutBuf: db "xxxx".
The code then fills this buffer with mov [OutBuf + r11], ebx (R11 == 0).
Fine, but seeing add r11, 4 ; add 4 to output index suggests that in some later version of your program you expect to write more data and you don't have the room for it!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.