Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Use ASAN for all (native) tests #20550

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

chrysn
Copy link
Member

@chrysn chrysn commented Apr 6, 2024

Contribution description

So far, ASAN has only been used in unit tests for native, but not in general tests.

ASAN would have spotted issues like #20549, and may spot security issues as well -- but only wen turned on.

There is a trade-off: Enabling ASAN pulls in the NATIVE_MEMORY define that disables TLSF. I think it's worth it: It's more likely we make bugs anywhere in the tested code base that are spotted this way than to overlook TLSF bugs that only occur on native (because it's still tested on other boards), and memory properties are different on native already anyway.

(FWIW I'd advocate enabling ASAN for developers all the time, but let's take it one step at a time.)

Testing procedure

  • Look at what CI does.

chrysn added 2 commits April 6, 2024 12:06
This verifies that unit tests still run under ASAN (they currently do
but are special-cased)
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Apr 6, 2024
@chrysn chrysn added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: tests Area: tests and testing framework and removed Area: tests Area: tests and testing framework labels Apr 6, 2024
@riot-ci
Copy link

riot-ci commented Apr 6, 2024

Murdock results

FAILED

4746811 fixup!

Success Failures Total Runtime
76 0 9072 01m:11s

Artifacts

@chrysn
Copy link
Member Author

chrysn commented Apr 6, 2024

This fails before the Do-Not-Merge should-really-fail commit on top, first time in tests_rtc, but I don't know enough about native's signal handling to make anything of it -- @fjmolinas, @OlegHahm, any ideas?

copy of the linked test failure
make: Entering directory '/tmp/dwq.0.23625481922180303/3ceeccac73369c4555874a7f14ea1734/tests/periph/rtc'
r
/tmp/dwq.0.23625481922180303/3ceeccac73369c4555874a7f14ea1734/build/tests_rtc.elf  tap0 
RIOT native interrupts/signals initialized.
RIOT native board initialized.
RIOT native hardware initialization complete.

Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: buildtest)

RIOT RTC low-level driver test
This test will display 'Alarm!' every 2 seconds for 4 times
Native RTC initialized.
Clock value is now   2024-04-06 10:12:19.871
  Setting clock to   2020-02-28 23:59:57
Clock value is now   2020-02-28 23:59:57.872
  Setting alarm to   2020-02-28 23:59:59
   Alarm is set to   2020-02-28 23:59:59
  Alarm cleared at   2020-02-28 23:59:57.872
==3281131==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
       No alarm at   2020-02-28 23:59:59.873
  Setting alarm to   2020-02-28 23:59:61

==3281131==WARNING: ASan is ignoring requested __asan_handle_no_return: stack type: default top: 0xffed7000; bottom 0x0806c000; size: 0xf7e6b000 (-135876608)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
/tmp/dwq.0.23625481922180303/3ceeccac73369c4555874a7f14ea1734/build/tests_rtc.elf: _native_popsig: real_read: Bad file descriptor
make[1]: *** [/tmp/dwq.0.23625481922180303/3ceeccac73369c4555874a7f14ea1734/Makefile.include:880: cleanterm] Error 1
Unexpected end of file in expect script at "child.expect_exact('Alarm!')" (tests/periph/rtc/tests/01-run.py:44)

make: Leaving directory '/tmp/dwq.0.23625481922180303/3ceeccac73369c4555874a7f14ea1734/tests/periph/rtc'
make: *** [/tmp/dwq.0.23625481922180303/3ceeccac73369c4555874a7f14ea1734/makefiles/tests/tests.inc.mk:26: test] Error 1

@OlegHahm
Copy link
Member

OlegHahm commented Apr 6, 2024

Hm, at least test_rtc is not failing on the mentioned commit on my machine (clang version 17.0.6). How can I trigger all tests at once?

tests/unittests/tests-asan_canary/tests-asan.c Outdated Show resolved Hide resolved
/**
* @{
*
* @file
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @file
* @file
* @brief Provide a true-positive (failing) test case for ASAN

Copy link
Member Author

Choose a reason for hiding this comment

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

This is only the "DO NOT MERGE" that will be removed once verified -- that commit will be removed again because AFAICT we don't have infrastructure for required-to-fail-to-build tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Bold words for someone working that late on the weekend ;)

@Teufelchen1
Copy link
Contributor

(FWIW I'd advocate enabling ASAN for developers all the time, but let's take it one step at a time.)

Yes please!

@Teufelchen1
Copy link
Contributor

Hm, at least test_rtc is not failing on the mentioned commit on my machine [..]

Not reproducible for me either.

Co-authored-by: Teufelchen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants