On Mon, Jan 24, 2022 at 9:21 AM Dumitru Ceara <[email protected]> wrote:
>
> Note: There still is an UB instance when using SSE4.2 as reported here:
> https://mail.openvswitch.org/pipermail/ovs-dev/2022-January/390904.html
>
> Acked-by: Aaron Conole <[email protected]>
> Signed-off-by: Dumitru Ceara <[email protected]>
> ---
> v3:
> - Fix typo in variable name.
> - Added SSE4.2 UB note to commit log.
> ---
>  .ci/linux-build.sh                   |    6 ++++++
>  .github/workflows/build-and-test.yml |    5 +++++
>  configure.ac                         |    1 +
>  tests/atlocal.in                     |   16 ++++++++++++++++
>  tests/automake.mk                    |    1 +
>  tests/daemon.at                      |    8 ++++++++
>  tests/ovs-macros.at                  |    5 +++++
>  tests/ovsdb-server.at                |   16 ++++++++++++++++
>  8 files changed, 58 insertions(+)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index 6cd38ff3efb5..afd4379e6923 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -250,6 +250,12 @@ if [ "$ASAN" ]; then
>      CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${CFLAGS_ASAN}"
>  fi
>
> +if [ "$UBSAN" ]; then
> +    # Use the default options configured in tests/atlocal.in, in 
> UBSAN_OPTIONS.
> +    CFLAGS_UBSAN="-O1 -fno-omit-frame-pointer -fno-common 
> -fsanitize=undefined"
> +    CFLAGS_FOR_OVS="${CFLAGS_FOR_OVS} ${CFLAGS_UBSAN}"
> +fi
> +
>  save_OPTS="${OPTS} $*"
>  OPTS="${EXTRA_OPTS} ${save_OPTS}"

Sanitizer flags usually placed for compile and link. So they usually
go in CFLAGS and LDFLAGS. You will be Ok with putting them in CFLAGS
only if you use GNU Make's default:

$ make -p | grep '^LINK\.c'
LINK.c = $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH)

But you will need to add them to LDFLAGS if you invoke LD directly.

You might also want to add -fno-sanitize=integer-divide-by-zero
-fno-sanitize=float-divide-by-zero. Divide-by-0's are usually not
needed due to IEEE-754, and if left in usually cause more trouble then
they are worth.

> diff --git a/.github/workflows/build-and-test.yml 
> b/.github/workflows/build-and-test.yml
> index eac3504e48f8..9e3583781baa 100644
> --- a/.github/workflows/build-and-test.yml
> +++ b/.github/workflows/build-and-test.yml
> @@ -13,6 +13,7 @@ jobs:
>          linux-headers-$(uname -r) build-essential fakeroot devscripts equivs
>        AFXDP:       ${{ matrix.afxdp }}
>        ASAN:        ${{ matrix.asan }}
> +      UBSAN:       ${{ matrix.ubsan }}
>        CC:          ${{ matrix.compiler }}
>        DEB_PACKAGE: ${{ matrix.deb_package }}
>        DPDK:        ${{ matrix.dpdk }}
> @@ -44,6 +45,10 @@ jobs:
>              testsuite:    test
>              kernel:       3.16
>              asan:         asan
> +          - compiler:     clang
> +            testsuite:    test
> +            kernel:       3.16
> +            ubsan:        ubsan
>
>            - compiler:     gcc
>              testsuite:    test
> diff --git a/configure.ac b/configure.ac
> index 298ea85aba22..198653a60528 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -197,6 +197,7 @@ OVS_CHECK_LINUX_SCTP_CT
>  OVS_CHECK_LINUX_VIRTIO_TYPES
>  OVS_CHECK_DPDK
>  OVS_CHECK_PRAGMA_MESSAGE
> +AC_SUBST([CFLAGS])
>  AC_SUBST([OVS_CFLAGS])
>  AC_SUBST([OVS_LDFLAGS])
>
> diff --git a/tests/atlocal.in b/tests/atlocal.in
> index a0ad239ecef2..142ea2090bc8 100644
> --- a/tests/atlocal.in
> +++ b/tests/atlocal.in
> @@ -4,6 +4,7 @@ OPENSSL_SUPPORTS_SNI='@OPENSSL_SUPPORTS_SNI@'
>  HAVE_UNBOUND='@HAVE_UNBOUND@'
>  EGREP='@EGREP@'
>  PYTHON3='@PYTHON3@'
> +CFLAGS='@CFLAGS@'
>
>  # PYTHONCOERCECLOCALE=0 disables the Unicode compatibility warning on
>  # stderr that breaks almost any Python3 test (PEP 0538)
> @@ -197,6 +198,16 @@ else
>      DIFF_SUPPORTS_NORMAL_FORMAT=no
>  fi
>
> +# Check whether UB Sanitizer is being used.
> +case "$CFLAGS" in
> +*fsanitize=undefined*)
> +    TESTS_WITH_UBSAN=yes
> +    ;;
> +*)
> +    TESTS_WITH_UBSAN=no
> +    ;;
> +esac
> +
>  # Turn off proxies.
>  unset http_proxy
>  unset https_proxy
> @@ -222,3 +233,8 @@ export OVS_CTL_TIMEOUT
>  # matter break everything.
>  ASAN_OPTIONS=detect_leaks=0:abort_on_error=true:log_path=asan:$ASAN_OPTIONS
>  export ASAN_OPTIONS
> +
> +# Add some default flags for UndefinedBehaviorSanitizer, if it was used
> +# for the build.
> +UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=true:log_path=ubsan:$UBSAN_OPTIONS
> +export UBSAN_OPTIONS
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 8a9151f81b86..a526be84b493 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -213,6 +213,7 @@ check-local:
>         set $(SHELL) '$(TESTSUITE)' -C tests AUTOTEST_PATH=$(AUTOTEST_PATH); \
>         "$$@" $(TESTSUITEFLAGS) || \
>         (test -z "$$(find $(TESTSUITE_DIR) -name 'asan.*')" && \
> +        test -z "$$(find $(TESTSUITE_DIR) -name 'ubsan.*')" && \
>          test X'$(RECHECK)' = Xyes && "$$@" --recheck)
>
>  # Python Coverage support.
> diff --git a/tests/daemon.at b/tests/daemon.at
> index 39d9aa391e9b..d7981f9d23a6 100644
> --- a/tests/daemon.at
> +++ b/tests/daemon.at
> @@ -81,6 +81,10 @@ AT_SKIP_IF([test "$IS_WIN32" = "yes"])
>  # This test intentionally causes SIGSEGV, so make Address Sanitizer ignore 
> it.
>  ASAN_OPTIONS=$ASAN_OPTIONS:handle_segv=0; export ASAN_OPTIONS
>
> +# Skip it if UB Sanitizer is being used.  There's no way to disable the
> +# SEGV check at runtime.
> +AT_SKIP_IF([test $TESTS_WITH_UBSAN = yes])
> +
>  # Start the daemon and wait for the pidfile to get created.
>  on_exit 'kill $(cat *.pid)'
>  AT_CHECK([ovsdb-server --monitor --pidfile --no-db 2>/dev/null & echo $!],
> @@ -149,6 +153,10 @@ AT_SKIP_IF([test "$IS_WIN32" = "yes"])
>  # This test intentionally causes SIGSEGV, so make Address Sanitizer ignore 
> it.
>  ASAN_OPTIONS=$ASAN_OPTIONS:handle_segv=0; export ASAN_OPTIONS
>
> +# Skip it if UB Sanitizer is being used.  There's no way to disable the
> +# SEGV check at runtime.
> +AT_SKIP_IF([test $TESTS_WITH_UBSAN = yes])
> +
>  on_exit 'kill $(cat *.pid)'
>
>  # Start the daemon and make sure that the pidfile exists immediately.
> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> index 66545da5728f..7899583f3c21 100644
> --- a/tests/ovs-macros.at
> +++ b/tests/ovs-macros.at
> @@ -216,6 +216,11 @@ ovs_cleanup() {
>          cat asan.*
>          AT_FAIL_IF([:])
>      fi
> +    if test "$(echo ubsan.*)" != 'ubsan.*'; then
> +        echo "Undefined Behavior Sanitizer reported errors in:" ubsan.*
> +        cat ubsan.*
> +        AT_FAIL_IF([:])
> +    fi
>  }
>
>  ovs_wait () {
> diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
> index 2b742f78b079..1e00be60684b 100644
> --- a/tests/ovsdb-server.at
> +++ b/tests/ovsdb-server.at
> @@ -292,6 +292,10 @@ AT_SKIP_IF([test "$IS_WIN32" = "yes"])
>  # This test intentionally causes SIGSEGV, so make Address Sanitizer ignore 
> it.
>  ASAN_OPTIONS=$ASAN_OPTIONS:handle_segv=0; export ASAN_OPTIONS
>
> +# Skip it if UB Sanitizer is being used.  There's no way to disable the
> +# SEGV check at runtime.
> +AT_SKIP_IF([test $TESTS_WITH_UBSAN = yes])
> +
>  # Start ovsdb-server, initially with one db.
>  ordinal_schema > schema
>  AT_CHECK([ovsdb-tool create db1 schema], [0], [ignore], [ignore])
> @@ -327,6 +331,10 @@ AT_SKIP_IF([test "$IS_WIN32" = "yes"])
>  # This test intentionally causes SIGSEGV, so make Address Sanitizer ignore 
> it.
>  ASAN_OPTIONS=$ASAN_OPTIONS:handle_segv=0; export ASAN_OPTIONS
>
> +# Skip it if UB Sanitizer is being used.  There's no way to disable the
> +# SEGV check at runtime.
> +AT_SKIP_IF([test $TESTS_WITH_UBSAN = yes])
> +
>  # Start ovsdb-server, initially with one db.
>  ordinal_schema > schema
>  AT_CHECK([ovsdb-tool create db1 schema], [0], [ignore], [ignore])
> @@ -477,6 +485,10 @@ AT_SKIP_IF([test "$IS_WIN32" = "yes"])
>  # This test intentionally causes SIGSEGV, so make Address Sanitizer ignore 
> it.
>  ASAN_OPTIONS=$ASAN_OPTIONS:handle_segv=0; export ASAN_OPTIONS
>
> +# Skip it if UB Sanitizer is being used.  There's no way to disable the
> +# SEGV check at runtime.
> +AT_SKIP_IF([test $TESTS_WITH_UBSAN = yes])
> +
>  # Start ovsdb-server, initially with no remotes.
>  ordinal_schema > schema
>  AT_CHECK([ovsdb-tool create db schema], [0], [ignore], [ignore])
> @@ -512,6 +524,10 @@ AT_SKIP_IF([test "$IS_WIN32" = "yes"])
>  # This test intentionally causes SIGSEGV, so make Address Sanitizer ignore 
> it.
>  ASAN_OPTIONS=$ASAN_OPTIONS:handle_segv=0; export ASAN_OPTIONS
>
> +# Skip it if UB Sanitizer is being used.  There's no way to disable the
> +# SEGV check at runtime.
> +AT_SKIP_IF([test $TESTS_WITH_UBSAN = yes])
> +
>  # Start ovsdb-server, initially with no remotes.
>  ordinal_schema > schema
>  AT_CHECK([ovsdb-tool create db schema], [0], [ignore], [ignore])

Jeff
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to