FreeRTOS / FreeRTOS-Plus-TCP

FreeRTOS-Plus-TCP library repository. +TCP files only. Submoduled into https://github.com/FreeRTOS/FreeRTOS and various other repos.
MIT License
125 stars 149 forks source link

Let's fix the tests enough to run with AddressSanitizer and UB Sanitizer and enable those in CI #1151

Closed anordal closed 3 weeks ago

anordal commented 1 month ago

This PR adds an option to build with sanitizers, fixes all AddressSanitizer and UB-Sanitizer errors (because these sanitizers can be run together) and enforces them in CI.

Motivation: The tests were crashing when compiled with Gcc 13. With these fixes, they no longer do.

I think it would be nice for future development to run the tests with sanitizers in CI to prevent the sort of bugs I've now fixed. They were not few, which should contribute to reduce the amount of flakiness (especially crashiness) in the tests.

I have focused on what's necessary to enable these sanitizers, but there is more. Particularly uninitialized variables. The next I would do is ban it with -Werror=maybe-uninitialized, but that's going to reveal the rest of that iceberg.

Test Matrix

Checklist:

Related Issue

None.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

tony-josi-aws commented 1 month ago

@anordal Thanks for taking time to contribute to FreeRTOS+TCP. Enabling address and undefined behavior sanitizers in the CI check would help improve the code quality.

tony-josi-aws commented 4 weeks ago

@anordal can you help fix the merge conflict with the test/unit-test/FreeRTOS_TCP_IP/FreeRTOS_TCP_IP_utest.c?

anordal commented 3 weeks ago

Yes. I see you fixed a use after free (ba6ba81f). I'll push a rebase.