[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