Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bcc_elf: add support for debug information from libdebuginfod #3393

Merged
merged 2 commits into from May 9, 2021

Conversation

rupran
Copy link
Contributor

@rupran rupran commented Apr 30, 2021

This change adds debuginfod as a new source for debug information. By using libdebuginfod we can query a server for a file containing debug information for a given ELF binary. The environment variable DEBUGINFOD_URLS has to be defined to an URL for a debuginfod server providing debug information files for your distribution or the federating server provided by the elfutils project:

For example, to use the Fedora server, you would need:

$ export DEBUGINFOD_URLS="https://debuginfod.fedoraproject.org/"

Or for the elfutils server which federates to servers for openSUSE, Void Linux, Debian and Fedora, among others:

$ export DEBUGINFOD_URLS="https://debuginfod.elfutils.org/"

Calls to the debuginfod_find_debuginfo function from libdebuginfod will fail if the environment variable is not set, otherwise the library will attempt to download debug information for a build ID extracted from the binary in question and store it in a local cache directory.

Fixes bpftrace/bpftrace#1774

While this first implementation works fine with and without libdebuginfod installed, I'm not sure about the way it should be integrated into the build system. For now, a check for the library is performed in cmake/FindLibElf.cmake and if it was found, the HAVE_LIBDEBUGINFOD definition is added to the compile options for libbcc. In addition, I'm not sure if we should already put the dependency on libdebuginfod1 and libdebuginfod-dev into debian/control, as it is only available from buster-backports on Debian Buster. I'd be happy if someone with more experience could give me advice on these decisions.

@yonghong-song
Copy link
Collaborator

[buildbot, test this please]

@rupran
Copy link
Contributor Author

rupran commented May 1, 2021

As I suspected, the checks fail because none of the build targets (Fedora 25-28 and Ubuntu 16.04, 17.10 and 18.04) have packages for libdebuginfod.

How should we handle this additional dependency? Alternatives I can see are (a) leaving it out of the packaging scripts until libdebuginfod is more wide-spread and keeping it as a from-source-only feature until then, or (b) creating variants of the packaging files to account for different distributions... or is there another preferable way?

@fche
Copy link

fche commented May 3, 2021

fedora 25-28 are several years out of support; fedora 32 (oldest one supported) does have debuginfod

not sure about ubuntu, but bionic 18.* appears to have PPAs for fresh elfutils, and ubuntu groovy 20.* includes new enough elfutils

Copy link
Collaborator

@yonghong-song yonghong-song left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree that using libdebuginfod to hook with external source is a good idea and some production environment may already did that.

I suggest we break the patch into two parts. The first is to ensure if the host has libdebuginfod and build from source, it works. The second is to do package thing. I have some suggestions how to do spec file and rpmbuild command line etc. Thanks!

@@ -57,7 +57,7 @@ make install/strip DESTDIR=%{buildroot}

%package -n libbcc
Summary: Shared Library for BPF Compiler Collection (BCC)
Requires: elfutils-libelf
Requires: elfutils-libelf elfutils-debuginfod-client
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a rpm build expert either. But I think we can do something like

having_debuginfod = %{?debuginfod 1 }  %{!?debuginfod 0} 
%if having_debuginfod == 1
Requires: elfutils-debuginfod-client
%endif

And in bcc/scripts/ various build scripts, you can detect whether debuginfod is available or not, if available
add --define "debuginfod 1" to the rpmbuild command line.

Copy link
Contributor Author

@rupran rupran May 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option would be to do something like this in bcc.spec:

# Build with debuginfod support for Fedora >= 32
%if 0%{?fedora} >= 32
%bcond_with libdebuginfod
%else
%bcond_without libdebuginfod
%endif
[...]
%if %{with libdebuginfod}
BuildRequires: elfutils-debuginfod-client-devel
%endif
[...]
%package -n libbcc
Summary: Shared Library for BPF Compiler Collection (BCC)
Requires: elfutils-libelf
%if %{with libdebuginfod}
Requires: elfutils-debuginfod-client
%endif

I'm not familiar with the build process for the packages, are SPECS/bcc+clang.spec and scripts/build-release-rpm.sh used at all? It seems that most variable parts are in bcc.spec which is used by build-rpm.sh.

Edit: If I'm parsing the build logs from Fedora Koji correctly, the build process indeed seems to call rpmbuild with SPECS/bcc.spec so it would be sufficient to update that file only.
Edit 2: However, Fedora uses their own bcc.spec (see here) so this change would only affect the long-outdated Fedora versions in buildbot anyway. For inclusion downstream, I will probably file an issue or pull request with the changes described above directly for the Fedora package sources once the debuginfod support is merged here.

@olsajiri, as you are the maintainer for bcc in Fedora, is that fine with you?

Unfortunately, debian/control does not have any similar options to dynamically modify the package build process/dependencies, so we would need some kind of template as a base and replace the options from build-deb.sh, if I understand correctly. In the end, this should also only affect the local builds as the Ubuntu maintainers also use their own debian/control and we could simply leave debian/control as it is for now.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with the build process for the packages, are SPECS/bcc+clang.spec and scripts/build-release-rpm.sh used at all? It seems that most variable parts are in bcc.spec which is used by build-rpm.sh.
I think buildbot may use them. Fedora may use different ways to build, I suspect.

For debian/control, not sure who used it. May be fine to leave it, but some comments in the file will be still good.

@rupran rupran requested a review from yonghong-song May 6, 2021 10:57
This change adds debuginfod as a new source for debug
information. By using libdebuginfod we can query a server
for a file containing debug information for a given ELF
binary. The environment variable DEBUGINFOD_URLS has to
be defined to an URL for a debuginfod server providing
debug information files for your distribution or the
federating server provided by the elfutils project:

For example, to use the Fedora server, you would need:
$ export DEBUGINFOD_URLS="https://debuginfod.fedoraproject.org/"

Or for the elfutils server which federates to servers for
openSUSE, Void Linux, Debian and Fedora, among others:
$ export DEBUGINFOD_URLS="https://debuginfod.elfutils.org/"

Calls to the debuginfod_find_debuginfo function from
libdebuginfod will fail if the environment variable is not
set, otherwise the library will attempt to download debug
information for a build ID extracted from the binary in
question and store it in a local cache directory.

Fixes bpftrace/bpftrace#1774

Signed-off-by: Andreas Ziegler <andreas.ziegler@fau.de>
@yonghong-song
Copy link
Collaborator

[buildbot, test this please]

@yonghong-song
Copy link
Collaborator

Your change in spec file seems not working, I see

+ . scripts/git-tag.sh
+++ git describe --tags --abbrev=0
++ git_tag_latest=v0.20.0
+++ git rev-list v0.20.0.. --count
++ git_rev_count=3
++ git_rev_count=4
+++ git log --pretty=%s -n 1
++ git_subject='Merge 3dc5e673402f529dc6dfd6b45c148a7818c3ef87 into 14278bf1a52dd76ff66eed02cc9db7c7ec240da6'
++ release=4
++ [[ 4 != \1 ]]
+++ git log --pretty=%h -n 1
++ release=4.git.86ca029a
++ revision=0.20.0
+ git archive HEAD --prefix=bcc/ --format=tar -o /tmp/rpmbuild.Y1jl5G/SOURCES/bcc.tar
+ pushd src/cc/libbpf
~/jenkins/workspace/bcc-pr/label/fc26/src/cc/libbpf ~/jenkins/workspace/bcc-pr/label/fc26
+ git archive HEAD --prefix=bcc/src/cc/libbpf/ --format=tar -o /tmp/rpmbuild.Y1jl5G/SOURCES/bcc_libbpf.tar
+ popd
~/jenkins/workspace/bcc-pr/label/fc26
+ pushd /tmp/rpmbuild.Y1jl5G/SOURCES
/tmp/rpmbuild.Y1jl5G/SOURCES ~/jenkins/workspace/bcc-pr/label/fc26
+ tar -A -f bcc.tar bcc_libbpf.tar
+ gzip bcc.tar
+ popd
~/jenkins/workspace/bcc-pr/label/fc26
+ sed -e 's/^\(Version:\s*\)@REVISION@/\10.20.0/' -e 's/^\(Release:\s*\)@GIT_REV_COUNT@/\14.git.86ca029a/' SPECS/bcc.spec
+ pushd /tmp/rpmbuild.Y1jl5G
/tmp/rpmbuild.Y1jl5G ~/jenkins/workspace/bcc-pr/label/fc26
++ pwd
+ rpmbuild --define '_topdir /tmp/rpmbuild.Y1jl5G' -ba SPECS/bcc.spec
error: Failed build dependencies:
	elfutils-debuginfod-client-devel is needed by bcc-0.20.0-4.git.86ca029a.x86_64
+ cleanup
+ [[ -d /tmp/rpmbuild.Y1jl5G ]]
+ rm -rf /tmp/rpmbuild.Y1jl5G
Build step 'Conditional step (single)' marked build as failure
Taking single-use slave fedora26-slave-85f offline.
Finished: FAILURE

@fche
Copy link

fche commented May 7, 2021

We usually use a syntax more like

%if 0%{?fedora} >= 32
BuildRequire: ....
%endif

@yonghong-song
Copy link
Collaborator

[buildbot, ok to test]

@rupran
Copy link
Contributor Author

rupran commented May 7, 2021

Your change in spec file seems not working, I see

Looks like I fell for the classic .spec pitfall that bcond_with implies having a build option but it sets the default for this option to disabled.

We usually use a syntax more like

%if 0%{?fedora} >= 32
BuildRequire: ....
%endif

I just went with the syntax similar to what's already in the file, but I can change this if you'd like.

On Fedora builds we can check the version number and add
build and runtime dependencies to debuginfod for all
currently supported releases (>= 32). Note that the buildbot
only has Fedora 25-28 so it will not try to build libbcc
with debuginfod support as the required packages are not
available on these releases.

For .deb packages there is no easy way to add dependencies
dynamically, so we do not add dependencies to libdebuginfod
there for now. For documentation purposes, however, let's
add a comment indicating which changes are required for
libdebuginfod support for downstream maintainers.

Signed-off-by: Andreas Ziegler <andreas.ziegler@fau.de>
@yonghong-song
Copy link
Collaborator

[buildbot, test this please]

1 similar comment
@yonghong-song
Copy link
Collaborator

[buildbot, test this please]

@yonghong-song yonghong-song merged commit 823321c into iovisor:master May 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically download user space debug symbols using a debuginfod service
3 participants