Search code examples
assemblyx86-64nasmyasm

Segmentation fault in NASM assembly code


My assembler is YASM and I am coding on 64-bit Linux.

I assemble using yasm -f elf -m amd64 -g dwarf2 filename.asm and link using ld

I'm trying to implement selection sort. rdi and rsi are pointing to various parts of a strbuf2 resb 10 array. What could possibly be the reason for this segmentation fault? Lines 105 and 106 do the exact same type of operation, so why does it crash on line 106 but not line 105?

I've included the relevant portion of the code, and the gdbtui screenshot when it crashes.

UPDATE: Counters fixed

; ====== Sorting begins here ======
; Register uses:
; bpl holds the current minimum value
; r8 holds the memory address of the current minimum value
; rdi points to the boundary of the "outer loop"
; rsi points to the boundary of the "inner loop"
sorting:
    mov rdi, strbuf2  ; outer loop pointer
    mov rsi, strbuf2+1  ; inner loop pointer
    mov rax, 1  ; inner loop counter
    mov rbx, 0  ; outer loop counter

innerloop:
    mov bpl, [rdi] ; assume beginning element of unsorted array is minimum

    ; store the value of first element of unsorted array
    mov dl, [rdi]

    ; compare the current small value with the value in rsi
    mov cl, [rsi]   
    cmp bpl, cl 
    jg  new_small

    inc rsi
    inc rax
    cmp rax, 9
    jle innerloop
    jg  innerloop_done

new_small:
    inc rax
    mov bpl, cl; save the new small value
    mov r8, rsi  ; save its index 
    inc rsi 
    cmp rax, 9
    jle     innerloop

innerloop_done:
    ; When the inner loop is completed...
    ; First, do the swap
    ; to swap r8 (target memory address)  with [rdi] (outer array boundary)
    mov dl, 0 ; initialise
    mov dl, [rdi]
    mov [rdi], bpl
    mov [r8], dl 

    inc rdi  ; move the outer loop pointer forward
    inc rsi  ; move the inner loop pointer forward
    inc rbx  ; increment the outer loop counter (the unsorted array becomes smaller)

    ; set the inner loop counter to the appropriate position
    mov rax, 1 
    add rax, rbx  ; now rax (inner loop counter)
                  ; will always be rbx+1 (outer loop counter + 1) 
    cmp rbx, 9  
    jle innerloop
; ====== Sorting ends here ======

Segmentation fault gdb output


Solution

  • I think you're getting lost in the details of the implementation and forgetting what the code should do. I suggest that you first implement the code in C and then gradually change it to become ASM-like until the point when you can write it in ASM fully.

    Note the progression from the small, clean and easy to understand implementation in C in sortC1() to the somewhat messy but completely equivalent ASM-like implementation in sortAsm(). Use your favorite file comparison tool to see what changes between the implementations.

    The code:

    #include <stdio.h>
    #include <string.h>
    
    char strOriginal[11] = "8163045297";
    char str[11];
    
    void sortC1(void)
    {
      int outIdx, inIdx, minIdx;
      char min, tmp;
    
      for (outIdx = 0; outIdx < 10; outIdx++)
      {
        minIdx = outIdx;
        min = str[minIdx];
    
        for (inIdx = outIdx; inIdx < 10; inIdx++)
        {
          if (min > str[inIdx])
          {
            minIdx = inIdx;
            min = str[minIdx];
          }
        }
    
        tmp = str[outIdx];
        str[outIdx] = min;
        str[minIdx] = tmp;
      }
    }
    
    void sortC2(void)
    {
      char *outPtr, *inPtr, *minPtr;
      int outCnt, inCnt;
      char min, tmp;
    
      for (outPtr = str, outCnt = 0;
           outCnt < 10;
           outPtr++, outCnt++)
      {
        minPtr = outPtr;
        min = *minPtr;
    
        for (inPtr = outPtr, inCnt = 10 - outCnt;
             inCnt > 0;
             inPtr++, inCnt--)
        {
          if (min > *inPtr)
          {
            minPtr = inPtr;
            min = *minPtr;
          }
        }
    
        tmp = *outPtr;
        *outPtr = min;
        *minPtr = tmp;
      }
    }
    
    void sortC3(void)
    {
      char *outPtr, *inPtr, *minPtr;
      int outCnt, inCnt;
      char min, tmp;
    
      outPtr = str;
      outCnt = 0;
    
      while (outCnt < 10)
      {
        minPtr = outPtr;
        min = *minPtr;
    
        inPtr = outPtr;
        inCnt = 10 - outCnt;
    
        while (inCnt > 0)
        {
          if (min > *inPtr)
          {
            minPtr = inPtr;
            min = *minPtr;
          }
    
          inPtr++;
          inCnt--;
        }
    
        tmp = *outPtr;
        *outPtr = min;
        *minPtr = tmp;
    
        outPtr++;
        outCnt++;
      }
    }
    
    void sortC4(void)
    {
      char *outPtr, *inPtr, *minPtr;
      int outCnt, inCnt;
      char min, tmp;
    
      outPtr = str;
      outCnt = 0;
    
    outerloop:
    
      minPtr = outPtr;
      min = *minPtr;
    
      inPtr = outPtr;
      inCnt = 10 - outCnt;
    
    innerloop:
    
      if (min > *inPtr)
      {
        minPtr = inPtr;
        min = *minPtr;
      }
    
      inPtr++;
      inCnt--;
    
      if (inCnt > 0)
        goto innerloop;
    
      tmp = *outPtr;
      *outPtr = min;
      *minPtr = tmp;
    
      outPtr++;
      outCnt++;
      if (outCnt < 10)
        goto outerloop;
    }
    
    void sortAsm(void)
    {
      char* rdi; // points to the boundary of the "outer loop"
      char* rsi; // points to the boundary of the "inner loop"
      char* r8; // holds the current minimum value
      char r9b; // holds the current minimum value
      char r10b; // temporary storage for character exchange
      long long rbx; // outer loop counter
      long long rax; // inner loop counter
    
      rdi = str; // initialize outer loop pointer
      rbx = 0; // initialize outer loop counter
    
    outerloop:
    
      r8 = rdi; // assume current element of partially sorted array is minimum,
      r9b = *r8; // save its index and value
    
      rsi = rdi; // initialize inner loop pointer
      rax = 10; // initialize inner loop counter
      rax -= rbx;
    
    innerloop:
    
      // compare the current small value with the value in [rsi]
      if (r9b > *rsi)
      {
        r8 = rsi; // save the new small value's index
        r9b = *r8; // save the new small value
      }
    
      rsi++; // move the inner loop pointer forward
      rax--; // decrement the inner loop counter
      if (rax > 0)
        goto innerloop;
    
      // When the inner loop is completed...
      // First, do the swap
      // to swap [r8] (target memory address) with [rdi] (outer array boundary)
      r10b = *rdi;
      *rdi = r9b;
      *r8 = r10b;
    
      rdi++; // move the outer loop pointer forward
      rbx++; // increment the outer loop counter
      if (rbx < 10)
        goto outerloop;
    }
    
    int main(void)
    {
      strcpy(str, strOriginal);
      printf("before sorting: %s\n", str);
      sortC1();
      printf("after sorting : %s\n\n", str);
    
      strcpy(str, strOriginal);
      printf("before sorting: %s\n", str);
      sortC2();
      printf("after sorting : %s\n\n", str);
    
      strcpy(str, strOriginal);
      printf("before sorting: %s\n", str);
      sortC3();
      printf("after sorting : %s\n\n", str);
    
      strcpy(str, strOriginal);
      printf("before sorting: %s\n", str);
      sortC4();
      printf("after sorting : %s\n\n", str);
    
      strcpy(str, strOriginal);
      printf("before sorting: %s\n", str);
      sortAsm();
      printf("after sorting : %s\n\n", str);
    
      return 0;
    }
    

    The output:

    before sorting: 8163045297
    after sorting : 0123456789
    
    before sorting: 8163045297
    after sorting : 0123456789
    
    before sorting: 8163045297
    after sorting : 0123456789
    
    before sorting: 8163045297
    after sorting : 0123456789
    
    before sorting: 8163045297
    after sorting : 0123456789