add ABI checks
authorDavid Marchand <david.marchand@redhat.com>
Thu, 19 Dec 2019 21:45:03 +0000 (22:45 +0100)
committerDavid Marchand <david.marchand@redhat.com>
Wed, 15 Jan 2020 09:38:14 +0000 (10:38 +0100)
Starting from Kevin and Bruce idea of using libabigail, here is an
alternate approach to implement ABI checks.

By default, those checks are disabled and enabling them requires a
manual step that generates the ABI dumps on a reference version for a
set of configurations.

Those checks are enabled in the CI by default for the default meson
options on x86 and aarch64 so that proposed patches are validated.
A cache of the ABI is stored in travis jobs.
Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
breaking the ABI in a future release.

For advanced developers and maintainers, the contributing guide details
the higher level scripts that are quite close to the existing devtools
scripts.

Signed-off-by: David Marchand <david.marchand@redhat.com>
.ci/linux-build.sh
.travis.yml
devtools/check-abi-dump.sh [new file with mode: 0755]
devtools/check-abi-reference.sh [new file with mode: 0755]
devtools/dpdk.abignore [new file with mode: 0644]
devtools/gen-abi-dump.sh [new file with mode: 0755]
devtools/gen-abi-reference.sh [new file with mode: 0755]
devtools/test-build.sh
devtools/test-meson-builds.sh
doc/guides/contributing/patches.rst

index ccc3a7c..2a75421 100755 (executable)
@@ -30,8 +30,52 @@ fi
 
 OPTS="$OPTS --default-library=$DEF_LIB"
 meson build --werror -Dexamples=all $OPTS
+
+if [ "$ABI_CHECKS" = "1" ]; then
+    git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
+    git fetch --tags ref ${REF_GIT_BRANCH:-master}
+
+    head=$(git describe --all)
+    tag=$(git describe --abbrev=0)
+
+    if [ "$(cat reference/VERSION 2>/dev/null)" != "$tag" ]; then
+        rm -rf reference
+    fi
+
+    if [ ! -d reference ]; then
+        gen_abi_dump=$(mktemp -t gen-abi-dump-XXX.sh)
+        cp -a devtools/gen-abi-dump.sh $gen_abi_dump
+
+        git checkout -qf $tag
+        ninja -C build
+        $gen_abi_dump build reference
+
+        if [ "$AARCH64" != "1" ]; then
+            mkdir -p reference/app
+            cp -a build/app/dpdk-testpmd reference/app/
+
+            export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
+            devtools/test-null.sh reference/app/dpdk-testpmd
+            unset LD_LIBRARY_PATH
+        fi
+        echo $tag > reference/VERSION
+
+        rm $gen_abi_dump
+        git checkout -qf $head
+    fi
+fi
+
 ninja -C build
 
+if [ "$ABI_CHECKS" = "1" ]; then
+    devtools/check-abi-dump.sh build reference ${ABI_CHECKS_WARN_ONLY:-}
+    if [ "$AARCH64" != "1" ]; then
+        export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
+        devtools/test-null.sh reference/app/dpdk-testpmd
+        unset LD_LIBRARY_PATH
+    fi
+fi
+
 if [ "$AARCH64" != "1" ]; then
     devtools/test-null.sh
 fi
index 980c760..1d44ae4 100644 (file)
@@ -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
@@ -72,6 +75,12 @@ matrix:
         packages:
           - *extra_packages
           - *doc_packages
+  - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
+    compiler: gcc
+    addons:
+      apt:
+        packages:
+          - *extra_packages
   - env: DEF_LIB="static" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
     compiler: gcc
     addons:
@@ -147,6 +156,14 @@ matrix:
       apt:
         packages:
           - *required_packages
+  - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
+    arch: arm64
+    dist: bionic
+    compiler: gcc
+    addons:
+      apt:
+        packages:
+          - *extra_packages
   - env: DEF_LIB="shared" OPTS="-Denable_kmods=false" BUILD_DOCS=1
     arch: arm64
     compiler: gcc
diff --git a/devtools/check-abi-dump.sh b/devtools/check-abi-dump.sh
new file mode 100755 (executable)
index 0000000..5e02215
--- /dev/null
@@ -0,0 +1,46 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Red Hat, Inc.
+
+if [ $# != 2 ] && [ $# != 3 ]; then
+       echo "Usage: $0 builddir dumpdir [warnonly]"
+       exit 1
+fi
+
+builddir=$1
+dumpdir=$2
+warnonly=${3:-}
+if [ ! -d $builddir ]; then
+       echo "Error: build directory '$builddir' does not exist."
+       exit 1
+fi
+if [ ! -d $dumpdir ]; then
+       echo "Error: dump directory '$dumpdir' does not exist."
+       exit 1
+fi
+
+ABIDIFF_OPTIONS="--suppr $(dirname $0)/dpdk.abignore"
+error=
+for dump in $(find $dumpdir -name "*.dump"); do
+       libname=$(basename $dump)
+       libname=${libname%.dump}
+       result=
+       for f in $(find $builddir -name "$libname.so.*"); do
+               if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
+                       continue
+               fi
+               result=found
+
+               if ! abidiff $ABIDIFF_OPTIONS $dump $f; then
+                       echo "Error: ABI issue reported for $dump, $f"
+                       error=1
+               fi
+               break
+       done
+       if [ "$result" != "found" ]; then
+               echo "Error: can't find a library for dump file $dump"
+               error=1
+       fi
+done
+
+[ -z "$error" ] || [ -n "$warnonly" ]
diff --git a/devtools/check-abi-reference.sh b/devtools/check-abi-reference.sh
new file mode 100755 (executable)
index 0000000..7addb09
--- /dev/null
@@ -0,0 +1,27 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Red Hat, Inc.
+
+devtools_dir=$(dirname $(readlink -f $0))
+. $devtools_dir/load-devel-config
+
+abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
+builds_dir=${DPDK_BUILD_TEST_DIR:-.}
+
+for dir in $abi_ref_build_dir/*; do
+       if [ "$dir" = "$abi_ref_build_dir" ]; then
+               exit 1
+       fi
+       if [ ! -d $dir/dump ]; then
+               echo "Skipping $dir"
+               continue
+       fi
+       target=$(basename $dir)
+       if [ -d $builds_dir/$target/install ]; then
+               libdir=$builds_dir/$target/install
+       else
+               libdir=$builds_dir/$target
+       fi
+       echo "Checking ABI between $libdir and $dir/dump"
+       $devtools_dir/check-abi-dump.sh $libdir $dir/dump
+done
diff --git a/devtools/dpdk.abignore b/devtools/dpdk.abignore
new file mode 100644 (file)
index 0000000..b866b7f
--- /dev/null
@@ -0,0 +1,2 @@
+[suppress_function]
+        symbol_version = EXPERIMENTAL
diff --git a/devtools/gen-abi-dump.sh b/devtools/gen-abi-dump.sh
new file mode 100755 (executable)
index 0000000..4e38d75
--- /dev/null
@@ -0,0 +1,29 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Red Hat, Inc.
+
+if [ $# != 2 ]; then
+       echo "Usage: $0 builddir dumpdir"
+       exit 1
+fi
+
+builddir=$1
+dumpdir=$2
+if [ ! -d $builddir ]; then
+       echo "Error: build directory '$builddir' does not exist."
+       exit 1
+fi
+if [ -d $dumpdir ]; then
+       echo "Error: dump directory '$dumpdir' already exists."
+       exit 1
+fi
+
+mkdir -p $dumpdir
+for f in $(find $builddir -name "*.so.*"); do
+       if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
+               continue
+       fi
+
+       libname=$(basename $f)
+       abidw --out-file $dumpdir/${libname%.so.*}.dump $f
+done
diff --git a/devtools/gen-abi-reference.sh b/devtools/gen-abi-reference.sh
new file mode 100755 (executable)
index 0000000..f41d7fa
--- /dev/null
@@ -0,0 +1,24 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Red Hat, Inc.
+
+devtools_dir=$(dirname $(readlink -f $0))
+. $devtools_dir/load-devel-config
+
+abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
+for dir in $abi_ref_build_dir/*; do
+       if [ "$dir" = "$abi_ref_build_dir" ]; then
+               exit 1
+       fi
+       if [ -d $dir/dump ]; then
+               echo "Skipping $dir"
+               continue
+       fi
+       if [ -d $dir/install ]; then
+               libdir=$dir/install
+       else
+               libdir=$dir
+       fi
+       echo "Dumping libraries from $libdir in $dir/dump"
+       $devtools_dir/gen-abi-dump.sh $libdir $dir/dump
+done
index 52305fb..8cb5b56 100755 (executable)
@@ -30,7 +30,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] ...]]"
@@ -64,6 +65,7 @@ print_help () {
 [ -z $MAKE ] && echo "Cannot find make or gmake" && exit 1
 
 J=$DPDK_MAKE_JOBS
+abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
 builds_dir=${DPDK_BUILD_TEST_DIR:-.}
 short=false
 unset verbose
@@ -97,7 +99,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,7 +235,7 @@ 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
@@ -246,6 +248,11 @@ for conf in $configs ; do
        export RTE_TARGET=$target
        rm -rf $dir/install
        ${MAKE} install O=$dir DESTDIR=$dir/install prefix=
+       if [ -d $abi_ref_build_dir/$conf/dump ]; then
+               echo "================== Check ABI $conf"
+               $devtools_dir/check-abi-dump.sh $dir/install \
+                       $abi_ref_build_dir/$conf/dump
+       fi
        echo "================== Build examples for $conf"
        export RTE_SDK=$(readlink -f $dir)/install/share/dpdk
        ln -sTf $(pwd)/lib $RTE_SDK/lib # workaround for vm_power_manager
index 6885677..aaefa38 100755 (executable)
@@ -16,6 +16,7 @@ srcdir=$(dirname $(readlink -f $0))/..
 
 MESON=${MESON:-meson}
 use_shared="--default-library=shared"
+abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
 builds_dir=${DPDK_BUILD_TEST_DIR:-.}
 
 if command -v gmake >/dev/null 2>&1 ; then
@@ -88,6 +89,11 @@ build () # <directory> <target compiler> <meson options>
                echo "$ninja_cmd -C $builddir"
                $ninja_cmd -C $builddir
        fi
+
+       if [ -d $abi_ref_build_dir/$1/dump ]; then
+               $srcdir/devtools/check-abi-dump.sh $builddir
+                       $abi_ref_build_dir/$1/dump
+       fi
 }
 
 if [ "$1" = "-vv" ] ; then
index 0686450..de3dff1 100644 (file)
@@ -513,6 +513,31 @@ 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
+--------------------------
+
+The first thing is to build reference binaries for the latest release your
+patches are built on top of.
+
+Either you are in a git tree and an easy way to identify this is to run::
+
+  git checkout $(git describe --abbrev=0)
+
+Or you use a tarball and you extract the sources in a director of your choice.
+
+Next is building those sources, refer to the previous paragraph.
+You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds occur in this
+directory.
+
+Finally, the ABI dump files are generated with the
+``devtools/gen-abi-reference.sh`` script. This script will look for builds in
+the current sub directory ``reference``. But you can set the environment
+variable ``DPDK_ABI_REF_BUILD_DIR`` to a different location.
+
+Once done, you can check your current binaries ABI with this reference with the
+``devtools/check-abi-reference.sh`` script.
+
+
 Sending Patches
 ---------------