x86 Assembly read ints into struct array then print

Question

I'm supposed to read 10 ints, store them as x and y points in a struct, then print them out. esi is not incrementing correctly in my input loop. When I put the values into the struct in code, they print properly when I use my input loop I get:

What are the x and y values for the points?
1
2
3
4
5
6
7
8
9
0
The point coordinates are 
134520936
134520940
134520936
134520940
134520936
134520940
134520936
134520940
134520936
134520940

I need to master gdb, but the instructor hasn't even touched on it in the course I am taking.

Anyway, it appears that two memory addresses, one for x and one for y are getting stored over and over again into the struc array and then printed. I've tried extern C printf instead of the printDec function and I get the same output. Can you see why an address instead of an integer would be going into my point variables? Do you know if scanf modifies the esi register?

STRUC Point ;define a point structure
.x RESD 1   ;reserve 4 bytes for x coordinate
.y RESD 1   ;reserve 4 bytes for y coordinate
.size:
ENDSTRUC

section .data   ;data section

msgPt: db "What are the x and y values for the points?", 10, 0
msgPtL: equ $-msgPt

msg2: db"The point coordinates are ", 10  ,0
msgL2: equ $-msg2

formatPointScan: db "%d", 0

xVal: dd 0
yVal: dd 0

loopCount: dd 0

section .bss    ;bss section

PtArr: RESB Point.size*5    ;reserver place for 5 structures
;PtArr points to the start of array
ArrCount: EQU($ - PtArr)/Point.size ;should be 5


section .text

extern scanf

global main
main:
    push ebp
    mov ebp, esp

   mov ecx, msgPt
   mov edx, msgPtL
   call PString

    mov dword[loopCount], ArrCount  ;ecx has the number of array elements
    mov esi, PtArr  ;esi contains address of first struct in array

getPointsLoop:
    push xVal
    push formatPointScan
    call scanf
    mov dword[esi + Point.x], xVal  

    push yVal
    push formatPointScan
    call scanf
    mov dword[esi + Point.y], yVal
    add esi, Point.size
    dec dword[loopCount]
    cmp dword[loopCount], 0
jne getPointsLoop

    mov ecx, msg2
    mov edx, msgL2
    call PString
    mov dword[loopCount], ArrCount ;set ecx to num of array elements
    mov esi, PtArr  ;point esi to 1st element of array

printPointsLoop:
    mov eax, [esi + Point.x]    ;indirect access to x value
    call printDec
    call println

    mov eax, [esi + Point.y]    ;indirect access to y value
    call printDec
    call println

    add esi, Point.size
    dec dword[loopCount]
    cmp dword[loopCount], 0
jne printPointsLoop

    ;exit program and cleaning
    mov esp, ebp
    pop ebp
    ret

PString:; save register values of the called function
    pusha
    mov eax,4 ; use 'write' system call = 4
    mov ebx,1 ; file descriptor 1 = STDOUT
    int 80h ; call the kernel

    ; restore the old register values of the called function
    popa
    ret

println:
    ;will call PString func
    ;will change content of ecx and edx
    ;need to save registers used by the main program
    section .data
    nl db 10
    section .text
    pusha

    mov ecx, nl
    mov edx, 1
    call PString

    ;return original register values
    popa
    ret

printDec:
;saves all registers so they return unmodified
;build the function to handle dword size

    section .bss
    decstr resb 10 ; 10 32-bit digits
    ct1 resd 1 ;keep track of dec-string size

    section .text
    pusha; save registers

    mov dword[ct1],0 ;initially assume 0
    mov edi, decstr ; edi points to dec-string
    add edi, 9 ; moved to the last element of string
    xor edx, edx ; clear edx for 64-bit div
whileNotZero:
    mov ebx, 10 ; get ready to divide by 10
    div ebx ; divide by 10
    add edx, '0' ; convert to ascii
    mov byte[edi], dl ; put it in string
    dec edi ; move to next char in str
    inc dword[ct1] ; inc char counter
    xor edx, edx ; clear edx
    cmp eax, 0  ;is remainder 0?
    jne whileNotZero ;if no, keep on looping

    inc edi ; conversion finished, bring edi
    mov ecx, edi ; back to start of string. make ecx
    mov edx, [ct1] ; point to counterm edx gets # chars
    mov eax, 4 ; print to stdout
    mov ebx, 1
    int 0x80 ; call kernel

    popa ; restore registers
    ret

Answer 1

mov dword[esi + Point.x], xVal is a mov-immediate of the address. You're filling your array with copies of the address of the static storage location you pass to scanf, not the value scanf stored there.

mov [mem], [mem] is illegal, so you need to load into a temporary register to copy it somewhere else with mov.

Or better, pass scanf the right address in the first place. The only reason for your xVal to even exist is as scratch space for scanf, which is totally unnecessary. (Like most of the rest of your static storage where you could just use registers.)

section .rodata
fmt_2int: db "%d %d", 0

section .text

lea   eax, [esi + Point.y]
push  eax
push  esi         ; no LEA needed, Point.x = 0
push  fmt_2int
call  scanf        ; eax = scanf("%d %d", &arr[i].x, &arr[i].y);

add   esp, 12     ; pop args

Or you could store args with mov and leave stack space for them allocated throughout the loop instead of pushing and popping.

---

Note that esi,edi, and ebx are call-preserved in the i386 System V calling convention, along with ebp and esp. Your main doesn't preserve its caller's values of those registers. Which is ironic because you use a super-slow pusha / popa in functions you call from main, but which only clobber a few registers that main doesn't need them to preserve.

Normal library functions will clobber EAX, ECX, and EDX, so it makes sense to use that calling convention. (Passing args in registers like you're doing is good, though; the standard 32-bit calling convention that passes args on the stack is obsolete and inefficient.)

System calls made with int 0x80 preserve all registers (except EAX which gets the return value).

You don't need a separate println function: your printDec could print a newline at the end of the number.