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

CI: Enable sanitizers #565

Merged

Conversation

rettichschnidi
Copy link
Contributor

This should help keep us fairly save from memory corruption bugs - at least as far as we have unit tests for the code in question.

Would also be OK with spelling it out as long as we keep it consistent.

Signed-off-by: Reto Schneider <code@reto-schneider.ch>
Also, add curly braces around variables.

Signed-off-by: Reto Schneider <code@reto-schneider.ch>
@rettichschnidi rettichschnidi force-pushed the gardena/rs/improve-ci-2 branch 3 times, most recently from 6f5ab6f to b559d83 Compare March 30, 2021 01:09
@rettichschnidi rettichschnidi changed the title CI: Run binaries with sanitizers CI: Enable sanitizers Mar 30, 2021
@rettichschnidi
Copy link
Contributor Author

@mlasch Please review

Please note:
 - Thread sanitizer does not make too much sense on a single threaded
   code base, but who knows, what the future will bring?
 - Memory sanitizer is not (yet) used as only Clang supports it
 - Set CMake build type to RelWithDebInfo to allow pinpointing found
   leaks to the respective source code line, while also allowing to find
   bugs which only show a higher optimization levels.
 - Bumping CMake requirement to 3.13 for target_link_options().

This commit fixes also the one memory leak found while adding sanitizer
support.

Signed-off-by: Reto Schneider <code@reto-schneider.ch>
@rettichschnidi rettichschnidi force-pushed the gardena/rs/improve-ci-2 branch 3 times, most recently from 6792eba to 3970fa1 Compare March 30, 2021 16:18
Found by undefined behavior sanitizer:
 core/utils.c:284:56: runtime error: signed integer overflow:
 0 - -9223372036854775808 cannot be represented in type 'long'

Signed-off-by: Reto Schneider <code@reto-schneider.ch>
This change makes it easier to read the code. I need this to hunt down a
runtime error found by the undefined behavior sanitizer.

Signed-off-by: Reto Schneider <code@reto-schneider.ch>
Fix findings by undefined behavior sanitizer:
 runtime error: null pointer passed as argument 2, which is declared to
 never be null

Signed-off-by: Reto Schneider <code@reto-schneider.ch>
Error found by the leak sanitizer.

Signed-off-by: Reto Schneider <code@reto-schneider.ch>
@rettichschnidi
Copy link
Contributor Author

@mlasch Pls retry

Copy link
Contributor

@mlasch mlasch left a comment

Choose a reason for hiding this comment

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

Looks good, the CI script also runs nicely on my local machine.

@sbernard31 sbernard31 merged commit 7c23858 into eclipse-wakaama:master Mar 31, 2021
@rettichschnidi rettichschnidi deleted the gardena/rs/improve-ci-2 branch March 31, 2021 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants