CODEWITHSUNDEEP
assemblysegmentation-faultnasm
kmer14
kmer14

Reputation: 11

NASM: getting segmentation fault in my print function

Code:

;The value you want to print must be in the eax register
;one 32-byte resb to hold bit value
;and must be newline db

printDD_start:
    push ebx
    push ecx
    push edx

    push esi
    push esp

    mov esi,0
printDD:
    cmp esi,31
    jg printDD_printStart

    test eax,1
    jz printDD_zero
    jmp printDD_one
printDD_zero:
    mov dword [printDD_var_save + esi],48
    inc esi
    shr eax,1
    jmp printDD
printDD_one:
    mov dword [printDD_var_save + esi],49
    inc esi
    shr eax,1
    jmp printDD

printDD_printStart:
    mov esi,31
printDD_print:
    cmp esi,0
    jl printDD_exit

    mov esp,printDD_var_save
    add esp,esi

    mov eax,4
    mov ebx,1
    mov ecx,esp
    mov edx,1
    int 80h

    dec esi
    jmp printDD_print

printDD_exit:
    mov eax,4
    mov ebx,1
    mov ecx,newline
    mov edx,1
    int 80h


    pop ebx
    pop ecx
    pop edx
    pop esi
    pop esp

    ret

I'm printing the bits of a 4-byte variable to the screen. The program is running. It prints the value in the EAX register correctly but I get the latest segmentation fault error.

Upvotes: 1

Views: 96

Answers (1)

Sep Roland
Sep Roland

Reputation: 39166

push ebx
push ecx
push edx

...

pop ebx
pop ecx
pop edx

The stack is a Last-In-First-Out structure (LIFO). The value that you pushed first must come off latest. I often tag these push and pop instructions expressly to more easily track errors:

push ebx    ; (1)
push ecx    ; (2)
push edx    ; (3)

...

pop  edx    ; (3)
pop  ecx    ; (2)
pop  ebx    ; (1)

mov esp,printDD_var_save
add esp,esi

You have chosen to calculate the address for the character in the ESP register. Not only is this very unusual, you don't need to use the extra register since you're going to have to store this address in the ECX register anyway. You can even do it in one instruction:

lea  ecx, [printDD_var_save + esi]

mov dword [printDD_var_save + esi],48
mov dword [printDD_var_save + esi],49

The character is a byte. Better then also write it as a byte:

mov byte [printDD_var_save + esi], 48
mov byte [printDD_var_save + esi], 49

You can simplify the task of converting the bits of EAX into characters "0" or "1". The below suggested solution shifts out the bits one by one and adds the value from the carry flag (which represents the shifted out bit) to the ASCII code of the "0" character. The result of the adc dl, 0 will therefore be either the character "0" or the character "1".
You can also optimize your loops with fewer jumps. Because you already know that these loops will run at least once, you can omit the test at the top of the loop and only have a suitable test near the bottom of the loop where you will be jumping back conditionally.

printDD_start:
    push ebx
    push ecx
    push edx
    push esi

    xor  esi, esi   ; Better than `mov esi, 0`
printDD:
    mov  dl, 48     ; You can replace the 48 by "0"
    shr  eax, 1
    adc  dl, 0      ; -> DL = {"0","1"}
    mov  [printDD_var_save + esi], dl
    inc  esi
    cmp  esi, 32
    jb   printDD

    dec  esi        ; 32 - 1 = 31
printDD_print:
    mov  eax, 4
    mov  ebx, 1
    lea  ecx, [printDD_var_save + esi]
    mov  edx, 1
    int  80h
    dec  esi        ; Until ESI == -1
    jns  printDD_print

printDD_exit:
    mov  eax, 4
    mov  ebx, 1
    mov  ecx, newline
    mov  edx, 1
    int  80h

    pop  esi
    pop  edx
    pop  ecx
    pop  ebx
    ret

I don't expect the new code to segfault any more...

Upvotes: 2

Related Questions