On 12/22/23 12:12, Eelco Chaudron wrote: > This patch combines the existing ubsan and asan GitHub actions > tests into one.
Not sure how important it is, but worth mentioning that this change reduces the test coverage. Some tests are skipped for ubsan specifically, but not for asan. IIRC, these are tests for the process monitoring and backtraces that trigger SIGSEGV; ubsan can't handle this signal, while asan can. At least, this should be added into commit message. > > Signed-off-by: Eelco Chaudron <[email protected]> > --- > .ci/linux-build.sh | 4 ++-- > .github/workflows/build-and-test.yml | 8 ++------ > 2 files changed, 4 insertions(+), 8 deletions(-) > > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh > index 90581c10b..c780dac01 100755 > --- a/.ci/linux-build.sh > +++ b/.ci/linux-build.sh > @@ -102,14 +102,14 @@ else > CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${SPARSE_FLAGS}" > fi > > -if [ "$ASAN" ]; then > +if [[ $SANITIZERS == *asan* ]]; then > # This will override default option configured in tests/atlocal.in. > export ASAN_OPTIONS='detect_leaks=1' > CFLAGS_ASAN="-fno-omit-frame-pointer -fno-common -fsanitize=address" > CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${CFLAGS_ASAN}" > fi > > -if [ "$UBSAN" ]; then > +if [[ $SANITIZERS == *ubsan* ]]; then > # Use the default options configured in tests/atlocal.in, in > UBSAN_OPTIONS. > CFLAGS_UBSAN="-fno-omit-frame-pointer -fno-common -fsanitize=undefined" > CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${CFLAGS_UBSAN}" Do we need two separate cases now? Also, we don't need to add -fno-omit-frame-pointer and -fno-common twice. It is also a little annoying that ASAN_OPTIONS and UBSAN_OPTIONS are not really separate sets of options, they do overlap. For example, if the log_path is defined in both, the one from UBSAN_OPTIONS will actually be used. So all the log files will be ubsan.* even if they come from asan. Maybe rename these files as well, similarly to what OVN project did? I'm not sure if there are other option conflicts. > diff --git a/.github/workflows/build-and-test.yml > b/.github/workflows/build-and-test.yml > index 710757693..b901e6d57 100644 > --- a/.github/workflows/build-and-test.yml > +++ b/.github/workflows/build-and-test.yml > @@ -78,14 +78,13 @@ jobs: > automake libtool gcc bc libjemalloc2 libjemalloc-dev libssl-dev \ > llvm-dev libnuma-dev libpcap-dev selinux-policy-dev libbpf-dev \ > lftp libreswan > - ASAN: ${{ matrix.asan }} > - UBSAN: ${{ matrix.ubsan }} > CC: ${{ matrix.compiler }} > DPDK: ${{ matrix.dpdk }} > DPDK_SHARED: ${{ matrix.dpdk_shared }} > LIBS: ${{ matrix.libs }} > M32: ${{ matrix.m32 }} > OPTS: ${{ matrix.opts }} > + SANITIZERS: ${{ matrix.sanitizers }} > STD: ${{ matrix.std }} > TESTSUITE: ${{ matrix.testsuite }} > TEST_RANGE: ${{ matrix.test_range }} > @@ -111,11 +110,8 @@ jobs: > - compiler: gcc > testsuite: test > - compiler: clang > + sanitizers: asan ubsan If we define this variable as 'address,undefined', we could plug this variable directly into the '-fsanitize='. > testsuite: test > - asan: asan > - - compiler: clang > - testsuite: test > - ubsan: ubsan > > - compiler: gcc > testsuite: test > Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
