| Message ID | 20201216125914.21757.90976.stgit@dceara.remote.csb |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ci: Enable AddressSanitizer for CI. | expand |
On 12/16/20 1:59 PM, 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. > > Note: This increases the GitHub Actions CI time for the clang test run > from ~9 minutes (before the change) to ~10 minutes (after the > change). > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- Thanks, Dumitru! This change looks good to me. I do not like that current OVN CI scripts are using OVS_CFLAGS variable, but that is a different story. For this patch: Acked-by: Ilya Maximets <i.maximets@ovn.org> > .ci/linux-build.sh | 11 +++++++++++ > .github/workflows/test.yml | 2 ++ > tests/automake.mk | 5 ++++- > 3 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh > index 0e9f87f..2a711a1 100755 > --- a/.ci/linux-build.sh > +++ b/.ci/linux-build.sh > @@ -4,6 +4,7 @@ set -o errexit > set -x > > CFLAGS="" > +OVN_CFLAGS="" > SPARSE_FLAGS="" > EXTRA_OPTS="--enable-Werror" > > @@ -19,6 +20,8 @@ function configure_ovs() > function configure_ovn() > { > configure_ovs $* > + > + export OVS_CFLAGS="${OVS_CFLAGS} ${OVN_CFLAGS}" > ./boot.sh && ./configure --with-ovs-source=$PWD/ovs_src $* || \ > { cat config.log; exit 1; } > } > @@ -26,6 +29,14 @@ function configure_ovn() > save_OPTS="${OPTS} $*" > OPTS="${EXTRA_OPTS} ${save_OPTS}" > > +# If AddressSanitizer is requested, enable it, but only for OVN, not for OVS. > +# However, disable some optimizations for OVS, to make AddressSanitizer > +# reports user friendly. > +if [ "$ASAN" ]; then > + CFLAGS="-fno-omit-frame-pointer -fno-common" > + OVN_CFLAGS="-fsanitize=address" > +fi > + > if [ "$CC" = "clang" ]; then > export OVS_CFLAGS="$CFLAGS -Wno-error=unused-command-line-argument" > elif [ "$M32" ]; then > diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml > index 7be75ca..0a8f50b 100644 > --- a/.github/workflows/test.yml > +++ b/.github/workflows/test.yml > @@ -16,6 +16,7 @@ jobs: > M32: ${{ matrix.m32 }} > OPTS: ${{ matrix.opts }} > TESTSUITE: ${{ matrix.testsuite }} > + ASAN: ${{ matrix.asan }} > > name: linux ${{ join(matrix.*, ' ') }} > runs-on: ubuntu-18.04 > @@ -33,6 +34,7 @@ jobs: > testsuite: test > - compiler: clang > testsuite: test > + asan: asan > > - compiler: gcc > testsuite: test > diff --git a/tests/automake.mk b/tests/automake.mk > index c5c286e..db934cb 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 > @@ -62,7 +63,9 @@ export ovs_srcdir > > 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/. >
On 12/16/20 1:12 PM, Ilya Maximets wrote: > On 12/16/20 1:59 PM, 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. >> >> Note: This increases the GitHub Actions CI time for the clang test run >> from ~9 minutes (before the change) to ~10 minutes (after the >> change). >> >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >> --- > > Thanks, Dumitru! > > This change looks good to me. I do not like that current OVN CI scripts > are using OVS_CFLAGS variable, but that is a different story. > > For this patch: > Acked-by: Ilya Maximets <i.maximets@ovn.org> I pushed this series to master. > >> .ci/linux-build.sh | 11 +++++++++++ >> .github/workflows/test.yml | 2 ++ >> tests/automake.mk | 5 ++++- >> 3 files changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh >> index 0e9f87f..2a711a1 100755 >> --- a/.ci/linux-build.sh >> +++ b/.ci/linux-build.sh >> @@ -4,6 +4,7 @@ set -o errexit >> set -x >> >> CFLAGS="" >> +OVN_CFLAGS="" >> SPARSE_FLAGS="" >> EXTRA_OPTS="--enable-Werror" >> >> @@ -19,6 +20,8 @@ function configure_ovs() >> function configure_ovn() >> { >> configure_ovs $* >> + >> + export OVS_CFLAGS="${OVS_CFLAGS} ${OVN_CFLAGS}" >> ./boot.sh && ./configure --with-ovs-source=$PWD/ovs_src $* || \ >> { cat config.log; exit 1; } >> } >> @@ -26,6 +29,14 @@ function configure_ovn() >> save_OPTS="${OPTS} $*" >> OPTS="${EXTRA_OPTS} ${save_OPTS}" >> >> +# If AddressSanitizer is requested, enable it, but only for OVN, not for OVS. >> +# However, disable some optimizations for OVS, to make AddressSanitizer >> +# reports user friendly. >> +if [ "$ASAN" ]; then >> + CFLAGS="-fno-omit-frame-pointer -fno-common" >> + OVN_CFLAGS="-fsanitize=address" >> +fi >> + >> if [ "$CC" = "clang" ]; then >> export OVS_CFLAGS="$CFLAGS -Wno-error=unused-command-line-argument" >> elif [ "$M32" ]; then >> diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml >> index 7be75ca..0a8f50b 100644 >> --- a/.github/workflows/test.yml >> +++ b/.github/workflows/test.yml >> @@ -16,6 +16,7 @@ jobs: >> M32: ${{ matrix.m32 }} >> OPTS: ${{ matrix.opts }} >> TESTSUITE: ${{ matrix.testsuite }} >> + ASAN: ${{ matrix.asan }} >> >> name: linux ${{ join(matrix.*, ' ') }} >> runs-on: ubuntu-18.04 >> @@ -33,6 +34,7 @@ jobs: >> testsuite: test >> - compiler: clang >> testsuite: test >> + asan: asan >> >> - compiler: gcc >> testsuite: test >> diff --git a/tests/automake.mk b/tests/automake.mk >> index c5c286e..db934cb 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 >> @@ -62,7 +63,9 @@ export ovs_srcdir >> >> 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/. >> >
On 12/16/20 7:29 PM, Mark Michelson wrote: > On 12/16/20 1:12 PM, Ilya Maximets wrote: >> On 12/16/20 1:59 PM, 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. >>> >>> Note: This increases the GitHub Actions CI time for the clang test run >>> from ~9 minutes (before the change) to ~10 minutes (after the >>> change). >>> >>> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >>> --- >> >> Thanks, Dumitru! >> >> This change looks good to me. I do not like that current OVN CI scripts >> are using OVS_CFLAGS variable, but that is a different story. >> >> For this patch: >> Acked-by: Ilya Maximets <i.maximets@ovn.org> > > I pushed this series to master. > Thanks!
diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index 0e9f87f..2a711a1 100755 --- a/.ci/linux-build.sh +++ b/.ci/linux-build.sh @@ -4,6 +4,7 @@ set -o errexit set -x CFLAGS="" +OVN_CFLAGS="" SPARSE_FLAGS="" EXTRA_OPTS="--enable-Werror" @@ -19,6 +20,8 @@ function configure_ovs() function configure_ovn() { configure_ovs $* + + export OVS_CFLAGS="${OVS_CFLAGS} ${OVN_CFLAGS}" ./boot.sh && ./configure --with-ovs-source=$PWD/ovs_src $* || \ { cat config.log; exit 1; } } @@ -26,6 +29,14 @@ function configure_ovn() save_OPTS="${OPTS} $*" OPTS="${EXTRA_OPTS} ${save_OPTS}" +# If AddressSanitizer is requested, enable it, but only for OVN, not for OVS. +# However, disable some optimizations for OVS, to make AddressSanitizer +# reports user friendly. +if [ "$ASAN" ]; then + CFLAGS="-fno-omit-frame-pointer -fno-common" + OVN_CFLAGS="-fsanitize=address" +fi + if [ "$CC" = "clang" ]; then export OVS_CFLAGS="$CFLAGS -Wno-error=unused-command-line-argument" elif [ "$M32" ]; then diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7be75ca..0a8f50b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,6 +16,7 @@ jobs: M32: ${{ matrix.m32 }} OPTS: ${{ matrix.opts }} TESTSUITE: ${{ matrix.testsuite }} + ASAN: ${{ matrix.asan }} name: linux ${{ join(matrix.*, ' ') }} runs-on: ubuntu-18.04 @@ -33,6 +34,7 @@ jobs: testsuite: test - compiler: clang testsuite: test + asan: asan - compiler: gcc testsuite: test diff --git a/tests/automake.mk b/tests/automake.mk index c5c286e..db934cb 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 @@ -62,7 +63,9 @@ export ovs_srcdir 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/.
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. Note: This increases the GitHub Actions CI time for the clang test run from ~9 minutes (before the change) to ~10 minutes (after the change). Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- .ci/linux-build.sh | 11 +++++++++++ .github/workflows/test.yml | 2 ++ tests/automake.mk | 5 ++++- 3 files changed, 17 insertions(+), 1 deletion(-)