Skip to content

Build: add sanitizer stages#1244

Merged
arogge merged 5 commits intomasterfrom
dev/pstorz/master/add-sanitizer-stage
Oct 4, 2022
Merged

Build: add sanitizer stages#1244
arogge merged 5 commits intomasterfrom
dev/pstorz/master/add-sanitizer-stage

Conversation

@pstorz
Copy link
Member

@pstorz pstorz commented Aug 29, 2022

This PR adds the option to run the bareos tests with sanitizers enabled ("-D ENABLE_SANITIZERS=yes") to immediately detect memory problems that the sanitzers can detect.

It also introduces the test label broken_with_enabled_sanitzers which marks tests that do not work with sanitizers enabled. These tests are skipped in the sanitizer stage.

It also fixes a memleak that was detected by the sanitizers stage in mntent_cache.cc

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Separate commit for this PR in the CHANGELOG.md, PR number referenced is same
  • Commit descriptions are understandable and well formatted
  • If backport: add original PR number and target branch at top of this file: Backport of PR#000 to bareos-2x

@pstorz pstorz marked this pull request as draft August 29, 2022 08:20
@pstorz pstorz force-pushed the dev/pstorz/master/add-sanitizer-stage branch from b71d6eb to ceba1ea Compare September 2, 2022 19:44
@pstorz pstorz changed the title Build: add sanitizer stage Build: add sanitizer stages Sep 7, 2022
@pstorz pstorz marked this pull request as ready for review September 7, 2022 11:40
@pstorz pstorz force-pushed the dev/pstorz/master/add-sanitizer-stage branch from f14774f to d47a638 Compare September 7, 2022 11:45
.matrix.yml Outdated
"F37":
TYPE: scripted
IMAGE: fedora37
BUILD_SCRIPT: CD/sanitize/build-sanitize.sh
Copy link
Member

Choose a reason for hiding this comment

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

we can move that to the Bareos repository, you just use "bareos/..." as a path here.
The script path is relative to Jenkins' $WORKSPACE and it always run with cwd set to the top of the cloned bareos repository.
So you can test and develop the script locally and it should just work in Jenkins.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@pstorz pstorz force-pushed the dev/pstorz/master/add-sanitizer-stage branch from 4428bdf to 8214c38 Compare September 19, 2022 13:58
@pstorz pstorz requested a review from arogge September 20, 2022 10:45
Copy link
Member Author

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Schaut doch gut aus!

@pstorz pstorz force-pushed the dev/pstorz/master/add-sanitizer-stage branch from 6546026 to bad4037 Compare September 30, 2022 14:52
pstorz and others added 4 commits October 4, 2022 10:29
The code before did not check for the return value of binary_insert(), so
that the failed inserts were not freed.
Signed-off-by: Bruno Friedmann <bruno.friedmann@bareos.com>
Add a stage with F37 building with a custom script, enabling sanitizers
and running tests.
Also cleanup the .matrix.yml a bit.

Co-Authored-By: Andreas Rogge <andreas.rogge@bareos.com>
@arogge arogge force-pushed the dev/pstorz/master/add-sanitizer-stage branch from bad4037 to b6fd36b Compare October 4, 2022 08:36
@arogge arogge merged commit 57c4c6a into master Oct 4, 2022
@arogge arogge deleted the dev/pstorz/master/add-sanitizer-stage branch October 4, 2022 10:11
alaaeddineelamri pushed a commit that referenced this pull request Dec 31, 2022
Build: add sanitizer stages
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