Re: [PATCH 10/10] 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.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

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

>  .ci/linux-build.sh          | 27 +++++++++++++++++++++++++--
>  .github/workflows/build.yml |  5 +++++
>  app/test/suites/meson.build |  3 +--
>  config/meson.build          | 18 +++++++++++++++++-
>  devtools/words-case.txt     |  1 +
>  5 files changed, 49 insertions(+), 5 deletions(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index e9272d3931..905c11b1bf 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,28 @@ 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
> +    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 +162,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 +204,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..4353bb97d8 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
> +            mini: mini
> +            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