| Message ID | 20210122190402.957407-1-i.maximets@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] github: Run clang test with AddressSanitizer enabled. | expand |
On 1/22/21 8:04 PM, Ilya Maximets wrote: > This commit is based on a similar one from OVN by Dumitru Ceara: > a429b24f7bf5 ("ci: Enable AddressSanitizer in Linux clang CI test runs.") > > It's useful to run testsuite with address sanitizer enabled to catch > memory leaks and invalid memory accesses. Skipping re-check if > AddressSanitizer reports are present in the test run directory to > not lose them. > > Right now OVS has no memory leaks detected on a testsuite run with -O1. > With -O2 there are few false-positive leak reports in test-ovsdb > application, so not using this optimization level for now. For the > same reason not enabling leak detection by default for everyone. > Enabled only in CI. > > AddressSanitizer increases execution time for this job from ~12 to ~16 > minutes, but it looks like a reasonable sacrifice. > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- Looks good to me, thanks! Acked-by: Dumitru Ceara <dceara@redhat.com>
On 1/27/21 11:42 AM, Dumitru Ceara wrote: > On 1/22/21 8:04 PM, Ilya Maximets wrote: >> This commit is based on a similar one from OVN by Dumitru Ceara: >> a429b24f7bf5 ("ci: Enable AddressSanitizer in Linux clang CI test runs.") >> >> It's useful to run testsuite with address sanitizer enabled to catch >> memory leaks and invalid memory accesses. Skipping re-check if >> AddressSanitizer reports are present in the test run directory to >> not lose them. >> >> Right now OVS has no memory leaks detected on a testsuite run with -O1. >> With -O2 there are few false-positive leak reports in test-ovsdb >> application, so not using this optimization level for now. For the >> same reason not enabling leak detection by default for everyone. >> Enabled only in CI. >> >> AddressSanitizer increases execution time for this job from ~12 to ~16 >> minutes, but it looks like a reasonable sacrifice. >> >> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> >> --- > > Looks good to me, thanks! > > Acked-by: Dumitru Ceara <dceara@redhat.com> > Thanks, Dumitru. I'll defer applying of this patch for now. The 'bfd decay' test is unstable while running under AddressSanitizer. It was always too time-sensitive. We need to look at this test and fix it or disable for this CI job. Best regards, Ilya Maximets.
On 1/27/21 12:03 PM, Ilya Maximets wrote: > On 1/27/21 11:42 AM, Dumitru Ceara wrote: >> On 1/22/21 8:04 PM, Ilya Maximets wrote: >>> This commit is based on a similar one from OVN by Dumitru Ceara: >>> a429b24f7bf5 ("ci: Enable AddressSanitizer in Linux clang CI test runs.") >>> >>> It's useful to run testsuite with address sanitizer enabled to catch >>> memory leaks and invalid memory accesses. Skipping re-check if >>> AddressSanitizer reports are present in the test run directory to >>> not lose them. >>> >>> Right now OVS has no memory leaks detected on a testsuite run with -O1. >>> With -O2 there are few false-positive leak reports in test-ovsdb >>> application, so not using this optimization level for now. For the >>> same reason not enabling leak detection by default for everyone. >>> Enabled only in CI. >>> >>> AddressSanitizer increases execution time for this job from ~12 to ~16 >>> minutes, but it looks like a reasonable sacrifice. >>> >>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> >>> --- >> >> Looks good to me, thanks! >> >> Acked-by: Dumitru Ceara <dceara@redhat.com> >> > > Thanks, Dumitru. > > I'll defer applying of this patch for now. The 'bfd decay' test is unstable > while running under AddressSanitizer. It was always too time-sensitive. > We need to look at this test and fix it or disable for this CI job. This test didn't fail for me for a last couple of weeks, so maybe the problem is not that frequent. Taking that into account, I think it's OK to apply this patch. Applied to master. Best regards, Ilya Maximets.
diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index 3e5136fd4..581a8888b 100755 --- a/.ci/linux-build.sh +++ b/.ci/linux-build.sh @@ -226,6 +226,15 @@ elif [ "$TRAVIS_ARCH" != "aarch64" ]; then CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${SPARSE_FLAGS}" fi +if [ "$ASAN" ]; then + # This will override default option configured in tests/atlocal.in. + export ASAN_OPTIONS='detect_leaks=1' + # -O2 generates few false-positive memory leak reports in test-ovsdb + # application, so lowering optimizations to -O1 here. + CLFAGS_ASAN="-O1 -fno-omit-frame-pointer -fno-common -fsanitize=address" + CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${CLFAGS_ASAN}" +fi + save_OPTS="${OPTS} $*" OPTS="${EXTRA_OPTS} ${save_OPTS}" diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index b29c300c5..e24970505 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -14,6 +14,7 @@ jobs: deb_dependencies: | linux-headers-$(uname -r) build-essential fakeroot devscripts equivs AFXDP: ${{ matrix.afxdp }} + ASAN: ${{ matrix.asan }} CC: ${{ matrix.compiler }} DEB_PACKAGE: ${{ matrix.deb_package }} DPDK: ${{ matrix.dpdk }} @@ -44,6 +45,7 @@ jobs: - compiler: clang testsuite: test kernel: 3.16 + asan: asan - compiler: gcc testsuite: test diff --git a/tests/automake.mk b/tests/automake.mk index 677b99a6b..dfec2ea10 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -189,6 +189,7 @@ check_SCRIPTS += tests/atlocal TESTSUITE = $(srcdir)/tests/testsuite TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch +TESTSUITE_DIR = $(abs_top_builddir)/tests/testsuite.dir SYSTEM_KMOD_TESTSUITE = $(srcdir)/tests/system-kmod-testsuite SYSTEM_USERSPACE_TESTSUITE = $(srcdir)/tests/system-userspace-testsuite SYSTEM_TSO_TESTSUITE = $(srcdir)/tests/system-tso-testsuite @@ -202,7 +203,9 @@ AUTOTEST_PATH = utilities:vswitchd:ovsdb:vtep:tests:$(PTHREAD_WIN32_DIR_DLL):$(S check-local: set $(SHELL) '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH); \ - "$$@" $(TESTSUITEFLAGS) || (test X'$(RECHECK)' = Xyes && "$$@" --recheck) + "$$@" $(TESTSUITEFLAGS) || \ + (test -z "$$(find $(TESTSUITE_DIR) -name 'asan.*')" && \ + test X'$(RECHECK)' = Xyes && "$$@" --recheck) # Python Coverage support. # Requires coverage.py http://nedbatchelder.com/code/coverage/.
This commit is based on a similar one from OVN by Dumitru Ceara: a429b24f7bf5 ("ci: Enable AddressSanitizer in Linux clang CI test runs.") It's useful to run testsuite with address sanitizer enabled to catch memory leaks and invalid memory accesses. Skipping re-check if AddressSanitizer reports are present in the test run directory to not lose them. Right now OVS has no memory leaks detected on a testsuite run with -O1. With -O2 there are few false-positive leak reports in test-ovsdb application, so not using this optimization level for now. For the same reason not enabling leak detection by default for everyone. Enabled only in CI. AddressSanitizer increases execution time for this job from ~12 to ~16 minutes, but it looks like a reasonable sacrifice. Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- .ci/linux-build.sh | 9 +++++++++ .github/workflows/build-and-test.yml | 2 ++ tests/automake.mk | 5 ++++- 3 files changed, 15 insertions(+), 1 deletion(-)