[ovs-dev] [PATCH ovn 2/2] ci: Enable AddressSanitizer in Linux CI runs.
Ilya Maximets
i.maximets at ovn.org
Wed Dec 16 10:24:47 UTC 2020
On 12/16/20 9:11 AM, Dumitru Ceara wrote:
> To make sure no memory leaks or invalid accesses reported by
> AddressSanitizer are missed, also skip rechecking tests if
> AddressSanitizer reports are present in the test run directory.
Hi, Dumitru.
Thanks for working on this! I think that it's very useful as we just
fixed all the leaks detected by the testsuite. Some comments inline.
Best regards, Ilya Maximets.
>
> Note: This doubles GitHub Actions CI time from ~9 minutes (before the
> change) to ~18 minutes (after the change).
It might make sense to only have one job that runs under address sanitizer.
That should save some CI time. It might be a single clang build with tests
enabled (I have no experience with gcc+sanitizer).
>
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> ---
> .ci/linux-build.sh | 2 +-
> tests/automake.mk | 6 +++++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index 0e9f87f..6943b01 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -3,7 +3,7 @@
> set -o errexit
> set -x
>
> -CFLAGS=""
> +CFLAGS="-fsanitize=address -fno-omit-frame-pointer -fno-common"
This affects both OVS and OVN. It might be better to only build OVN
with sanitizer enabled to avoid random issues in OVS code.
This should also speed tests up a bit, I guess.
> SPARSE_FLAGS=""
> EXTRA_OPTS="--enable-Werror"
>
> diff --git a/tests/automake.mk b/tests/automake.mk
> index c5c286e..34464c7 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -52,6 +52,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
> DISTCLEANFILES += tests/atconfig tests/atlocal
> @@ -61,8 +62,11 @@ AUTOTEST_PATH = $(ovs_builddir)/utilities:$(ovs_builddir)/vswitchd:$(ovs_builddi
> export ovs_srcdir
>
> check-local:
> + find $(TESTSUITE_DIR) -name 'asan.*' | xargs -n1 rm -f
This should not be necessary to remove asan reports, because autotest removes
directories before the test. As we discussed off-list, this was to avoid
skipping the recheck if started with e.g., 'TESTSUITEFLAGS="43" RECHECK="yes',
while there is an asan report for some other test, but it's not very frequent
scenario, and I don't think that we will want to loose asan reports for other
tests in this case anyway.
> 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/.
>
More information about the dev
mailing list