Conversation
Update memory-layout and calling-convention reading materials to correctly reflect the 64-bit System V AMD64 ABI. Add the missing `xor rax, rax` before variadic function calls and convert 32-bit stack argument examples to 64-bit register arguments. Fix #141 Signed-off-by: alexf05 <alexfechet16@gmail.com>
Correct multiple issues introduced during the x86_64 transition: - max-c-calls: Fix signedness comparison by using `jae` and fix pointer initialization bug in the solution. Revert support file. - regs-preserve: Add required 16-byte stack alignment before calling printf and fix the order of stack restoration in the solution. Fix #141 Signed-off-by: alexf05 <alexfechet16@gmail.com>
Signed-off-by: alexf05 <alexfechet16@gmail.com>
teodutu
left a comment
teodutu
left a comment
There was a problem hiding this comment.
You're right about max-c-calls-x86. Please convert it to x86. As for cleaning the stack in max-assembly-calls-x86, that's not needed because leave already does that by settign rsp = rbp, so leave this as it is.
| leave | ||
| ret | ||
|
|
||
| ret No newline at end of file |
|
|
||
| mov rdi, fmt | ||
| mov rsi, text | ||
| xor rax, rax |
There was a problem hiding this comment.
Explain why clearing al is needed for variadic functions. And I'd say you should only clear al itself as the System-V ABI requires to be precise.
|
|
||
| ; RDI = array pointer | ||
| ; RSI = array length | ||
| ; RSI = array lengthget_max |
There was a problem hiding this comment.
| ; RSI = array lengthget_max | |
| ; RSI = array length |
A wrong copy-paste I guess.
| mov rcx, rsi | ||
|
|
||
| next: | ||
| ; TODO1: uncomment the following two lines |
There was a problem hiding this comment.
Which two? Only one is commented. Also add a comment to remember to align the stack to 16 bytes for printf.
|
|
||
| leave | ||
| ret | ||
| ret No newline at end of file |
| compare: | ||
| cmp eax, [rdi + 4*rcx] | ||
| jge check_end | ||
| jae check_end ; <-- Keep the unsigned fix here! |
There was a problem hiding this comment.
| jae check_end ; <-- Keep the unsigned fix here! | |
| jae check_end |
| sub rsp, 8 | ||
| xor rax, rax | ||
| mov rdi, newline | ||
| call printf | ||
|
|
||
| add rsp, 8 | ||
|
|
There was a problem hiding this comment.
| sub rsp, 8 | |
| xor rax, rax | |
| mov rdi, newline | |
| call printf | |
| add rsp, 8 | |
| xor rax, rax | |
| mov rdi, newline | |
| call printf | |
Adding these lines here actually causes a seg fault.
|
And don't forget to fix the checkpatch error: https://github.com/cs-pub-ro/hardware-software-interface/actions/runs/23609427165/job/68760945678?pr=160 |
2 participants
Prerequisite Checklist
Description of changes
This PR resolves the remaining bugs, ABI violations, and typos from the x86_64 transition in
lab-09.Specific fixes include:
raxfor variadic functions likeprintfand updating 32-bit stack argument examples to 64-bit register arguments).max-c-calls: Fixed a logical bug by changing the signed comparison (jge) to unsigned (jae). Fixed pointer initialization in the solution.regs-preserve: Added the required 16-byte stack alignment (sub rsp, 8) to the support code before callingprintfto prevent immediate segfaults. Corrected the stack restoration order (pop rbxvsadd rsp, 8) in the solution.Question for Reviewer (@teodutu): I noticed that the
max-c-calls-x86task currently contains 64-bit code instead of 32-bit, and themax-assembly-calls-x86task is missingadd esp, Xstack cleanups for thecdeclcalling convention. Should I convert these files back to proper 32-bit assembly in a follow-up commit, or are these x86 legacy folders planned for deletion?Fix #141