On Fri, 2021-05-07 at 08:36 +0200, Erik Skultety wrote:
On Thu, May 06, 2021 at 05:34:55PM +0200, Peter Krempa wrote:
On Thu, May 06, 2021 at 17:08:38 +0200, Tim Wiederhake wrote:
meson supports the following sanitizers: "address" (e.g. out-of- bounds memory access, use-after-free, etc.), "thread" (data races), "undefined" (e.g. signed integer overflow), and "memory" (use of uninitialized memory). Note that not all sanitizers are supported by all compilers, and that more sanitizers exist.
Not all sanitizers can be enabled at the same time, but "address" and "undefined" can. Both thread and memory sanitizers require an instrumented build of all dependencies, including libc.
gcc and clang use different implementations of these sanitizers and have proven to find different issues. Create CI jobs for both.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- .gitlab-ci.yml | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 89f618e678..4de4c30d7f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -73,6 +73,26 @@ stages: rpmbuild --nodeps -ta build/meson-dist/libvirt-*.tar.xz; fi
+.sanitizer_build_job: + stage: builds + image: $CI_REGISTRY_IMAGE/ci-ubuntu-2004:latest + needs: + - x64-ubuntu-2004-container + rules: + - if: "$TEMPORARILY_DISABLED" + allow_failure: true
Does this mean that if $TEMPORARILY_DISABLED is not passed then the sanitizer error causes a pipeline failure?
Yes, that's exactly what would happen.
If yes then I'd like to know how we are going to waive false- positives as modifying the code is the wrong action in such case.
I agree that sanitizers should probably not cause hard failures of the pipeline.
AddressSanitizer finds issues like leaks, use-after-free, and double- free of memory. As there are currently none of these issues found, any new finding would be a regression which in my opinion does warrant a build failure. The sanitizer is not known to produce false positives, but should that situation arise in the future, we can use of suppression lists, see https://clang.llvm.org/docs/AddressSanitizer.html#issue-suppression.
On the other hand that's exactly what would happen with coverity which is also setup as a hard failure, so we kinda have a precedent. The question you need to answer for yourself is - if we set both coverity and sanitizer jobs to soft failures by default, how likely it is that someone is going to look at those failures and fix them in a timely manner? That's why the TEMPORARILY_DISABLED variable exists in the first place, if a failure occurs, someone has to look at the issue, determine that it's a false positive and unless we're immediately able to figure out how to alleviate the issue (e.g. adding a rule to coverity to ignore a certain false positive), we convert the job to a soft failing one. Once we addressed the problem in the sanitizer, we can again enable the job fully.
Erik
I agree. $TEMPORARILY_DISABLED is a rather big hammer though to disable sanitation entirely; I do not see a need for this but left it in for consistency with how other build jobs can be made non-required. Cheers, Tim