search

exercism / x86-64-assembly

Exercism exercises in x86-64 Assembly.
https://exercism.org/tracks/x86-64-assembly
MIT License
22 stars 18 forks source link

All tests pass with no code submitted, in x86-64 Assembly - Triangle #164

Closed TheRealOwenRees closed 9 months ago

TheRealOwenRees commented 2 years ago

In the browser code editor, all tests pass if you do not submit any code. This user did just that and it has passed.

I have not tested this with a CLI submission.

sudhackar commented 1 year ago

Nice catch! I ported this exercise to the x86 track. I can see why this could happen - by calling convention the return(al in x86) isn't changed at all and could contain some garbage value from a previous call value which could trigger this outcome.

Here I see that rax could contain the address of the function being tested and that pointer would always pass the TRUE check. For FALSE check only the LSB is negated by xor, al, 1 and that passes the check again since its non zero.

Changing datatype to int might work here - I don't have a plan to fix this at the moment.

rdipardo commented 1 year ago

I have not tested this with a CLI submission.

An empty solution does not pass the tests locally, at least not in the following environment:

glibc version: 2.36
gcc (Debian 12.2.0-14) 12.2.0
GNU Make 4.3

$ make
triangle_test.c:24:test_equilateral_triangle_all_sides_are_equal:FAIL: Expected TRUE Was FALSE
triangle_test.c:138:test_equilateral_triangle_any_side_is_unequal:PASS
triangle_test.c:139:test_equilateral_triangle_no_sides_are_equal:PASS
triangle_test.c:140:test_equilateral_triangle_all_zero_sides_is_not_a_triangle:PASS
triangle_test.c:48:test_equilateral_triangle_sides_may_be_floats:FAIL: Expected TRUE Was FALSE
triangle_test.c:54:test_isosceles_triangle_last_two_sides_are_equal:FAIL: Expected TRUE Was FALSE
triangle_test.c:60:test_isosceles_triangle_first_two_sides_are_equal:FAIL: Expected TRUE Was FALSE
triangle_test.c:66:test_isosceles_triangle_first_and_last_sides_are_equal:FAIL: Expected TRUE Was FALSE
triangle_test.c:72:test_isosceles_triangle_equilateral_triangles_are_also_isosceles:FAIL: Expected TRUE Was FALSE
triangle_test.c:146:test_isosceles_triangle_no_sides_are_equal:PASS
triangle_test.c:147:test_isosceles_triangle_first_triangle_inequality_violation:PASS
triangle_test.c:148:test_isosceles_triangle_second_triangle_inequality_violation:PASS
triangle_test.c:149:test_isosceles_triangle_third_triangle_inequality_violation:PASS
triangle_test.c:102:test_isosceles_triangle_sides_may_be_floats:FAIL: Expected TRUE Was FALSE
triangle_test.c:108:test_scalene_triangle_no_sides_are_equal:FAIL: Expected TRUE Was FALSE
triangle_test.c:152:test_scalene_triangle_all_sides_are_equal:PASS
triangle_test.c:153:test_scalene_triangle_two_sides_are_equal:PASS
triangle_test.c:154:test_scalene_triangle_may_not_violate_triangle_inequality:PASS
triangle_test.c:155:test_scalene_triangle_sides_may_be_floats:PASS

-----------------------
19 Tests 8 Failures 0 Ignored
FAIL
make: *** [Makefile:29: all] Error 8

See next comment

The real problem is the test runner ignores the exit code of the make task: https://github.com/exercism/x86-64-assembly-test-runner/blob/2360f54b486d33974faa1dbe96b6811f7ea270c9/run.sh#L13

The shell only gets the exit code returned by the python3 command. As long as it runs the script successfully (and it always will), the exit code is 0 regardless of what "results.out" actually contains. It's an easy enough oversight to make; the Scheme track is currently suffering from a similar one.

A simple fix would be storing the exit code of stdbuf (which is non-zero when a test fails). Then, after running the Python script, exit with whatever stdbuf returned

bergjohan commented 1 year ago

I've had this issue before because of using bool as a return type. My fix last time was to switch to returning an int instead. See https://github.com/exercism/x86-64-assembly/issues/69 and https://github.com/exercism/x86-64-assembly/pull/70

rdipardo commented 1 year ago

It seems an outdated version of GCC was the problem all along.

A Debian 10 image with only the standard-issue compiler behaves identically to Alpine 3.10 (the runner's current base image):

Debian (Buster) image output: 

$ docker-run.sh triangle ../exercism-x86-64-assembly/exercises/practice/triangle
[+] Building 0.8s (11/11) FINISHED
 => [internal] load build definition from Dockerfile                                                                                                             0.0s
 => => transferring dockerfile: 430B                                                                                                                             0.0s
 => [internal] load .dockerignore                                                                                                                                0.0s
 => => transferring context: 113B                                                                                                                                0.0s
 => [internal] load metadata for docker.io/library/debian:buster-slim                                                                                            0.4s
 => [internal] load build context                                                                                                                                0.0s
 => => transferring context: 707B                                                                                                                                0.0s
 => [1/6] FROM docker.io/library/debian:buster-slim@sha256:738002a6e94e8629a0f869be8ea56519887979d5ed411486441981a8b16f2fc8                                      0.0s
 => CACHED [2/6] RUN apt update     && apt -qqy --no-install-recommends install         gcc         libc-dev         make         nasm         python3-minimal   0.0s
 => CACHED [3/6] WORKDIR /opt/test-runner                                                                                                                        0.0s
 => [4/6] COPY run.sh bin/                                                                                                                                       0.0s
 => [5/6] COPY process_results.py ./                                                                                                                             0.0s
 => [6/6] COPY debug.asm ./                                                                                                                                      0.0s
 => exporting to image                                                                                                                                           0.1s
 => => exporting layers                                                                                                                                          0.1s
 => => writing image sha256:8a393b11a87b18db6d913b0faf5d314c44fb7943821b29712536b26231432501                                                                     0.0s
 => => naming to docker.io/library/test-runner                                                                                                                   0.0s
Solution code:
section .text

global is_equilateral
is_equilateral:
    ; Provide your implementation here
    ret

global is_isosceles
is_isosceles:
    ; Provide your implementation here
    ret

global is_scalene
is_scalene:
    ; Provide your implementation here
    ret

%ifidn __OUTPUT_FORMAT__,elf64
section .note.GNU-stack noalloc noexec nowrite progbits
%endif-e

triangle_test.c:137:test_equilateral_triangle_all_sides_are_equal:PASS
triangle_test.c:138:test_equilateral_triangle_any_side_is_unequal:PASS
triangle_test.c:139:test_equilateral_triangle_no_sides_are_equal:PASS
triangle_test.c:140:test_equilateral_triangle_all_zero_sides_is_not_a_triangle:PASS
triangle_test.c:141:test_equilateral_triangle_sides_may_be_floats:PASS
triangle_test.c:142:test_isosceles_triangle_last_two_sides_are_equal:PASS
triangle_test.c:143:test_isosceles_triangle_first_two_sides_are_equal:PASS
triangle_test.c:144:test_isosceles_triangle_first_and_last_sides_are_equal:PASS
triangle_test.c:145:test_isosceles_triangle_equilateral_triangles_are_also_isosceles:PASS
triangle_test.c:146:test_isosceles_triangle_no_sides_are_equal:PASS
triangle_test.c:147:test_isosceles_triangle_first_triangle_inequality_violation:PASS
triangle_test.c:148:test_isosceles_triangle_second_triangle_inequality_violation:PASS
triangle_test.c:149:test_isosceles_triangle_third_triangle_inequality_violation:PASS
triangle_test.c:150:test_isosceles_triangle_sides_may_be_floats:PASS
triangle_test.c:151:test_scalene_triangle_no_sides_are_equal:PASS
triangle_test.c:152:test_scalene_triangle_all_sides_are_equal:PASS
triangle_test.c:153:test_scalene_triangle_two_sides_are_equal:PASS
triangle_test.c:154:test_scalene_triangle_may_not_violate_triangle_inequality:PASS
triangle_test.c:155:test_scalene_triangle_sides_may_be_floats:PASS

-----------------------
19 Tests 0 Failures 0 Ignored
OK

gcc (Debian 8.3.0-6) 8.3.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

At least there's plenty of time to get this fixed before Nybbly November.

bergjohan commented 1 year ago

Well, I don't think we should depend on codegen by a specific compiler version. It might break again on the next upgrade. I still think the best way is to change the return type to int.

rdipardo commented 1 year ago

For illustration, attached is the disassembled triangle test program, generated with objdump -M intel -S tests, after building with gcc 8.3 and then gcc 12.

The first major difference is the .init section. GCC 8.3 calls the initialization subroutines frame_dummy and __do_global_ctors_aux, while gcc 12 does not:

$ diff -b tests.gcc.8.3.asm.txt tests.gcc.12.asm.txt | head -n 12
2c2
< tests.gcc.8.3:     file format elf64-x86-64
---
> tests.gcc.12:     file format elf64-x86-64
9,12c9,10
<     1001:     e8 a5 01 00 00          call   11ab <frame_dummy>
<     1006:     e8 e7 2e 00 00          call   3ef2 <__do_global_ctors_aux>
<     100b:     58                      pop    rax
<     100c:     c3                      ret
---
>     1001:     58                      pop    rax
>     1002:     c3                      ret

The purpose of frame_dummy seems to be initializing RSI and RDI whenever the register frame at rip+0x5e45 is not NULL:

00000000000011ab <frame_dummy>:
    11ab:   48 83 3d 45 5e 00 00    cmp    QWORD PTR [rip+0x5e45],0x0        # 6ff8 <__register_frame_info>
    11b2:   00
    11b3:   74 18                   je     11cd <frame_dummy+0x22>
    11b5:   55                      push   rbp
    11b6:   48 8d 35 a3 5e 00 00    lea    rsi,[rip+0x5ea3]        # 7060 <object.6149>
    11bd:   48 8d 3d 4c 38 00 00    lea    rdi,[rip+0x384c]        # 4a10 <__EH_FRAME_BEGIN__>
    11c4:   48 89 e5                mov    rbp,rsp
    11c7:   e8 b4 fe ff ff          call   1080 <__register_frame_info@plt>
    11cc:   5d                      pop    rbp
    11cd:   e9 1b ff ff ff          jmp    10ed <register_tm_clones>

This might be eliminated with optimization, but I think that would ultimately impoverish students' learning opportunities, at least for the ones using their PCs.