From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 92081A04F3; Fri, 20 Dec 2019 16:32:13 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D64831C01; Fri, 20 Dec 2019 16:32:12 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 3DE21F90 for ; Fri, 20 Dec 2019 16:32:10 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Dec 2019 07:32:09 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,336,1571727600"; d="scan'208";a="210843340" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by orsmga008.jf.intel.com with ESMTP; 20 Dec 2019 07:32:08 -0800 Received: from irsmsx103.ger.corp.intel.com ([169.254.3.76]) by irsmsx110.ger.corp.intel.com ([169.254.15.224]) with mapi id 14.03.0439.000; Fri, 20 Dec 2019 15:32:07 +0000 From: "Richardson, Bruce" To: David Marchand , "dev@dpdk.org" CC: "thomas@monjalon.net" , "Laatz, Kevin" , "aconole@redhat.com" , "nhorman@tuxdriver.com" , Michael Santana , "Mcnamara, John" , "Kovacevic, Marko" , "Kinsella, Ray" Thread-Topic: [PATCH] add ABI checks Thread-Index: AQHVt0lHeytI8KABvU62ODvABqKYnqfDJfBA Date: Fri, 20 Dec 2019 15:32:06 +0000 Message-ID: <59AF69C657FD0841A61C55336867B5B07EEAB0AE@IRSMSX103.ger.corp.intel.com> References: <20191220152058.10739-1-david.marchand@redhat.com> In-Reply-To: <20191220152058.10739-1-david.marchand@redhat.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMDVlOWRhMzYtMWFmYi00OWYzLTk4MDQtMmIyYzIyYTZlZjJkIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiSFlocE8xUFNDaDYzcHpUM3pYajA3ejdjUUw1VHRacU9wb2I1SU9OakRuMlZtNlwvZXpXTFJjdDJcL2VTeGEzanF5In0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] add ABI checks X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: David Marchand > Sent: Friday, December 20, 2019 3:21 PM > To: dev@dpdk.org > Cc: thomas@monjalon.net; Richardson, Bruce ; > Laatz, Kevin ; aconole@redhat.com; > nhorman@tuxdriver.com; Michael Santana ; > Mcnamara, John ; Kovacevic, Marko > > Subject: [PATCH] add ABI checks >=20 > Starting from Kevin and Bruce idea of using libabigail, here is an > alternate approach to implement ABI checks. >=20 > 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. >=20 > 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. >=20 > For advanced developers and maintainers, the contributing guide details > the higher level scripts that are quite close to the existing devtools > scripts. >=20 > Signed-off-by: David Marchand > --- > .ci/linux-build.sh | 43 +++++++++++++++++++++++++++ > .travis.yml | 20 +++++++++++-- > devtools/check-abi-dump.sh | 46 +++++++++++++++++++++++++++++ > devtools/check-abi-reference.sh | 27 +++++++++++++++++ > devtools/dpdk.abignore | 2 ++ > devtools/gen-abi-dump.sh | 29 ++++++++++++++++++ > devtools/gen-abi-reference.sh | 24 +++++++++++++++ > devtools/test-build.sh | 13 ++++++-- > devtools/test-meson-builds.sh | 6 ++++ > doc/guides/contributing/patches.rst | 25 ++++++++++++++++ > 10 files changed, 230 insertions(+), 5 deletions(-) create mode 100755 > devtools/check-abi-dump.sh create mode 100755 devtools/check-abi- > reference.sh create mode 100644 devtools/dpdk.abignore create mode > 100755 devtools/gen-abi-dump.sh create mode 100755 devtools/gen-abi- > reference.sh >=20 > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index > ccc3a7ccd..345dba264 100755 > --- a/.ci/linux-build.sh > +++ b/.ci/linux-build.sh > @@ -30,8 +30,51 @@ fi >=20 > OPTS=3D"$OPTS --default-library=3D$DEF_LIB" > meson build --werror -Dexamples=3Dall $OPTS > + > +if [ "$ABI_CHECKS" =3D "1" ]; then > + git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk} > + git fetch --tags ref ${REF_GIT_BRANCH:-master} > + > + head=3D$(git describe --all) > + tag=3D$(git describe --abbrev=3D0) > + > + if [ "$(cat reference/VERSION 2>/dev/null)" !=3D "$tag" ]; then > + rm -rf reference > + fi > + > + if [ ! -d reference ]; then > + gen_abi_dump=3D$(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" !=3D "1" ]; then > + mkdir -p reference/app > + cp -a build/app/dpdk-testpmd reference/app/ > + > + export LD_LIBRARY_PATH=3D$(pwd)/build/lib:$(pwd)/build/drive= rs > + devtools/test-null.sh reference/app/dpdk-testpmd > + unset LD_LIBRARY_PATH > + fi > + echo $tag > reference/VERSION > + > + git checkout -qf $head > + fi > +fi > + > ninja -C build >=20 > +if [ "$ABI_CHECKS" =3D "1" ]; then > + devtools/check-abi-dump.sh build reference ${ABI_CHECKS_WARN_ONLY:-} > + if [ "$AARCH64" !=3D "1" ]; then > + export LD_LIBRARY_PATH=3D$(pwd)/build/lib:$(pwd)/build/drivers > + devtools/test-null.sh reference/app/dpdk-testpmd > + unset LD_LIBRARY_PATH > + fi > +fi > + > if [ "$AARCH64" !=3D "1" ]; then > devtools/test-null.sh > fi > diff --git a/.travis.yml b/.travis.yml > index 8f90d06f2..bbb060fa2 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 >=20 > extra_packages: &extra_packages > - *required_packages > - - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4] > + - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4, > + abigail-tools] >=20 > build_32b_packages: &build_32b_packages > - *required_packages > @@ -59,6 +62,13 @@ matrix: > apt: > packages: > - *aarch64_packages > + - env: DEF_LIB=3D"shared" EXTRA_PACKAGES=3D1 ABI_CHECKS=3D1 AARCH64=3D= 1 > + compiler: gcc > + addons: > + apt: > + packages: > + - *aarch64_packages > + - *extra_packages > - env: DEF_LIB=3D"static" EXTRA_PACKAGES=3D1 > compiler: gcc > addons: > @@ -72,6 +82,12 @@ matrix: > packages: > - *extra_packages > - *doc_packages > + - env: DEF_LIB=3D"shared" EXTRA_PACKAGES=3D1 ABI_CHECKS=3D1 > + compiler: gcc > + addons: > + apt: > + packages: > + - *extra_packages > - env: DEF_LIB=3D"static" OPTS=3D"-Denable_kmods=3Dfalse" EXTRA_PACKAG= ES=3D1 > compiler: gcc > addons: > diff --git a/devtools/check-abi-dump.sh b/devtools/check-abi-dump.sh new > file mode 100755 index 000000000..f48a2ae7e > --- /dev/null > +++ b/devtools/check-abi-dump.sh > @@ -0,0 +1,46 @@ > +#!/bin/sh -e > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red Hat, > +Inc. > + > +if [ $# !=3D 2 ] && [ $# !=3D 3 ]; then > + echo "Usage: $0 builddir dumpdir [warnonly]" > + exit 1 > +fi > + > +builddir=3D$1 > +dumpdir=3D$2 > +warnonly=3D${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=3D"--suppr $(dirname $0)/dpdk.abignore" > +for dump in $(find $dumpdir -name "*.dump"); do > + libname=3D$(basename $dump) > + libname=3D${libname%.dump} > + result=3D > + for f in $(find $builddir -name "$libname.so.*"); do > + if test -L $f || [ "$f" !=3D "${f%%.symbols}" ]; then > + continue > + fi > + result=3Dfound > + > + if ! abidiff $ABIDIFF_OPTIONS $dump $f; then > + if [ -z "$warnonly" ]; then > + echo "Error: ABI issue reported for $dump, $f" > + exit 1 > + fi > + echo "Warning: ABI issue reported for $dump, $f" > + fi > + break > + done > + if [ "$result" !=3D "found" ]; then > + echo "Error: can't find a library for dump file $dump" > + exit 1 > + fi > +done > diff --git a/devtools/check-abi-reference.sh b/devtools/check-abi- > reference.sh new file mode 100755 index 000000000..7addb094e > --- /dev/null > +++ b/devtools/check-abi-reference.sh > @@ -0,0 +1,27 @@ > +#!/bin/sh -e > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red Hat, > +Inc. > + > +devtools_dir=3D$(dirname $(readlink -f $0)) . > +$devtools_dir/load-devel-config > + > +abi_ref_build_dir=3D${DPDK_ABI_REF_BUILD_DIR:-reference} > +builds_dir=3D${DPDK_BUILD_TEST_DIR:-.} > + > +for dir in $abi_ref_build_dir/*; do > + if [ "$dir" =3D "$abi_ref_build_dir" ]; then > + exit 1 > + fi > + if [ ! -d $dir/dump ]; then > + echo "Skipping $dir" > + continue > + fi > + target=3D$(basename $dir) > + if [ -d $builds_dir/$target/install ]; then > + libdir=3D$builds_dir/$target/install > + else > + libdir=3D$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 mod= e > 100644 index 000000000..b866b7f26 > --- /dev/null > +++ b/devtools/dpdk.abignore > @@ -0,0 +1,2 @@ > +[suppress_function] > + symbol_version =3D EXPERIMENTAL > diff --git a/devtools/gen-abi-dump.sh b/devtools/gen-abi-dump.sh new file > mode 100755 index 000000000..4e38d751f > --- /dev/null > +++ b/devtools/gen-abi-dump.sh > @@ -0,0 +1,29 @@ > +#!/bin/sh -e > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red Hat, > +Inc. > + > +if [ $# !=3D 2 ]; then > + echo "Usage: $0 builddir dumpdir" > + exit 1 > +fi > + > +builddir=3D$1 > +dumpdir=3D$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" !=3D "${f%%.symbols}" ]; then > + continue > + fi > + > + libname=3D$(basename $f) > + abidw --out-file $dumpdir/${libname%.so.*}.dump $f done > diff --git a/devtools/gen-abi-reference.sh b/devtools/gen-abi-reference.s= h > new file mode 100755 index 000000000..f41d7fadc > --- /dev/null > +++ b/devtools/gen-abi-reference.sh > @@ -0,0 +1,24 @@ > +#!/bin/sh -e > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red Hat, > +Inc. > + > +devtools_dir=3D$(dirname $(readlink -f $0)) . > +$devtools_dir/load-devel-config > + > +abi_ref_build_dir=3D${DPDK_ABI_REF_BUILD_DIR:-reference} > +for dir in $abi_ref_build_dir/*; do > + if [ "$dir" =3D "$abi_ref_build_dir" ]; then > + exit 1 > + fi > + if [ -d $dir/dump ]; then > + echo "Skipping $dir" > + continue > + fi > + if [ -d $dir/install ]; then > + libdir=3D$dir/install > + else > + libdir=3D$dir > + fi > + echo "Dumping libraries from $libdir in $dir/dump" > + $devtools_dir/gen-abi-dump.sh $libdir $dir/dump done > diff --git a/devtools/test-build.sh b/devtools/test-build.sh index > 52305fbb8..8cb5b56fb 100755 > --- a/devtools/test-build.sh > +++ b/devtools/test-build.sh > @@ -30,7 +30,8 @@ default_path=3D$PATH > # - LIBSSO_SNOW3G_PATH > # - LIBSSO_KASUMI_PATH > # - LIBSSO_ZUC_PATH > -. $(dirname $(readlink -f $0))/load-devel-config > +devtools_dir=3D$(dirname $(readlink -f $0)) . > +$devtools_dir/load-devel-config >=20 > 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 >=20 > J=3D$DPDK_MAKE_JOBS > +abi_ref_build_dir=3D${DPDK_ABI_REF_BUILD_DIR:-reference} > builds_dir=3D${DPDK_BUILD_TEST_DIR:-.} > short=3Dfalse > unset verbose > @@ -97,7 +99,7 @@ trap "signal=3DINT ; trap - INT ; kill -INT $$" INT # > notify result on exit trap on_exit EXIT >=20 > -cd $(dirname $(readlink -f $0))/.. > +cd $devtools_dir/.. >=20 > reset_env () > { > @@ -233,7 +235,7 @@ for conf in $configs ; do > # reload config with DPDK_TARGET set > DPDK_TARGET=3D$target > reset_env > - . $(dirname $(readlink -f $0))/load-devel-config > + . $devtools_dir/load-devel-config >=20 > options=3D$(echo $conf | sed 's,[^~+]*,,') > dir=3D$builds_dir/$conf > @@ -246,6 +248,11 @@ for conf in $configs ; do > export RTE_TARGET=3D$target > rm -rf $dir/install > ${MAKE} install O=3D$dir DESTDIR=3D$dir/install prefix=3D > + if [ -d $abi_ref_build_dir/$conf/dump ]; then > + echo "=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D Check ABI= $conf" > + $devtools_dir/check-abi-dump.sh $dir/install \ > + $abi_ref_build_dir/$conf/dump > + fi > echo "=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D Build exam= ples for $conf" > export RTE_SDK=3D$(readlink -f $dir)/install/share/dpdk > ln -sTf $(pwd)/lib $RTE_SDK/lib # workaround for vm_power_manager > diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.s= h > index 688567714..aaefa38a2 100755 > --- a/devtools/test-meson-builds.sh > +++ b/devtools/test-meson-builds.sh > @@ -16,6 +16,7 @@ srcdir=3D$(dirname $(readlink -f $0))/.. >=20 > MESON=3D${MESON:-meson} > use_shared=3D"--default-library=3Dshared" > +abi_ref_build_dir=3D${DPDK_ABI_REF_BUILD_DIR:-reference} > builds_dir=3D${DPDK_BUILD_TEST_DIR:-.} >=20 > if command -v gmake >/dev/null 2>&1 ; then @@ -88,6 +89,11 @@ build () # > > 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 > } >=20 > if [ "$1" =3D "-vv" ] ; then > diff --git a/doc/guides/contributing/patches.rst > b/doc/guides/contributing/patches.rst > index 0686450e4..de3dff145 100644 > --- a/doc/guides/contributing/patches.rst > +++ b/doc/guides/contributing/patches.rst > @@ -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. >=20 >=20 > +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=3D0) > + > +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=3Dreference``, so that the builds occu= r > +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 > --------------- >=20 > -- > 2.23.0 I still very much dislike forcing the user to generate his own reference ve= rsion to compare the ABI against. These should be archived and the user should ju= st be able to pull them down via git or http or otherwise. Two reasons for thi= s: 1. Less error prone, since there is no chance of the user having an incorre= ct build for whatever reason. 2. Less effort for the user than asking them to do extra builds. The more s= teps the user has to follow, the less likely they are to attempt the process. Regards, /Bruce