From: David Marchand Date: Sun, 2 Feb 2020 21:08:34 +0000 (+0100) Subject: devtools: add ABI checks X-Git-Url: http://git.droids-corp.org/?a=commitdiff_plain;h=777014e56d07;p=dpdk.git devtools: add ABI checks For normal developers, those checks are disabled. Enabling them requires a configuration that will trigger the ABI dumps generation as part of the existing devtools/test-build.sh and devtools/test-meson-builds.sh scripts. Those checks are enabled in the CI for the default meson options on x86 and aarch64 so that proposed patches are validated via our CI robot. A cache of the ABI is stored in travis jobs to avoid rebuilding too often. Checks can be informational only, by setting ABI_CHECKS_WARN_ONLY when breaking the ABI in a future release. Explicit suppression rules have been added on internal structures exposed to crypto drivers as the current ABI policy does not apply to them. This could be improved in the future by carefully splitting the headers content with application and driver "users" in mind. We currently have issues reported for librte_crypto recent changes for which suppression rules have been added too. Mellanox glue libraries are explicitly skipped as they are not part of the application ABI. Signed-off-by: David Marchand Acked-by: Luca Boccassi --- diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index ccc3a7ccd9..c7c3840fc2 100755 --- a/.ci/linux-build.sh +++ b/.ci/linux-build.sh @@ -29,6 +29,7 @@ if [ "$BUILD_32BIT" = "1" ]; then fi OPTS="$OPTS --default-library=$DEF_LIB" +OPTS="$OPTS --buildtype=debugoptimized" meson build --werror -Dexamples=all $OPTS ninja -C build @@ -36,6 +37,29 @@ if [ "$AARCH64" != "1" ]; then devtools/test-null.sh fi +if [ "$ABI_CHECKS" = "1" ]; then + REF_GIT_REPO=${REF_GIT_REPO:-https://dpdk.org/git/dpdk} + REF_GIT_TAG=${REF_GIT_TAG:-v19.11} + + if [ "$(cat reference/VERSION 2>/dev/null)" != "$REF_GIT_TAG" ]; then + rm -rf reference + fi + + if [ ! -d reference ]; then + refsrcdir=$(readlink -f $(pwd)/../dpdk-$REF_GIT_TAG) + git clone --single-branch -b $REF_GIT_TAG $REF_GIT_REPO $refsrcdir + meson --werror $OPTS $refsrcdir $refsrcdir/build + ninja -C $refsrcdir/build + DESTDIR=$(pwd)/reference ninja -C $refsrcdir/build install + devtools/gen-abi.sh reference + echo $REF_GIT_TAG > reference/VERSION + fi + + DESTDIR=$(pwd)/install ninja -C build install + devtools/gen-abi.sh install + devtools/check-abi.sh reference install ${ABI_CHECKS_WARN_ONLY:-} +fi + if [ "$RUN_TESTS" = "1" ]; then sudo meson test -C build --suite fast-tests -t 3 fi diff --git a/.travis.yml b/.travis.yml index 8162f1c059..22539d8238 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,8 @@ language: c -cache: ccache +cache: + ccache: true + directories: + - reference compiler: - gcc - clang @@ -21,7 +24,7 @@ aarch64_packages: &aarch64_packages extra_packages: &extra_packages - *required_packages - - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4] + - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4, abigail-tools] build_32b_packages: &build_32b_packages - *required_packages @@ -151,5 +154,18 @@ matrix: packages: - *required_packages - *doc_packages + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1 + compiler: gcc + addons: + apt: + packages: + - *extra_packages + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1 + arch: arm64 + compiler: gcc + addons: + apt: + packages: + - *extra_packages script: ./.ci/${TRAVIS_OS_NAME}-build.sh diff --git a/MAINTAINERS b/MAINTAINERS index 8047aaf2a1..08aa0d8a81 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -144,8 +144,11 @@ M: Neil Horman F: lib/librte_eal/common/include/rte_compat.h F: lib/librte_eal/common/include/rte_function_versioning.h F: doc/guides/rel_notes/deprecation.rst +F: devtools/check-abi.sh F: devtools/check-abi-version.sh F: devtools/check-symbol-change.sh +F: devtools/gen-abi.sh +F: devtools/libabigail.abignore F: devtools/update-abi.sh F: devtools/update_version_map_abi.py F: devtools/validate-abi.sh diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh new file mode 100755 index 0000000000..0b4d1a37e2 --- /dev/null +++ b/devtools/check-abi.sh @@ -0,0 +1,59 @@ +#!/bin/sh -e +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2019 Red Hat, Inc. + +if [ $# != 2 ] && [ $# != 3 ]; then + echo "Usage: $0 refdir newdir [warnonly]" + exit 1 +fi + +refdir=$1 +newdir=$2 +warnonly=${3:-} +ABIDIFF_OPTIONS="--suppr $(dirname $0)/libabigail.abignore --no-added-syms" + +if [ ! -d $refdir ]; then + echo "Error: reference directory '$refdir' does not exist." + exit 1 +fi +incdir=$(find $refdir -type d -a -name include) +if [ -z "$incdir" ] || [ ! -e "$incdir" ]; then + echo "WARNING: could not identify a include directory for $refdir, expect false positives..." +else + ABIDIFF_OPTIONS="$ABIDIFF_OPTIONS --headers-dir1 $incdir" +fi + +if [ ! -d $newdir ]; then + echo "Error: directory to check '$newdir' does not exist." + exit 1 +fi +incdir2=$(find $newdir -type d -a -name include) +if [ -z "$incdir2" ] || [ ! -e "$incdir2" ]; then + echo "WARNING: could not identify a include directory for $newdir, expect false positives..." +else + ABIDIFF_OPTIONS="$ABIDIFF_OPTIONS --headers-dir2 $incdir2" +fi + +error= +for dump in $(find $refdir -name "*.dump"); do + name=$(basename $dump) + # skip glue drivers, example librte_pmd_mlx5_glue.dump + # We can't rely on a suppression rule for now: + # https://sourceware.org/bugzilla/show_bug.cgi?id=25480 + if [ "$name" != "${name%%_glue.dump}" ]; then + echo "Skipping ${dump}..." + continue + fi + dump2=$(find $newdir -name $name) + if [ -z "$dump2" ] || [ ! -e "$dump2" ]; then + echo "Error: can't find $name in $newdir" + error=1 + continue + fi + if ! abidiff $ABIDIFF_OPTIONS $dump $dump2; then + echo "Error: ABI issue reported for 'abidiff $ABIDIFF_OPTIONS $dump $dump2'" + error=1 + fi +done + +[ -z "$error" ] || [ -n "$warnonly" ] diff --git a/devtools/gen-abi.sh b/devtools/gen-abi.sh new file mode 100755 index 0000000000..c44b0e228a --- /dev/null +++ b/devtools/gen-abi.sh @@ -0,0 +1,26 @@ +#!/bin/sh -e +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2019 Red Hat, Inc. + +if [ $# != 1 ]; then + echo "Usage: $0 installdir" + exit 1 +fi + +installdir=$1 +if [ ! -d $installdir ]; then + echo "Error: install directory '$installdir' does not exist." + exit 1 +fi + +dumpdir=$installdir/dump +rm -rf $dumpdir +mkdir -p $dumpdir +for f in $(find $installdir -name "*.so.*"); do + if test -L $f; then + continue + fi + + libname=$(basename $f) + abidw --out-file $dumpdir/${libname%.so*}.dump $f +done diff --git a/devtools/libabigail.abignore b/devtools/libabigail.abignore new file mode 100644 index 0000000000..a59df8f135 --- /dev/null +++ b/devtools/libabigail.abignore @@ -0,0 +1,13 @@ +[suppress_function] + symbol_version = EXPERIMENTAL +[suppress_variable] + symbol_version = EXPERIMENTAL + +; Explicit ignore for driver-only ABI +[suppress_type] + name = rte_cryptodev_ops +; Ignore this enum update as it is part of an experimental API +[suppress_type] + type_kind = enum + name = rte_crypto_asym_xform_type + changed_enumerators = RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END diff --git a/devtools/test-build.sh b/devtools/test-build.sh index 52305fbb8c..c30c53d189 100755 --- a/devtools/test-build.sh +++ b/devtools/test-build.sh @@ -6,6 +6,8 @@ default_path=$PATH # Load config options: # - ARMV8_CRYPTO_LIB_PATH +# - DPDK_ABI_REF_DIR +# - DPDK_ABI_REF_VERSION # - DPDK_BUILD_TEST_CONFIGS (defconfig1+option1+option2 defconfig2) # - DPDK_BUILD_TEST_DIR # - DPDK_DEP_ARCHIVE @@ -30,7 +32,8 @@ default_path=$PATH # - LIBSSO_SNOW3G_PATH # - LIBSSO_KASUMI_PATH # - LIBSSO_ZUC_PATH -. $(dirname $(readlink -f $0))/load-devel-config +devtools_dir=$(dirname $(readlink -f $0)) +. $devtools_dir/load-devel-config print_usage () { echo "usage: $(basename $0) [-h] [-jX] [-s] [config1 [config2] ...]]" @@ -67,7 +70,8 @@ J=$DPDK_MAKE_JOBS builds_dir=${DPDK_BUILD_TEST_DIR:-.} short=false unset verbose -maxerr=-Wfatal-errors +# for ABI checks, we need debuginfo +test_cflags="-Wfatal-errors -g" while getopts hj:sv ARG ; do case $ARG in j ) J=$OPTARG ;; @@ -97,7 +101,7 @@ trap "signal=INT ; trap - INT ; kill -INT $$" INT # notify result on exit trap on_exit EXIT -cd $(dirname $(readlink -f $0))/.. +cd $devtools_dir/.. reset_env () { @@ -233,14 +237,14 @@ for conf in $configs ; do # reload config with DPDK_TARGET set DPDK_TARGET=$target reset_env - . $(dirname $(readlink -f $0))/load-devel-config + . $devtools_dir/load-devel-config options=$(echo $conf | sed 's,[^~+]*,,') dir=$builds_dir/$conf config $dir $target $options echo "================== Build $conf" - ${MAKE} -j$J EXTRA_CFLAGS="$maxerr $DPDK_DEP_CFLAGS" \ + ${MAKE} -j$J EXTRA_CFLAGS="$test_cflags $DPDK_DEP_CFLAGS" \ EXTRA_LDFLAGS="$DPDK_DEP_LDFLAGS" $verbose O=$dir ! $short || break export RTE_TARGET=$target @@ -253,6 +257,43 @@ for conf in $configs ; do EXTRA_LDFLAGS="$DPDK_DEP_LDFLAGS" $verbose \ O=$(readlink -f $dir)/examples unset RTE_TARGET + if [ -n "$DPDK_ABI_REF_VERSION" ]; then + abirefdir=${DPDK_ABI_REF_DIR:-reference}/$DPDK_ABI_REF_VERSION + if [ ! -d $abirefdir/$conf ]; then + # clone current sources + if [ ! -d $abirefdir/src ]; then + git clone --local --no-hardlinks \ + --single-branch \ + -b $DPDK_ABI_REF_VERSION \ + $(pwd) $abirefdir/src + fi + + cd $abirefdir/src + + rm -rf $abirefdir/build + config $abirefdir/build $target $options + + echo -n "================== Build $conf " + echo "($DPDK_ABI_REF_VERSION)" + ${MAKE} -j$J \ + EXTRA_CFLAGS="$test_cflags $DPDK_DEP_CFLAGS" \ + EXTRA_LDFLAGS="$DPDK_DEP_LDFLAGS" $verbose \ + O=$abirefdir/build + export RTE_TARGET=$target + ${MAKE} install O=$abirefdir/build \ + DESTDIR=$abirefdir/$conf \ + prefix= + unset RTE_TARGET + $devtools_dir/gen-abi.sh $abirefdir/$conf + + # back to current workdir + cd $devtools_dir/.. + fi + + echo "================== Check ABI $conf" + $devtools_dir/gen-abi.sh $dir/install + $devtools_dir/check-abi.sh $abirefdir/$conf $dir/install + fi echo "################## $conf done." unset dir done diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh index fb6c404e52..c2d33b3b9f 100755 --- a/devtools/test-meson-builds.sh +++ b/devtools/test-meson-builds.sh @@ -64,9 +64,18 @@ config () # builddir=$1 shift if [ -f "$builddir/build.ninja" ] ; then + # for existing environments, switch to debugoptimized if unset + # so that ABI checks can run + if ! $MESON configure $builddir | + awk '$1=="buildtype" {print $2}' | + grep -qw debugoptimized; then + $MESON configure --buildtype=debugoptimized $builddir + fi return fi - options="--werror -Dexamples=all" + options= + options="$options --werror -Dexamples=all" + options="$options --buildtype=debugoptimized" for option in $DPDK_MESON_OPTIONS ; do options="$options -D$option" done @@ -92,6 +101,13 @@ compile () # fi } +install_target () # +{ + rm -rf $2 + echo "DESTDIR=$2 $ninja_cmd -C $1 install" + DESTDIR=$2 $ninja_cmd -C $1 install +} + build () # { targetdir=$1 @@ -103,6 +119,31 @@ build () # load_env $targetcc || return 0 config $srcdir $builds_dir/$targetdir $* compile $builds_dir/$targetdir + if [ -n "$DPDK_ABI_REF_VERSION" ]; then + abirefdir=${DPDK_ABI_REF_DIR:-reference}/$DPDK_ABI_REF_VERSION + if [ ! -d $abirefdir/$targetdir ]; then + # clone current sources + if [ ! -d $abirefdir/src ]; then + git clone --local --no-hardlinks \ + --single-branch \ + -b $DPDK_ABI_REF_VERSION \ + $srcdir $abirefdir/src + fi + + rm -rf $abirefdir/build + config $abirefdir/src $abirefdir/build $* + compile $abirefdir/build + install_target $abirefdir/build $abirefdir/$targetdir + $srcdir/devtools/gen-abi.sh $abirefdir/$targetdir + fi + + install_target $builds_dir/$targetdir \ + $(readlink -f $builds_dir/$targetdir/install) + $srcdir/devtools/gen-abi.sh \ + $(readlink -f $builds_dir/$targetdir/install) + $srcdir/devtools/check-abi.sh $abirefdir/$targetdir \ + $(readlink -f $builds_dir/$targetdir/install) + fi } if [ "$1" = "-vv" ] ; then @@ -153,8 +194,11 @@ done # Test installation of the x86-default target, to be used for checking # the sample apps build using the pkg-config file for cflags and libs build_path=$(readlink -f $builds_dir/build-x86-default) -export DESTDIR=$build_path/install-root -$ninja_cmd -C $build_path install +export DESTDIR=$build_path/install +# No need to reinstall if ABI checks are enabled +if [ -z "$DPDK_ABI_REF_VERSION" ]; then + install_target $build_path $DESTDIR +fi load_env cc pc_file=$(find $DESTDIR -name libdpdk.pc) diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst index 0686450e45..59442824a1 100644 --- a/doc/guides/contributing/patches.rst +++ b/doc/guides/contributing/patches.rst @@ -513,6 +513,21 @@ in a single subfolder called "__builds" created in the current directory. Setting ``DPDK_BUILD_TEST_DIR`` to an absolute directory path e.g. ``/tmp`` is also supported. +Checking ABI compatibility +-------------------------- + +By default, ABI compatibility checks are disabled. + +To enable them, a reference version must be selected via the environment +variable ``DPDK_ABI_REF_VERSION``. + +The ``devtools/test-build.sh`` and ``devtools/test-meson-builds.sh`` scripts +then build this reference version in a temporary directory and store the +results in a subfolder of the current working directory. +The environment variable ``DPDK_ABI_REF_DIR`` can be set so that the results go +to a different location. + + Sending Patches ---------------