Skip to content

labs/lab-05: Fix improper testing of 128 bit sum in sum-array subtask#164

Open
AndreiDurlea wants to merge 3 commits intocs-pub-ro:mainfrom
AndreiDurleaEX:andreidurlea-lab-05-fixes
Open

labs/lab-05: Fix improper testing of 128 bit sum in sum-array subtask#164
AndreiDurlea wants to merge 3 commits intocs-pub-ro:mainfrom
AndreiDurleaEX:andreidurlea-lab-05-fixes

Conversation

@AndreiDurlea
Copy link
Copy Markdown

@AndreiDurlea AndreiDurlea commented Mar 29, 2026

Fix improper testing of 128 bit sum in sum-array subtask

This fixes #165

TL;DR

I found a test in sum-array of HSI lab-05 which did passed my improper 128-bit addition code with full marks. Fixed it here

Problem description

In labs/lab-05/tasks/sum-array, the test.sh checker expects the last subtask (128-bit addition) to result in 0x1565ddbe509d3ffe8, which is a 65-bit value.

The test used bash arithmetic evaluation with the -eq operator:

if [[ $big_sum -eq $((0x1565ddbe509d3ffe8)) ]]; then

Because bash evaluates arithmetic as 64-bit signed integers, $((0x1565ddbe509d3ffe8)) removes the top bit and compares against the lower 64-bits, making any logic for 128-bit addition redundant.

If a student's assembly code incorrectly computes the sum by simply ignoring the carry for the 128-bit addition and prints the lower 64-bits in decimal, they still get full marks.

Proof
Given my lazy solution

    ; TODO Compute sum for elements in big_qword_array

    mov rcx, 3
    xor rax, rax

add_big_qword_array_element:
    mov rdx, QWORD [big_qword_array + rcx * 8 - 8]
    add rax, rdx
    loop add_big_qword_array_element

    PRINTF64 `128-bit addition example: %llu\n\x0`, rax ; stupid checker

    leave
    ret

This will produce 128-bit addition example: 6223372036854775288. Because 6223372036854775288 -eq 6223372036854775288 within the bounds of -eq, the bash check returned true successfully.

andrei@UPB-AC-Host1:.../hardware-software-interface-labs/labs/lab-05$ cd tasks/sum-array/tests/
andrei@UPB-AC-Host1:.../tasks/sum-array/tests$ ./run_all_tests.sh 

word_sum_test                    ........................ passed ...  20
dword_sum_test                   ........................ passed ...  20
qword_sum_test                   ........................ passed ...  30
big_sum_test                     ........................ passed ...  30

========================================================================

Total:                                                           100/100

Fix

Replaced the numerical comparison -eq with an exact string comparison == in tests/test.sh.

if [[ "$big_sum" == "0x1565ddbe509d3ffe8" ]]; then

@AndreiDurlea
Copy link
Copy Markdown
Author

AndreiDurlea commented Mar 29, 2026

@rarescazan30 @teodutu can you review this when you're free?

Copy link
Copy Markdown

@teodutu teodutu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @AndreiDurlea, nice catch. Before I merge this, fix the checkpatch errors [1]: add a commit description and a signed-off line.

[1] https://github.com/cs-pub-ro/hardware-software-interface/actions/runs/23713477889/job/69091298064?pr=164#step:3:217

@teodutu teodutu added the student-contrib Fix or improvement made by a student label Mar 29, 2026
Changed a int comparison to a string comparison in test.sh in order to accomodate >64-bit addition

Signed-off-by: Durlea Andrei<andrei.durlea@stud.acs.upb.ro>
@AndreiDurlea
Copy link
Copy Markdown
Author

*Fixed the commit description

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

student-contrib Fix or improvement made by a student

Projects

None yet

Development

Successfully merging this pull request may close these issues.

labs/lab-05 Improper testing of 128 bit sum in sum-array subtask

2 participants