Adding address sanitizer to auto running tests

[elyashiv]

Some of the most common errors are only caught by the address sanitizer.

I started looking into adding it into the tests runner.

The first obstacle I encountered is that alpine linux doesn't seem to support the address sanitizer, and it is used as the docker base.

1. Is it that the address sanitizer won't work on alpine linux?
2. Is it necessarily we use alpine? Can we use a different dist?

[bergjohan]

Thanks for looking into this!

You probably need to install clang. Did you try adding clang to the apk add command in the Dockerfile?

[elyashiv]

I tried installing clang, and setting the environment variable CC to clang.

I get:

/usr/bin/ld: cannot find /usr/lib/clang/8.0.0/lib/linux/libclang_rt.asan-x86_64.a: No such file or directory

I think it doesn't work with alpine: google/sanitizers#1080

[bergjohan]

Yeah, you're right!

I think alpine is preferred, since it's a really lightweight distribution, but it might be possible to choose another distro if we have to.

I'm not sure how we would add this to the test-runner based on how it works right now. Do we really want the solution to fail, if the sanitizer gives a warning?

I just remembered that @wolf99 opened an issue on this in the automated-tests repo, see: exercism/automated-tests#39

[elyashiv]

I think that if a student has a buffer over flow, a memory leak or a use after free, their solution is wrong, and I will want to detect this. These at not warnings, but bugs (and potential vulnerabilities) that are detected.

…

On Mon, Jul 20, 2020, 13:26 Johan Berg ***@***.***> wrote: Yeah, you're right! I think alpine is preferred, since it's a really lightweight distribution, but it might be possible to choose another distro if we have to. I'm not sure how we would add this to the test-runner based on how it works right now. Do we really want the solution to fail, if the sanitizer gives a warning? I just remembered that @wolf99 <https://github.com/wolf99> opened an issue on this in the automated-tests repo, see: exercism/automated-tests#39 <exercism/automated-tests#39> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#10 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMBEONJLZJ2M5PAQ2FMGJTR4QLU7ANCNFSM4O6G23GQ> .

[bergjohan]

I agree that we would want to detect this. However, for a student trying to learn C, an asan error message will probably be very confusing. I think it would be better if we could run this separately, and not as a part of the test run. And let mentors provide automatic feedback which will be more easy to understand for the students.

[elyashiv]

The output for a leak (gcc 9.3 on ubuntu) is:

==205801==ERROR: LeakSanitizer: detected memory leaks                                                                                         
...

Direct leak of 1000 byte(s) in 1 object(s) allocated from:
    #0 0x7f32e23b4bc8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
    #1 0x5641dee94446 in new_list src/list_ops.c:5
    #2 0x5641dee98aa0 in test_append_empty_lists test/test_list_ops.c:88
    #3 0x5641dee98517 in UnityDefaultTestRun test/vendor/unity.c:1813
    #4 0x5641dee9aef6 in main test/test_list_ops.c:323
    #5 0x7f32e20dc0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

SUMMARY: AddressSanitizer: 38000 byte(s) leaked in 38 allocation(s)

(Just added a malloc somewhere and compiled).

This is pretty straight froward - just a stack trace of an allocation that wasn't free. This is, hopefully easy enough to swallow.

This is the output for a buffer overflow:

Compiling memcheck
=================================================================
==206206==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000018 at pc 0x557d96da248a bp 0x7ffd85e2fa40 sp 0x7ffd85e2fa30
WRITE of size 4 at 0x602000000018 thread T0
    #0 0x557d96da2489 in new_list src/list_ops.c:6
    #1 0x557d96da6af5 in test_append_empty_lists test/test_list_ops.c:88
    #2 0x557d96da656c in UnityDefaultTestRun test/vendor/unity.c:1813
    #3 0x557d96da8f4b in main test/test_list_ops.c:323
    #4 0x7f9ce85820b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #5 0x557d96da236d in _start (/home/elyashiv/exercism/users/mstange22/c/list-ops/memcheck.out+0x336d)

0x602000000018 is located 7 bytes to the right of 1-byte region [0x602000000010,0x602000000011)
allocated by thread T0 here:
    #0 0x7f9ce885abc8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
    #1 0x557d96da2446 in new_list src/list_ops.c:5
    #2 0x557d96da6af5 in test_append_empty_lists test/test_list_ops.c:88
    #3 0x557d96da656c in UnityDefaultTestRun test/vendor/unity.c:1813
    #4 0x557d96da8f4b in main test/test_list_ops.c:323
    #5 0x7f9ce85820b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

SUMMARY: AddressSanitizer: heap-buffer-overflow src/list_ops.c:6 in new_list
Shadow bytes around the buggy address:
  0x0c047fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c047fff8000: fa fa 01[fa]fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==206206==ABORTING
make: *** [makefile:28: memcheck] Error 1

Generated with:

int *a = malloc(1);
a[2] = 3;
free(a);

This is a little harder to swallow, but the main part is the stack trace.

I guess we can filter some of this, or help a student having trouble.

[bergjohan]

I'll cc @exercism/c for some more input on this.

For those of you who don't know what this repo is about, it is used to enable in-browser coding for version 3 of Exercism. The student will be able to press a button and run the tests in a Docker container and be presented with the results.

To see this in action: https://research.exercism.io/experiments/1

[arcuru]

We've maintained a memory check Make target for years in the C track (first with valgrind and later using ASAN), but it was never enabled by default so that students who aren't expecting it don't see it. I think there's a lot of value to having it but I do worry about a student writing their first C program and seeing that error.

If it was explained first for the student, or if it was somehow opt-in to avoid confusion, it would be much more useful. I haven't followed much of the v3 work, but there could easily be a whole exercise showing "here's how C lets you do dangerous things with memory, here's why you don't want to do that, and here is a tool (ASAN) that we'll now run on the exercises to make sure those problems are caught".

[wolf99]

Exercism V3 doesn't really focus so much on aspects that different tools can bring (such as a memory sanitizer).
There has been some discussions on teaching use of specific tools (which this implies) before but the general consensus is that that would fall on the proficiency side of the fluency vs proficiency divide, see the two sections here: https://exercism.github.io/v3/#/docs/concept-exercises?id=what-do-we-mean-by-concepts

With that in mind, and the reasoning with which we left memory checking as optional in V2, I would err on the side of leaving it as optional. At least for now. I guess depending on how V3 pans out it may be good to add in a at a later stage.

May be worth raising in the V3 slack channel though to get some more opinions on.

---

@patricksjackson

for years in the C track

Has it been that long already! 😄 Time flies when you're having fun

[elyashiv]

This discussion has divided into three different things:

1. Should we support address sanitizer.
2. Should we run it by default/every time/something else.
3. How to create a docker that will run it.

If someone thinks that 1 shouldn't be answered with "YES!", I suggest we move that discussion into a separate issue, just so things are kept organized.

I opened issue #11 for discussing point 2.

Point 3 is actually the reason I opened this issue. Should I move this discussion into a separate issue? Should I edit the title?

[bergjohan]

I asked a question about this in the Exercism slack v3 channel. It was suggested by @iHiD and @ErikSchierboom to use the analyzer for this, see https://github.com/exercism/automated-analysis

[iHiD]

(@elyashiv Happy to answer questions on the why/what/how behind that if you have any! :))