#17 Add a test to validate libLLVM ABI compatibility on Fedora
Closed 7 months ago by tuliom. Opened 8 months ago by tuliom.
tests/ tuliom/llvm fedora-abi-test  into  main

file added
+22
@@ -0,0 +1,22 @@ 

+ summary: Test ABI compatibility.

+ test: ./test.sh

+ framework: shell

+ tier: 1

+ component:

+   - llvm-toolset

+   - llvm

+ require:

+   - abi-compliance-checker

+   - binutils

+   - gzip

+   - llvm-devel

+   - llvm-libs

+   - llvm-libs-debuginfo

+   - wget

+   - xz

+ duration: 200m

+ 

+ adjust:

+   # TODO: Test on rhel before enabling it

+   - enabled: false

+     when: distro == rhel or distro == centos

file added
+50
@@ -0,0 +1,50 @@ 

+ #!/usr/bin/sh -ex

+ 

+ # Test the compatibility of the current libLLVM with specific versions that

+ # have been previously generated.

+ # The artifacts used as input in this test are maintained in this git-lfs

+ # repo: https://gitlab.com/tuliom-rh/llvm-fedora-el-abi

+ 

+ is_fedora=`rpm -E %{fedora}`

+ arch=`rpm -E %{_arch}`

+ 

+ case $(rpm -E %dist) in

+     .fc*) dist_prefix=fc;;

+     .el*) dist_prefix=el;;

+     *) echo "Error: which distro is this ($(rpm -E %dist))?" >&2

+ esac

+ 

+ # Download ABI dumps from previous versions.

+ wget -q https://gitlab.com/tuliom-rh/llvm-fedora-el-abi/-/archive/main/llvm-fedora-el-abi-main.tar.gz

+ tar xzf llvm-fedora-el-abi-main.tar.gz --strip-components=1 '*/libLLVM.*.abi.xz'

+ 

+ for f in *.abi.xz; do

+     unxz $f

+ done

+ 

+ abi_files=$(ls libLLVM*.$dist_prefix*.$arch.abi)

+ 

+ abi_file=libLLVM.$(rpm -q llvm-libs | sed 's/~//g').abi

+ version=$(rpm -q --qf "%{V}-%{R}.%{ARCH}\n" llvm-libs)

+ library=$(rpm -ql llvm-libs | grep -E 'libLLVM-.*.so' | sort | head -n 1)

+ debuginfo=$(rpm -ql llvm-libs-debuginfo | grep -E 'libLLVM-.*\.so.*\.debug')

+ 

+ nm $debuginfo \

+     | awk '/T _LLVM/ || /T LLVM/ { print $3 }' \

+     | sort -u \

+     | sed -e 's/^_//g' | cut -d ' ' -f 3 > llvm.symbols

+ 

+ # Create the ABI dump from the current version. This takes a long time.

+ abi-dumper -quiet -symbols-list llvm.symbols \

+     -lver $version \

+     -skip-cxx \

+     -public-headers /usr/include/llvm-c \

+     --search-debuginfo=/usr/lib/debug \

+     -o $abi_file $library

+ 

+ sed -i 's/LLVM_[0-9]\+/LLVM_NOVERSION/' $abi_file

+ 

+ for old_abi_file in $abi_files; do

+     abi-compliance-checker -symbols-list llvm.symbols \

+         -l libLLVM.so -old $old_abi_file -new $abi_file

+ done

file modified
+1 -1
@@ -59,4 +59,4 @@ 

      how: tmt

  provision:

    hardware:

-     memory: ">= 4 GiB"

+     memory: ">= 10 GiB"

This test uses abi-compliance-checker in order to compare the ABI of the
current libLLVM.so with previous versions that are stored in a separate
git-lfs repository.

This new test can take very long to complete (100 minutes on a small
VMs) and consumes far more RAM than previous tests (up to 10 GiB).

rpminspect already runs abidiff, e.g. see https://artifacts.dev.testing-farm.io/072d8e50-278d-4c94-aa6c-e8af724b652c/ for the rc4 update and open the abidiff tab there.

Can we review that output rather than adding a separate test?

rpminspect already runs abidiff, e.g. see https://artifacts.dev.testing-farm.io/072d8e50-278d-4c94-aa6c-e8af724b652c/ for the rc4 update and open the abidiff tab there.

Can we review that output rather than adding a separate test?

@nikic there are at least 2 reasons to add a new test:

  1. Comparison between more versions
    AFAIU, rpminspect only compares the last version in the repository against the new build.
    That means it will miss a comparison if 2 Bodhi updates are submitted before the first one reaches stable.
    While the code does not support CentOS/RHEL yet, the new code is able to compare libraries from different distros too.
    However, the new test will not compare all released libraries against the current one. In other words: it will miss comparisons too. We can decide the amount of comparisons will be tested here.

  2. Comparison of header files
    abidiff is based on libabigail. AFAIK, libabigail can only compare ELF files.
    Meanwhile, abi-compliance-checker can also compare header files.

@tstellar what do you think?

rebased onto 8992e4f

8 months ago

Fixed an issue pointed out by @jcheca .

@tuliom thanks for the fix. Could you apply the suggestion from this comment in this PR too, please?

In the past, the abidiff tests always timed out, but if it is working now, I think it makes sense to try to use it. Is there some way we can make this a gating test?

@tstellar We can make all of the rpminspect tests as gating: https://docs.fedoraproject.org/en-US/ci/gating/#_enable_gating

Notice there were some Unicode issues on clang last release: https://artifacts.dev.testing-farm.io/fa52afbc-5ddf-439b-9dfa-0f2d7595cced/

I will play with this for the next release.

In the past, the abidiff tests always timed out, but if it is working now, I think it makes sense to try to use it. Is there some way we can make this a gating test?

If you mean, blocking the update if rpminspect fails, yes, it can be done adding it to the gating.yaml file as Tulio suggested.

Notice there were some Unicode issues on clang last release: https://artifacts.dev.testing-farm.io/fa52afbc-5ddf-439b-9dfa-0f2d7595cced/

Sorry that's my bad. It's a known (expected) failure and since we weren't requiring rpminspect to be green, I've neglected it for too long. This PR should fix the issue: https://github.com/rpminspect/rpminspect-data-fedora/pull/46

It's a known (expected) failure and since we weren't requiring rpminspect to be green, I've neglected it for too long. This PR should fix the issue: https://github.com/rpminspect/rpminspect-data-fedora/pull/46

Awesome! Is there a way to ignore this error from within this repository? That way we won't have to add a new line for every Fedora release.

Awesome! Is there a way to ignore this error from within this repository? That way we won't have to add a new line for every Fedora release.

Unfortunately, unicode is a security inspection and hence it needs to be skipped this way. Check out https://www.burdell.org/posts/2023/Apr/19/security-checks-in-rpminspect/.

OK.

In that case, I'm going to close both PRs now.
I have patches for llvm and clang that I plan to submit after building 17.0.2.

Pull-Request has been closed by tuliom

7 months ago