Re: [PATCH v3 10/18] build: support Undefined Behavior Sanitizer

[Aaron Conole <aconole@redhat.com>]

David Marchand <david.marchand@redhat.com> writes:

> Enable UBSan in GHA.
> There are still a lot of issues so only run unit tests for a "mini"
> target.
>
> Building with debugoptimized forces -O2 and consumes too much memory
> with UBSan, prefer plain build (iow -O0) even though this hides a number
> of build issues.

It's probably okay for many of the issues because we have other builds
in github that should enable the code path analysis.  Maybe someday in
the future we can get this with UBSan as well.

> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>

> Changes since v2:
> - left the mini configuration alone and instead filtered
>   enabled component when running unit tests with UBSan,
>
> ---
>  .ci/linux-build.sh          | 38 +++++++++++++++++++++++++++++++++++--
>  .github/workflows/build.yml |  5 +++++
>  app/test/suites/meson.build |  3 +--
>  config/meson.build          | 18 +++++++++++++++++-
>  devtools/words-case.txt     |  1 +
>  5 files changed, 60 insertions(+), 5 deletions(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index e9272d3931..a24820c65b 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -44,6 +44,13 @@ catch_coredump() {
>      return 1
>  }
>  
> +catch_ubsan() {
> +    [ "$UBSAN" = "true" ] || return 0
> +    grep -q UndefinedBehaviorSanitizer $2 2>/dev/null || return 0
> +    grep -E "($1|UndefinedBehaviorSanitizer)" $2
> +    return 1
> +}
> +
>  check_traces() {
>      which babeltrace >/dev/null || return 0
>      for file in $(sudo find $HOME -name metadata); do
> @@ -100,7 +107,6 @@ fi
>  
>  OPTS="$OPTS -Dplatform=generic"
>  OPTS="$OPTS -Ddefault_library=$DEF_LIB"
> -OPTS="$OPTS -Dbuildtype=$buildtype"
>  if [ "$STDATOMIC" = "true" ]; then
>  	OPTS="$OPTS -Denable_stdatomic=true"
>  else
> @@ -117,13 +123,39 @@ else
>  fi
>  OPTS="$OPTS -Dlibdir=lib"
>  
> +buildtype=debugoptimized
> +sanitizer=
>  if [ "$ASAN" = "true" ]; then
> -    OPTS="$OPTS -Db_sanitize=address"
> +    sanitizer=${sanitizer:+$sanitizer,}address
> +fi
> +
> +if [ "$UBSAN" = "true" ]; then
> +    sanitizer=${sanitizer:+$sanitizer,}undefined
> +    if [ "$RUN_TESTS" = "true" ]; then
> +        # UBSan takes too much memory with -O2
> +        buildtype=plain
> +        # Only enable test binaries
> +        OPTS="$OPTS -Denable_apps=test,test-pmd"
> +        # Restrict drivers to a minimum set for now
> +        OPTS="$OPTS -Denable_drivers=net/null"
> +        # There are still known issues in various libraries, disable them for now
> +        OPTS="$OPTS -Ddisable_libs=*"
> +        # No examples are run
> +        OPTS="$OPTS -Dexamples="
> +        # The UBSan target takes a good amount of time and headers coverage is done in other
> +        # targets already.
> +        OPTS="$OPTS -Dcheck_includes=false"
> +    fi
> +fi
> +
> +if [ -n "$sanitizer" ]; then
> +    OPTS="$OPTS -Db_sanitize=$sanitizer"
>      if [ "${CC%%clang}" != "$CC" ]; then
>          OPTS="$OPTS -Db_lundef=false"
>      fi
>  fi
>  
> +OPTS="$OPTS -Dbuildtype=$buildtype"
>  OPTS="$OPTS -Dwerror=true"
>  
>  if [ -d build ]; then
> @@ -141,6 +173,7 @@ if [ -z "$cross_file" ]; then
>      configure_coredump
>      devtools/test-null.sh || failed="true"
>      catch_coredump
> +    catch_ubsan DPDK:fast-tests build/meson-logs/testlog.txt
>      check_traces
>      [ "$failed" != "true" ]
>  fi
> @@ -182,6 +215,7 @@ if [ "$RUN_TESTS" = "true" ]; then
>      configure_coredump
>      sudo meson test -C build --suite fast-tests -t 3 || failed="true"
>      catch_coredump
> +    catch_ubsan DPDK:fast-tests build/meson-logs/testlog.txt
>      check_traces
>      [ "$failed" != "true" ]
>  fi
> diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
> index 6c4bc664d0..0c8ca23bbb 100644
> --- a/.github/workflows/build.yml
> +++ b/.github/workflows/build.yml
> @@ -54,6 +54,7 @@ jobs:
>        RISCV64: ${{ matrix.config.cross == 'riscv64' }}
>        RUN_TESTS: ${{ contains(matrix.config.checks, 'tests') }}
>        STDATOMIC: ${{ contains(matrix.config.checks, 'stdatomic') }}
> +      UBSAN: ${{ contains(matrix.config.checks, 'ubsan') }}
>  
>      strategy:
>        fail-fast: false
> @@ -62,6 +63,10 @@ jobs:
>            - os: ubuntu-22.04
>              compiler: gcc
>              mini: mini
> +          - os: ubuntu-22.04
> +            compiler: clang
> +            library: shared
> +            checks: tests+ubsan
>            - os: ubuntu-22.04
>              compiler: gcc
>              checks: stdatomic
> diff --git a/app/test/suites/meson.build b/app/test/suites/meson.build
> index e482373330..0cba63ee12 100644
> --- a/app/test/suites/meson.build
> +++ b/app/test/suites/meson.build
> @@ -72,8 +72,7 @@ foreach suite:test_suites
>              elif not has_hugepage
>                  continue  #skip this tests
>              endif
> -            if not asan and (get_option('b_sanitize') == 'address'
> -                    or get_option('b_sanitize') == 'address,undefined')
> +            if not asan and get_option('b_sanitize').contains('address')
>                  continue  # skip this test
>              endif
>  
> diff --git a/config/meson.build b/config/meson.build
> index f31fef216c..b8a8511a7f 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -499,7 +499,7 @@ if get_option('b_lto')
>      endif
>  endif
>  
> -if get_option('b_sanitize') == 'address' or get_option('b_sanitize') == 'address,undefined'
> +if get_option('b_sanitize').contains('address')
>      if is_windows
>          error('ASan is not supported on windows')
>      endif
> @@ -519,6 +519,22 @@ if get_option('b_sanitize') == 'address' or get_option('b_sanitize') == 'address
>      endif
>  endif
>  
> +if get_option('b_sanitize').contains('undefined')
> +    if is_windows
> +        error('UBSan is not supported on windows')
> +    endif
> +
> +    if cc.get_id() == 'gcc'
> +        ubsan_dep = cc.find_library('ubsan', required: true)
> +        if (not cc.links('int main(int argc, char *argv[]) { return 0; }',
> +                dependencies: ubsan_dep))
> +            error('broken dependency, "libubsan"')
> +        endif
> +        add_project_link_arguments('-lubsan', language: 'c')
> +        dpdk_extra_ldflags += '-lubsan'
> +    endif
> +endif
> +
>  if get_option('default_library') == 'both'
>      error( '''
>   Unsupported value "both" for "default_library" option.
> diff --git a/devtools/words-case.txt b/devtools/words-case.txt
> index bc35c160ba..d315fde8fe 100644
> --- a/devtools/words-case.txt
> +++ b/devtools/words-case.txt
> @@ -105,6 +105,7 @@ TSO
>  TTL
>  Tx
>  uAPI
> +UBSan
>  UDM
>  UDP
>  ULP