Converting C code to x86-64 assembly

Question

I'm trying to convert my C code to x86-64. My goal is to reverse a linked list. The two parameters that are passed in are the head ptr and the offset to to get the address of the pointer field (i.e. the pointer to the next node in the list).

From what I understand, the head ptr is passed in through the rdi register, and the offset is passed in through the rsi register. I keep getting a segmentation fault when it reaches the line "mov rcx, [rbx]." The segmentation fault goes away when it's just "mov rcx, rbx" and the line following is changed from "mov [rbx], rdx" to "mov rbx, rdx." However, I end up in an infinite loop because it keeps simply assigning the same values over and over again.

When I'm following along with my C code, all of the logic in the x86-64 makes sense to me, so I'm really at a standstill. Any ideas? This is my first time using x86-64.

.intel_syntax noprefix
.text
.global reverse_asm_64

reverse_asm_64:
push rbx
push r12

mov rax, 0x0
#headptr
mov rbx, rax
#nextptr
mov rcx, rax
#new_headptr
mov rdx, rax
#head
mov rax, [rdi]

#checks if head is null
cmp rax, 0
je null_ret

#move offset into a register
mov r12, rsi
add rax, r12
#add offset to rax to get the next ptr
mov rbx, rax

while_start:

#checks that next ptr isn't null
cmp rbx, 0x0
je while_done

#setting the next ptr
mov rcx, [rbx]

# *headptr = new_headptr
mov [rbx], rdx

#new_headptr = headptr
mov rdx, rbx

#sets headptr to nextptr
mov rbx, rcx

jmp while_start

while_done:
mov rax, rdx
sub rax, rsi

null_ret:
pop r12
pop rbx
ret

Answer 1

I'm reluctant to just post the re-worked code I created as part of writing this answer. You aren't going to learn anything that way.

So, here are some things you might want to fix to start:

1) Given that linux has ~7 registers you can use for scratch, there doesn't seem to be a need to push/pop rbx and r12. Use other registers that don't need to be saved.

2) It looks like you are putting the comments after the code they describe (#headptr et al). This is not what people who read your code are going to expect. Most common is either to put it on the line before, or (especially in assembler) on the same line.

3) It is common practice in C to always zero out all the variables (especially pointers) before you use them. However, less so in asm. Especially when the next statement is going to assign a different value to that same register. This isn't a problem in C since the compiler's optimizers will simply discard the redundant initializer. But assembler doesn't have optimizers, so this is just a waste of space/cycles. Only zero things that must be zeroed.

4) When zeroing a register, use xor eax, eax instead of mov. It smaller/faster.

5) If your code is called with head_ptr = reverse_asm_64(head_ptr, 16), you are going to want to check rdi to see if it is null before dereferencing it.

6) In asm, you should use test rdi, rdi to see if rdi is zero rather than cmp rdi, 0. It's smaller/faster.

7) "move offset into a register" Say what? The offset is already in a register: rsi. Why make a copy in r12?

8) The very first time you "checks that next ptr isn't null", you have just added the offset to the value. Unless your offset is zero, this isn't going to do what you intend. See also #6.

9) The "add offset to rax to get the next ptr" is only done once (ie outside the loop). Wouldn't each pointer in the list need to have this offset added?

There's more, but that's 9 items. Seems like enough for a start.