Once upon a time, the kernel needed a lot of special handling to generate proper debuginfo as the kernel was ahead in technology. These days, rpm has improved debuginfo support. The kernel has not kept up with this and it's forward looking calls are now out of date. Switch to more standard invocations of debuginfo calls. --- This is a slightly reduced version of the previous patch. It keeps the existing behavior by using the standard find-debuginfo.sh call with the proper undefines. --- kbuild-AFTER_LINK.patch | 126 ------------------------------------------------ kernel.spec | 43 +++++------------ 2 files changed, 12 insertions(+), 157 deletions(-) delete mode 100644 kbuild-AFTER_LINK.patch
diff --git a/kbuild-AFTER_LINK.patch b/kbuild-AFTER_LINK.patch deleted file mode 100644 index ab738c62..00000000 --- a/kbuild-AFTER_LINK.patch +++ /dev/null @@ -1,126 +0,0 @@ -From 649d991ca7737dd227f2a1ca4f30247daf6a7b4b Mon Sep 17 00:00:00 2001 -From: Roland McGrath roland@redhat.com -Date: Mon, 6 Oct 2008 23:03:03 -0700 -Subject: [PATCH] kbuild: AFTER_LINK - -If the make variable AFTER_LINK is set, it is a command line to run -after each final link. This includes vmlinux itself and vDSO images. - -Bugzilla: N/A -Upstream-status: ?? - -Signed-off-by: Roland McGrath roland@redhat.com ---- - arch/arm64/kernel/vdso/Makefile | 3 ++- - arch/powerpc/kernel/vdso32/Makefile | 3 ++- - arch/powerpc/kernel/vdso64/Makefile | 3 ++- - arch/s390/kernel/vdso32/Makefile | 3 ++- - arch/s390/kernel/vdso64/Makefile | 3 ++- - arch/x86/entry/vdso/Makefile | 5 +++-- - scripts/link-vmlinux.sh | 4 ++++ - 7 files changed, 17 insertions(+), 7 deletions(-) - -diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile -index 62c84f7..f44236a 100644 ---- a/arch/arm64/kernel/vdso/Makefile -+++ b/arch/arm64/kernel/vdso/Makefile -@@ -54,7 +54,8 @@ $(obj-vdso): %.o: %.S FORCE - - # Actual build commands - quiet_cmd_vdsold = VDSOL $@ -- cmd_vdsold = $(CC) $(c_flags) -Wl,-n -Wl,-T $^ -o $@ -+ cmd_vdsold = $(CC) $(c_flags) -Wl,-n -Wl,-T $^ -o $@ \ -+ $(if $(AFTER_LINK),;$(AFTER_LINK)) - quiet_cmd_vdsoas = VDSOA $@ - cmd_vdsoas = $(CC) $(a_flags) -c -o $@ $< - -diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile -index 78a7449..c9592c0 100644 ---- a/arch/powerpc/kernel/vdso32/Makefile -+++ b/arch/powerpc/kernel/vdso32/Makefile -@@ -44,7 +44,8 @@ $(obj-vdso32): %.o: %.S FORCE - - # actual build commands - quiet_cmd_vdso32ld = VDSO32L $@ -- cmd_vdso32ld = $(CROSS32CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) -+ cmd_vdso32ld = $(CROSS32CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) \ -+ $(if $(AFTER_LINK),; $(AFTER_LINK)) - quiet_cmd_vdso32as = VDSO32A $@ - cmd_vdso32as = $(CROSS32CC) $(a_flags) -c -o $@ $< - -diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile -index 31107bf..96aded3 100644 ---- a/arch/powerpc/kernel/vdso64/Makefile -+++ b/arch/powerpc/kernel/vdso64/Makefile -@@ -33,7 +33,8 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE - - # actual build commands - quiet_cmd_vdso64ld = VDSO64L $@ -- cmd_vdso64ld = $(CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) -+ cmd_vdso64ld = $(CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) \ -+ $(if $(AFTER_LINK),; $(AFTER_LINK)) - - # install commands for the unstripped file - quiet_cmd_vdso_install = INSTALL $@ -diff --git a/arch/s390/kernel/vdso32/Makefile b/arch/s390/kernel/vdso32/Makefile -index 6cc9478..94fb536 100644 ---- a/arch/s390/kernel/vdso32/Makefile -+++ b/arch/s390/kernel/vdso32/Makefile -@@ -46,7 +46,8 @@ $(obj-vdso32): %.o: %.S - - # actual build commands - quiet_cmd_vdso32ld = VDSO32L $@ -- cmd_vdso32ld = $(CC) $(c_flags) -Wl,-T $^ -o $@ -+ cmd_vdso32ld = $(CC) $(c_flags) -Wl,-T $^ -o $@ \ -+ $(if $(AFTER_LINK),; $(AFTER_LINK)) - quiet_cmd_vdso32as = VDSO32A $@ - cmd_vdso32as = $(CC) $(a_flags) -c -o $@ $< - -diff --git a/arch/s390/kernel/vdso64/Makefile b/arch/s390/kernel/vdso64/Makefile -index 2d54c18..a0e3e9d 100644 ---- a/arch/s390/kernel/vdso64/Makefile -+++ b/arch/s390/kernel/vdso64/Makefile -@@ -46,7 +46,8 @@ $(obj-vdso64): %.o: %.S - - # actual build commands - quiet_cmd_vdso64ld = VDSO64L $@ -- cmd_vdso64ld = $(CC) $(c_flags) -Wl,-T $^ -o $@ -+ cmd_vdso64ld = $(CC) $(c_flags) -Wl,-T $^ -o $@ \ -+ $(if $(AFTER_LINK),; $(AFTER_LINK)) - quiet_cmd_vdso64as = VDSO64A $@ - cmd_vdso64as = $(CC) $(a_flags) -c -o $@ $< - -diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile -index d540966..eeb47b6 100644 ---- a/arch/x86/entry/vdso/Makefile -+++ b/arch/x86/entry/vdso/Makefile -@@ -167,8 +167,9 @@ $(obj)/vdso32.so.dbg: FORCE \ - quiet_cmd_vdso = VDSO $@ - cmd_vdso = $(CC) -nostdlib -o $@ \ - $(VDSO_LDFLAGS) $(VDSO_LDFLAGS_$(filter %.lds,$(^F))) \ -- -Wl,-T,$(filter %.lds,$^) $(filter %.o,$^) && \ -- sh $(srctree)/$(src)/checkundef.sh '$(NM)' '$@' -+ -Wl,-T,$(filter %.lds,$^) $(filter %.o,$^) \ -+ $(if $(AFTER_LINK),; $(AFTER_LINK)) && \ -+ sh $(srctree)/$(src)/checkundef.sh '$(NM)' '$@' - - VDSO_LDFLAGS = -fPIC -shared $(call cc-ldoption, -Wl$(comma)--hash-style=both) \ - $(call cc-ldoption, -Wl$(comma)--build-id) -Wl,-Bsymbolic $(LTO_CFLAGS) -diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh -index f742c65..526eee4 100755 ---- a/scripts/link-vmlinux.sh -+++ b/scripts/link-vmlinux.sh -@@ -111,6 +111,10 @@ vmlinux_link() - -lutil -lrt -lpthread - rm -f linux - fi -+ if [ -n "${AFTER_LINK}" ]; then -+ /usr/lib/rpm/debugedit -b ${RPM_BUILD_DIR} -d /usr/src/debug -i ${2} \ -+ > ${2}.id -+ fi - } - - --- -2.7.4 - diff --git a/kernel.spec b/kernel.spec index 78f692e9..97d8d464 100644 --- a/kernel.spec +++ b/kernel.spec @@ -395,7 +395,14 @@ BuildRequires: pciutils-devel gettext ncurses-devel BuildConflicts: rhbuildsys(DiskFree) < 500Mb %if %{with_debuginfo} BuildRequires: rpm-build, elfutils -%define debuginfo_args --strict-build-id -r +# Most of these should be enabled after more investigation +%undefine _include_minidebuginfo +%undefine _find_debuginfo_dwz_opts +%undefine _unique_build_ids +%undefine _unique_debug_names +%undefine _unique_debug_srcs +%global _find_debuginfo_opts -r +%global _missing_build_ids_terminate_build 1 %endif
%if %{signkernel}%{signmodules} @@ -492,9 +499,6 @@ Source5000: patch-4.%{base_sublevel}-git%{gitrev}.xz
## Patches needed for building this package
-# build tweak for build ID magic, even for -vanilla -Patch001: kbuild-AFTER_LINK.patch - ## compile fixes
# ongoing complaint, full discussion delayed until ksummit/plumbers @@ -705,7 +709,7 @@ This package provides debug information for the perf package. # symlinks because of the trailing nonmatching alternation and # the leading .*, because of find-debuginfo.sh's buggy handling # of matching the pattern against the symlinks file. -%{expand:%%global debuginfo_args %{?debuginfo_args} -p '.*%%{_bindir}/perf(.debug)?|.*%%{_libexecdir}/perf-core/.*|.*%%{_libdir}/traceevent/plugins/.*|XXX' -o perf-debuginfo.list} +%{expand:%%global _find_debuginfo_opts %{?_find_debuginfo_opts} -p '.*%%{_bindir}/perf(.debug)?|.*%%{_libexecdir}/perf-core/.*|.*%%{_libdir}/traceevent/plugins/.*|XXX' -o perf-debuginfo.list}
%package -n python-perf Summary: Python bindings for apps which will manipulate perf events @@ -726,7 +730,7 @@ AutoReqProv: no This package provides debug information for the perf python bindings.
# the python_sitearch macro should already be defined from above -%{expand:%%global debuginfo_args %{?debuginfo_args} -p '.*%%{python_sitearch}/perf.so(.debug)?|XXX' -o python-perf-debuginfo.list} +%{expand:%%global _find_debuginfo_opts %{?_find_debuginfo_opts} -p '.*%%{python_sitearch}/perf.so(.debug)?|XXX' -o python-perf-debuginfo.list}
%endif # with_perf @@ -781,7 +785,7 @@ This package provides debug information for package kernel-tools. # symlinks because of the trailing nonmatching alternation and # the leading .*, because of find-debuginfo.sh's buggy handling # of matching the pattern against the symlinks file. -%{expand:%%global debuginfo_args %{?debuginfo_args} -p '.*%%{_bindir}/centrino-decode(.debug)?|.*%%{_bindir}/powernow-k8-decode(.debug)?|.*%%{_bindir}/cpupower(.debug)?|.*%%{_libdir}/libcpupower.*|.*%%{_bindir}/turbostat(.debug)?|.*%%{_bindir}/x86_energy_perf_policy(.debug)?|.*%%{_bindir}/tmon(.debug)?|.*%%{_bindir}/lsgpio(.debug)?|.*%%{_bindir}/gpio-hammer(.debug)?|.*%%{_bindir}/gpio-event-mon(.debug)?|.*%%{_bindir}/iio_event_monitor(.debug)?|.*%%{_bindir}/iio_generic_buffer(.debug)?|.*%%{_bindir}/lsiio(.debug)?|XXX' -o kernel-tools-debuginfo.list} +%{expand:%%global _find_debuginfo_opts %{?_find_debuginfo_opts} -p '.*%%{_bindir}/centrino-decode(.debug)?|.*%%{_bindir}/powernow-k8-decode(.debug)?|.*%%{_bindir}/cpupower(.debug)?|.*%%{_libdir}/libcpupower.*|.*%%{_bindir}/turbostat(.debug)?|.*%%{_bindir}/x86_energy_perf_policy(.debug)?|.*%%{_bindir}/tmon(.debug)?|.*%%{_bindir}/lsgpio(.debug)?|.*%%{_bindir}/gpio-hammer(.debug)?|.*%%{_bindir}/gpio-event-mon(.debug)?|.*%%{_bindir}/iio_event_monitor(.debug)?|.*%%{_bindir}/iio_generic_buffer(.debug)?|.*%%{_bindir}/lsiio(.debug)?|XXX' -o kernel-tools-debuginfo.list}
%endif # with_tools
@@ -801,7 +805,7 @@ AutoReqProv: no\ %description %{?1:%{1}-}debuginfo\ This package provides debug information for package %{name}%{?1:-%{1}}.\ This is required to use SystemTap with %{name}%{?1:-%{1}}-%{KVERREL}.\ -%{expand:%%global debuginfo_args %{?debuginfo_args} -p '/.*/%%{KVERREL}%{?1:[+]%{1}}/.*|/.*%%{KVERREL}%{?1:+%{1}}(.debug)?' -o debuginfo%{?1}.list}\ +%{expand:%%global _find_debuginfo_opts %{?_find_debuginfo_opts} -p '/.*/%%{KVERREL}%{?1:[+]%{1}}/.*|/.*%%{KVERREL}%{?1:+%{1}}(.debug)?' -o debuginfo%{?1}.list}\ %{nil}
# @@ -1282,18 +1286,6 @@ cd .. %define sparse_mflags C=1 %endif
-%if %{with_debuginfo} -# This override tweaks the kernel makefiles so that we run debugedit on an -# object before embedding it. When we later run find-debuginfo.sh, it will -# run debugedit again. The edits it does change the build ID bits embedded -# in the stripped object, but repeating debugedit is a no-op. We do it -# beforehand to get the proper final build ID bits into the embedded image. -# This affects the vDSO images in vmlinux, and the vmlinux image in bzImage. -export AFTER_LINK=\ -'sh -xc "/usr/lib/rpm/debugedit -b $$RPM_BUILD_DIR -d /usr/src/debug \ - -i $@ > $@.id"' -%endif - cp_vmlinux() { eu-strip --remove-comment -o "$2" "$1" @@ -1505,13 +1497,6 @@ BuildKernel() { cp $RPM_BUILD_ROOT/lib/modules/$KernelVer/build/.config $RPM_BUILD_ROOT/lib/modules/$KernelVer/build/include/config/auto.conf
%if %{with_debuginfo} - if test -s vmlinux.id; then - cp vmlinux.id $RPM_BUILD_ROOT/lib/modules/$KernelVer/build/vmlinux.id - else - echo >&2 "*** ERROR *** no vmlinux build ID! ***" - exit 1 - fi - # # save the vmlinux file for kernel debugging into the kernel-debuginfo rpm # @@ -1747,10 +1732,6 @@ popd
%if %{with_debuginfo}
-%define __debug_install_post \ - /usr/lib/rpm/find-debuginfo.sh %{debuginfo_args} %{_builddir}/%{?buildsubdir}\ -%{nil} - %ifnarch noarch %global __debug_package 1 %files -f debugfiles.list debuginfo-common-%{_target_cpu}
On Mon, Apr 10, 2017 at 06:22:06PM -0700, Laura Abbott wrote:
Once upon a time, the kernel needed a lot of special handling to generate proper debuginfo as the kernel was ahead in technology. These days, rpm has improved debuginfo support. The kernel has not kept up with this and it's forward looking calls are now out of date. Switch to more standard invocations of debuginfo calls.
This is a slightly reduced version of the previous patch. It keeps the existing behavior by using the standard find-debuginfo.sh call with the proper undefines.
Thanks Laura! Sorry about the previous version.
Acked-by: Don Zickus dzickus@redhat.com
Mark W.,
Do you have much insight into how the below definitions would interact with the kernel?
@@ -395,7 +395,14 @@ BuildRequires: pciutils-devel gettext ncurses-devel BuildConflicts: rhbuildsys(DiskFree) < 500Mb %if %{with_debuginfo} BuildRequires: rpm-build, elfutils -%define debuginfo_args --strict-build-id -r +# Most of these should be enabled after more investigation +%undefine _include_minidebuginfo +%undefine _find_debuginfo_dwz_opts +%undefine _unique_build_ids +%undefine _unique_debug_names +%undefine _unique_debug_srcs +%global _find_debuginfo_opts -r +%global _missing_build_ids_terminate_build 1 %endif
I am only poking you because it seems like you spent a lot of time cleaning up some of the rpm macros in the rpm pkg.
Cheers, Don
Hi,
On Tue, 2017-04-11 at 11:01 -0400, Don Zickus wrote:
Mark W.,
Do you have much insight into how the below definitions would interact with the kernel?
@@ -395,7 +395,14 @@ BuildRequires: pciutils-devel gettext ncurses-devel BuildConflicts: rhbuildsys(DiskFree) < 500Mb %if %{with_debuginfo} BuildRequires: rpm-build, elfutils -%define debuginfo_args --strict-build-id -r
I am happy you seem able to use the defines instead of having to override the whole find-debuginfo call. Please let me know if there are other defines you need/want.
+# Most of these should be enabled after more investigation +%undefine _include_minidebuginfo
This prevents adding a .gnu_debugdata section to the binaries which contains a minisymtab with just the function symbols/addresses. This is not really relevant for the kernel and modules. If added to binaries/shared libraries it allows some tools to show function names for more addresses even without having any debuginfo installed.
+%undefine _find_debuginfo_dwz_opts
This prevents running dwz (the DWARF compressor) which doesn't work for the kernel and kernel modules. The problem is that kernel modules are not normal ET_EXEC or ET_DYN, but look like ET_REL object files. object files contain relocations between ELF sections that dwz cannot handle. It would be nice to fix this because kernel modules are ideal for DWARF deduplication. They all contain the same basic debug type information that could be shared. eu-strip --reloc-debug-sections (-r, see below) could help with this, but hasn't been integrated. I'll try to make some time (no promises!) to look at it, because getting dwz working against the kernel modules would probably provide a huge saving.
+%undefine _unique_build_ids +%undefine _unique_debug_names +%undefine _unique_debug_srcs
It is somewhat unfortunate these don't work with the kernel build. The idea is that the package nvra is used to make all of the above unique between all versions of the package/debuginfo.
If I understand correctly we "uniqueify" too late assuming nothing depends on the build-id till we start handling the debuginfo. But the kernel builds seem to already pick out build-ids to store and embeds some ELF images in other ELF images and then those build-ids shouldn't be touched anymore. The AFTER_LINK hack was used to counter this, but it is somewhat fragile/doesn't really seem to interact correctly anymore.
If you don't have _unique_build_ids you should also set: %global _build_id_links alldebug to make sure build-ids are only used for the debuginfo packages (otherwise your main packages stops being parallel installable, which the kernel needs).
I think _unique_debug_names and _unique_debug_srcs should in theory still work. But they aren't very useful if the build-ids aren't unique.
+%global _find_debuginfo_opts -r
This was specifically designed for the kernel. It makes sure relocations between debug sections are removed in ET_REL files. Which normally would not be correct if those ET_REL object files would be linked together again. But kernel modules are not real/normal object files. It should save a couple of 100MBs when installing the debuginfo for the .ko files. (As said above we really should figure out a way to combine this with dwz processing so we can safe even more space.)
+%global _missing_build_ids_terminate_build 1
Yes please. It should show if any ELF file rpm processes doesn't have a build-id, which would be a bug in the toolchain.
I am only poking you because it seems like you spent a lot of time cleaning up some of the rpm macros in the rpm pkg.
Thanks. I am really interested in this. I did see one of the earlier patches, but missed that I had been CCed on the followups. My email setup is a bit of a mess currently. My employer recently decided on short notice that my engineering group didn't need normal email, removed all our filters and forced us to adopt a "email provider" that "helpfully" removes any emails it believes you have already received and that hides everything behind a proprietary web frontend. Sigh. So I had to switch email addresses. Sorry.
Cheers,
Mark
On 04/12/2017 03:55 AM, Mark Wielaard wrote:
Hi,
On Tue, 2017-04-11 at 11:01 -0400, Don Zickus wrote:
Mark W.,
Do you have much insight into how the below definitions would interact with the kernel?
@@ -395,7 +395,14 @@ BuildRequires: pciutils-devel gettext ncurses-devel BuildConflicts: rhbuildsys(DiskFree) < 500Mb %if %{with_debuginfo} BuildRequires: rpm-build, elfutils -%define debuginfo_args --strict-build-id -r
I am happy you seem able to use the defines instead of having to override the whole find-debuginfo call. Please let me know if there are other defines you need/want.
I brought this up in the previous version but a macro for the default arguments to --ver-rel and --unique-debug-arch would be useful for packages like kernel that do filtering with -p. My previous solution required knowing what the arguments are to filter.
+# Most of these should be enabled after more investigation +%undefine _include_minidebuginfo
This prevents adding a .gnu_debugdata section to the binaries which contains a minisymtab with just the function symbols/addresses. This is not really relevant for the kernel and modules. If added to binaries/shared libraries it allows some tools to show function names for more addresses even without having any debuginfo installed.
This was explicitly causing problems for me when I was working. I think it was because it expected to find dynamic symbols and there were none.
+%undefine _find_debuginfo_dwz_opts
This prevents running dwz (the DWARF compressor) which doesn't work for the kernel and kernel modules. The problem is that kernel modules are not normal ET_EXEC or ET_DYN, but look like ET_REL object files. object files contain relocations between ELF sections that dwz cannot handle. It would be nice to fix this because kernel modules are ideal for DWARF deduplication. They all contain the same basic debug type information that could be shared. eu-strip --reloc-debug-sections (-r, see below) could help with this, but hasn't been integrated. I'll try to make some time (no promises!) to look at it, because getting dwz working against the kernel modules would probably provide a huge saving.
+%undefine _unique_build_ids +%undefine _unique_debug_names +%undefine _unique_debug_srcs
It is somewhat unfortunate these don't work with the kernel build. The idea is that the package nvra is used to make all of the above unique between all versions of the package/debuginfo.
These were working from what I could see, there was just some discussion about why we didn't have it for everything so I went with a smaller change first.
If I understand correctly we "uniqueify" too late assuming nothing depends on the build-id till we start handling the debuginfo. But the kernel builds seem to already pick out build-ids to store and embeds some ELF images in other ELF images and then those build-ids shouldn't be touched anymore. The AFTER_LINK hack was used to counter this, but it is somewhat fragile/doesn't really seem to interact correctly anymore.
Thanks for the history. I'm a bit concerned we shouldn't actually be dropping the AFTER_LINK call even if everything seems to be generated okay. Do you have a suggestion of something more specific I can check besides throwing selected modules into gdb?
I'm really tempted to just throw this patch in and see what happens but I also don't want to completely break anything unless I have to.
If you don't have _unique_build_ids you should also set: %global _build_id_links alldebug to make sure build-ids are only used for the debuginfo packages (otherwise your main packages stops being parallel installable, which the kernel needs).
Yes, that's still there after you suggested it before.
I think _unique_debug_names and _unique_debug_srcs should in theory still work. But they aren't very useful if the build-ids aren't unique.
+%global _find_debuginfo_opts -r
This was specifically designed for the kernel. It makes sure relocations between debug sections are removed in ET_REL files. Which normally would not be correct if those ET_REL object files would be linked together again. But kernel modules are not real/normal object files. It should save a couple of 100MBs when installing the debuginfo for the .ko files. (As said above we really should figure out a way to combine this with dwz processing so we can safe even more space.)
+%global _missing_build_ids_terminate_build 1
Yes please. It should show if any ELF file rpm processes doesn't have a build-id, which would be a bug in the toolchain.
I am only poking you because it seems like you spent a lot of time cleaning up some of the rpm macros in the rpm pkg.
Thanks. I am really interested in this. I did see one of the earlier patches, but missed that I had been CCed on the followups. My email setup is a bit of a mess currently. My employer recently decided on short notice that my engineering group didn't need normal email, removed all our filters and forced us to adopt a "email provider" that "helpfully" removes any emails it believes you have already received and that hides everything behind a proprietary web frontend. Sigh. So I had to switch email addresses. Sorry.
No problem. Thanks for replying.
Cheers,
Mark _______________________________________________ kernel mailing list -- kernel@lists.fedoraproject.org To unsubscribe send an email to kernel-leave@lists.fedoraproject.org
On Wed, 2017-04-12 at 10:20 -0700, Laura Abbott wrote:
On 04/12/2017 03:55 AM, Mark Wielaard wrote:
I am happy you seem able to use the defines instead of having to override the whole find-debuginfo call. Please let me know if there are other defines you need/want.
I brought this up in the previous version but a macro for the default arguments to --ver-rel and --unique-debug-arch would be useful for packages like kernel that do filtering with -p. My previous solution required knowing what the arguments are to filter.
Aha. I see what you are doing. You are creating separate debuginfo file lists for the separate subpackages. And with unique debug name it gets hard to predict which .debug file names to match.
The good news is that we are working on doing that automagically from rpmbuild (and then you also get a separate debugsources package). But I have to see if it will match your specific split.
For now I would indeed recommend to turn off _unique_debug_names and _unique_debug_srcs to not have to rely on the exact ver-rel-arch triple it will generate.
+# Most of these should be enabled after more investigation +%undefine _include_minidebuginfo
This prevents adding a .gnu_debugdata section to the binaries which contains a minisymtab with just the function symbols/addresses. This is not really relevant for the kernel and modules. If added to binaries/shared libraries it allows some tools to show function names for more addresses even without having any debuginfo installed.
This was explicitly causing problems for me when I was working. I think it was because it expected to find dynamic symbols and there were none.
I'll do a kernel build with it enabled and see if I can figure out why it is complaining. The support is a little crude with some fairly generic sed matches that I can certainly believe might fail in various corner cases.
It isn't super important I think, so just leave it off for now.
If I understand correctly we "uniqueify" too late assuming nothing depends on the build-id till we start handling the debuginfo. But the kernel builds seem to already pick out build-ids to store and embeds some ELF images in other ELF images and then those build-ids shouldn't be touched anymore. The AFTER_LINK hack was used to counter this, but it is somewhat fragile/doesn't really seem to interact correctly anymore.
Thanks for the history. I'm a bit concerned we shouldn't actually be dropping the AFTER_LINK call even if everything seems to be generated okay. Do you have a suggestion of something more specific I can check besides throwing selected modules into gdb?
One tricky one is the build-id of the vdso. The vdso is inserted in the process by the kernel from the kernel image, the debugger can find the matching vdso.debug by checking the build-id (and following the symlink). You could check if the version found in a running process is the same as the one in the vdso.debug file on disk.
$ eu-unstrip -n -p $$ | grep vdso 0x7ffe4a89a000+0x2000 50d1ccf000162361f74a9d7c2ecea856f7881f07@0x7ffe4a89a340 . /usr/lib/debug/usr/lib/modules/3.10.0-514.10.2.el7.x86_64/vdso/vdso.so.debug [vdso: 29852]
$ eu-readelf -n /usr/lib/debug/lib/modules/`uname -r`/vdso/vdso.so.debug | grep "Build ID" Build ID: 50d1ccf000162361f74a9d7c2ecea856f7881f07
The above matches, but that is on RHEL. I guess I really should be running Fedora :)
To be absolutely sure I think you need an explicit way to tell not to touch build-ids ever. Currently it is hardcoded to always check and update them. There is no way to say that you only want to check they are there, but never to update them because you know what you are doing. I think it is this way because originally we couldn't rely on the toolchain to generate the build-ids correctly, so we would always use the debuginfo to calculate one. But now with _missing_build_ids_terminate_build you can rely on them I think (well not, if we want to uniqueify them against nvra build, but we are not doing that for the kernel for now). I'll propose some update upstream so you can set _do_not_update_build_ids or something.
The only issue with that is that because it is currently hard-coded to always update, that would only work for rawhide. Which would make the spec file fedora version specific. Is that a problem?
Cheers,
Mark
On 04/12/2017 01:16 PM, Mark Wielaard wrote:
On Wed, 2017-04-12 at 10:20 -0700, Laura Abbott wrote:
On 04/12/2017 03:55 AM, Mark Wielaard wrote:
I am happy you seem able to use the defines instead of having to override the whole find-debuginfo call. Please let me know if there are other defines you need/want.
I brought this up in the previous version but a macro for the default arguments to --ver-rel and --unique-debug-arch would be useful for packages like kernel that do filtering with -p. My previous solution required knowing what the arguments are to filter.
Aha. I see what you are doing. You are creating separate debuginfo file lists for the separate subpackages. And with unique debug name it gets hard to predict which .debug file names to match.
The good news is that we are working on doing that automagically from rpmbuild (and then you also get a separate debugsources package). But I have to see if it will match your specific split.
For now I would indeed recommend to turn off _unique_debug_names and _unique_debug_srcs to not have to rely on the exact ver-rel-arch triple it will generate.
+# Most of these should be enabled after more investigation +%undefine _include_minidebuginfo
This prevents adding a .gnu_debugdata section to the binaries which contains a minisymtab with just the function symbols/addresses. This is not really relevant for the kernel and modules. If added to binaries/shared libraries it allows some tools to show function names for more addresses even without having any debuginfo installed.
This was explicitly causing problems for me when I was working. I think it was because it expected to find dynamic symbols and there were none.
I'll do a kernel build with it enabled and see if I can figure out why it is complaining. The support is a little crude with some fairly generic sed matches that I can certainly believe might fail in various corner cases.
It isn't super important I think, so just leave it off for now.
If I understand correctly we "uniqueify" too late assuming nothing depends on the build-id till we start handling the debuginfo. But the kernel builds seem to already pick out build-ids to store and embeds some ELF images in other ELF images and then those build-ids shouldn't be touched anymore. The AFTER_LINK hack was used to counter this, but it is somewhat fragile/doesn't really seem to interact correctly anymore.
Thanks for the history. I'm a bit concerned we shouldn't actually be dropping the AFTER_LINK call even if everything seems to be generated okay. Do you have a suggestion of something more specific I can check besides throwing selected modules into gdb?
One tricky one is the build-id of the vdso. The vdso is inserted in the process by the kernel from the kernel image, the debugger can find the matching vdso.debug by checking the build-id (and following the symlink). You could check if the version found in a running process is the same as the one in the vdso.debug file on disk.
$ eu-unstrip -n -p $$ | grep vdso 0x7ffe4a89a000+0x2000 50d1ccf000162361f74a9d7c2ecea856f7881f07@0x7ffe4a89a340 . /usr/lib/debug/usr/lib/modules/3.10.0-514.10.2.el7.x86_64/vdso/vdso.so.debug [vdso: 29852]
$ eu-readelf -n /usr/lib/debug/lib/modules/`uname -r`/vdso/vdso.so.debug | grep "Build ID" Build ID: 50d1ccf000162361f74a9d7c2ecea856f7881f07
The above matches, but that is on RHEL. I guess I really should be running Fedora :)
Yeah it doesn't match with my patches, so much for my optimism :(
To be absolutely sure I think you need an explicit way to tell not to touch build-ids ever. Currently it is hardcoded to always check and update them. There is no way to say that you only want to check they are there, but never to update them because you know what you are doing. I think it is this way because originally we couldn't rely on the toolchain to generate the build-ids correctly, so we would always use the debuginfo to calculate one. But now with _missing_build_ids_terminate_build you can rely on them I think (well not, if we want to uniqueify them against nvra build, but we are not doing that for the kernel for now). I'll propose some update upstream so you can set _do_not_update_build_ids or something.
Sounds good.
The only issue with that is that because it is currently hard-coded to always update, that would only work for rawhide. Which would make the spec file fedora version specific. Is that a problem?
I don't think so as long as everything follows the usual branching policy.
I'm going to look at a v3 that keeps the AFTER_LINK but still moves to a more default invocation of find-debuginfo.sh
Cheers,
Mark
Thanks, Laura
On Thu, Apr 13, 2017 at 06:59:38PM -0700, Laura Abbott wrote:
This was explicitly causing problems for me when I was working. I think it was because it expected to find dynamic symbols and there were none.
I'll do a kernel build with it enabled and see if I can figure out why it is complaining. The support is a little crude with some fairly generic sed matches that I can certainly believe might fail in various corner cases.
It isn't super important I think, so just leave it off for now.
Found the issue and posted a patch upstream: http://lists.rpm.org/pipermail/rpm-maint/2017-April/005441.html That makes sure the minisymtab support skips kernel modules, which is what generated the errors.
One tricky one is the build-id of the vdso. The vdso is inserted in the process by the kernel from the kernel image, the debugger can find the matching vdso.debug by checking the build-id (and following the symlink). You could check if the version found in a running process is the same as the one in the vdso.debug file on disk.
$ eu-unstrip -n -p $$ | grep vdso 0x7ffe4a89a000+0x2000 50d1ccf000162361f74a9d7c2ecea856f7881f07@0x7ffe4a89a340 . /usr/lib/debug/usr/lib/modules/3.10.0-514.10.2.el7.x86_64/vdso/vdso.so.debug [vdso: 29852]
$ eu-readelf -n /usr/lib/debug/lib/modules/`uname -r`/vdso/vdso.so.debug | grep "Build ID" Build ID: 50d1ccf000162361f74a9d7c2ecea856f7881f07
The above matches, but that is on RHEL. I guess I really should be running Fedora :)
Yeah it doesn't match with my patches, so much for my optimism :(
You need rpmbuild/debugedit to NOT touch build-ids ever. I wrote a patch for that so when a package sets %global _no_recompute_build_ids 1 it will not do that. Patch also submitted upstream: http://lists.rpm.org/pipermail/rpm-maint/2017-April/005442.html
With that the above does match for me.
When upstream accepts both patches I will add them to the fedora rpm. If you are interested in testing with them you can find a scratch build here: https://koji.fedoraproject.org/koji/taskinfo?taskID=19029567
I'm going to look at a v3 that keeps the AFTER_LINK but still moves to a more default invocation of find-debuginfo.sh
I think it would be really nice if we could drop the AFTER_LINK patch it looks really intrusive and messy. I think we can drop it with the above two patches to rpm. Then there are only 2 issues left as far as I can see:
1) The kbuild-AFTER_LINK.patch generated a file that contained vmlinux buildid in $RPM_BUILD_ROOT/lib/modules/$KernelVer/build/vmlinux.id With the _no_recompute_build_ids setting that should be easy to add back with some simple scripting (systemtap seems to check that file). eu-readelf -n vmlinux | grep "Build ID" | awk '{print $NF}' > vmlinux.id
2) The build-id symlinks for the kernel modules are not there. But... This is already broken in rawhide. I "fixed" rpmbuild to generate those after the file list was generated instead of through find-debuginfo.sh. This is too late for the kernel because it will xz compress all .ko files and rpmbuild doesn't know it should handle .ko.xz files too. Surprisingly this is already partly broken in f25. The build-id symlinks are there, but they point to non-existing files because after the symlink was created to the .ko file the file gets compressed to .ko.xz. I am slightly surprised this hasn't broken more things like systemtap which should rely on the build-ids. But it might fall back to finding the modules by name.
Anyway, this is something that was broken without the AFTER_LINK patch removal. So it needs a separate fix. I think I know a trick. Instead of just compressing each .ko file with xz we should add a script that at the same time generates the symlinks. I'll try to come up with a patch for that.
Cheers,
Mark
On Sun, Apr 16, 2017 at 08:04:18PM +0200, Mark Wielaard wrote:
Anyway, this is something that was broken without the AFTER_LINK patch removal. So it needs a separate fix. I think I know a trick. Instead of just compressing each .ko file with xz we should add a script that at the same time generates the symlinks. I'll try to come up with a patch for that.
Haha, I said that before realizing the intricate ways the files lists for core, modules, extra arch variants are generated. It seems it isn't as simple as I thought to augment those lists with the correct build-id symlinks. It looks like it would be easier, and nicer, to make rpmbuild do that by somehow teaching it to look into the .ko.xz files. The downside to that is that we then have to uncompress them again in rpm. Maybe we can do some automagic compression at the same time? So looks like I don't have a very simple solution yet.
BTW. Random observation. The kernel.spec does: find $RPM_BUILD_ROOT/lib/modules/ -type f -name '*.ko' | xargs xz Might it be efficient to do some of that in parallel? find $RPM_BUILD_ROOT/lib/modules/ -type f -name '*.ko' \ | xargs -n16 -P$(getconf _NPROCESSORS_ONLN) xz -T1 This assumes the .ko files are too small to parallel compress themselves.
Cheers,
Mark
On Sun, 2017-04-16 at 20:04 +0200, Mark Wielaard wrote:
Found the issue and posted a patch upstream: http://lists.rpm.org/pipermail/rpm-maint/2017-April/005441.html That makes sure the minisymtab support skips kernel modules, which is what generated the errors. [...] You need rpmbuild/debugedit to NOT touch build-ids ever. I wrote a patch for that so when a package sets %global _no_recompute_build_ids 1 it will not do that. Patch also submitted upstream: http://lists.rpm.org/pipermail/rpm-maint/2017-April/005442.html
With that the above does match for me.
When upstream accepts both patches I will add them to the fedora rpm. If you are interested in testing with them you can find a scratch build here: https://koji.fedoraproject.org/koji/taskinfo?taskID=19029567
Both patches were accepted upstream and I added them to the fedora rawhide rpm package (rpm-4.13.0.1-19.fc27).
Cheers,
Mark
On 04/19/2017 04:22 AM, Mark Wielaard wrote:
On Sun, 2017-04-16 at 20:04 +0200, Mark Wielaard wrote:
Found the issue and posted a patch upstream: http://lists.rpm.org/pipermail/rpm-maint/2017-April/005441.html That makes sure the minisymtab support skips kernel modules, which is what generated the errors. [...] You need rpmbuild/debugedit to NOT touch build-ids ever. I wrote a patch for that so when a package sets %global _no_recompute_build_ids 1 it will not do that. Patch also submitted upstream: http://lists.rpm.org/pipermail/rpm-maint/2017-April/005442.html
With that the above does match for me.
When upstream accepts both patches I will add them to the fedora rpm. If you are interested in testing with them you can find a scratch build here: https://koji.fedoraproject.org/koji/taskinfo?taskID=19029567
Both patches were accepted upstream and I added them to the fedora rawhide rpm package (rpm-4.13.0.1-19.fc27).
Cheers,
Mark
Thanks! I did a test with your scratch build and the results look good. I'll send a v3 based on that.
Thanks, Laura
On Wed, Apr 12, 2017 at 12:55:34PM +0200, Mark Wielaard wrote:
Hi,
On Tue, 2017-04-11 at 11:01 -0400, Don Zickus wrote:
Mark W.,
Do you have much insight into how the below definitions would interact with the kernel?
@@ -395,7 +395,14 @@ BuildRequires: pciutils-devel gettext ncurses-devel BuildConflicts: rhbuildsys(DiskFree) < 500Mb %if %{with_debuginfo} BuildRequires: rpm-build, elfutils -%define debuginfo_args --strict-build-id -r
I am happy you seem able to use the defines instead of having to override the whole find-debuginfo call. Please let me know if there are other defines you need/want.
Hi Mark,
Thanks for responding. Laura already provided some good feedback. I was curious if some of this would correctly for our non-kernel package stuff we build. I might be reading Laura's patch wrong, but I think some of the options applied to the kernel _and_ the userspace applications too.
Also, I was trying to understand Laura's previous patch with the -p changes. It looked like the output of the application's debuginfo binaries did not match with other programs. I was curious if you had some thoughts on that patch (it was split out of this one; you would have to see the original patch to understand).
Cheers, Don
+# Most of these should be enabled after more investigation +%undefine _include_minidebuginfo
This prevents adding a .gnu_debugdata section to the binaries which contains a minisymtab with just the function symbols/addresses. This is not really relevant for the kernel and modules. If added to binaries/shared libraries it allows some tools to show function names for more addresses even without having any debuginfo installed.
+%undefine _find_debuginfo_dwz_opts
This prevents running dwz (the DWARF compressor) which doesn't work for the kernel and kernel modules. The problem is that kernel modules are not normal ET_EXEC or ET_DYN, but look like ET_REL object files. object files contain relocations between ELF sections that dwz cannot handle. It would be nice to fix this because kernel modules are ideal for DWARF deduplication. They all contain the same basic debug type information that could be shared. eu-strip --reloc-debug-sections (-r, see below) could help with this, but hasn't been integrated. I'll try to make some time (no promises!) to look at it, because getting dwz working against the kernel modules would probably provide a huge saving.
+%undefine _unique_build_ids +%undefine _unique_debug_names +%undefine _unique_debug_srcs
It is somewhat unfortunate these don't work with the kernel build. The idea is that the package nvra is used to make all of the above unique between all versions of the package/debuginfo.
If I understand correctly we "uniqueify" too late assuming nothing depends on the build-id till we start handling the debuginfo. But the kernel builds seem to already pick out build-ids to store and embeds some ELF images in other ELF images and then those build-ids shouldn't be touched anymore. The AFTER_LINK hack was used to counter this, but it is somewhat fragile/doesn't really seem to interact correctly anymore.
If you don't have _unique_build_ids you should also set: %global _build_id_links alldebug to make sure build-ids are only used for the debuginfo packages (otherwise your main packages stops being parallel installable, which the kernel needs).
I think _unique_debug_names and _unique_debug_srcs should in theory still work. But they aren't very useful if the build-ids aren't unique.
+%global _find_debuginfo_opts -r
This was specifically designed for the kernel. It makes sure relocations between debug sections are removed in ET_REL files. Which normally would not be correct if those ET_REL object files would be linked together again. But kernel modules are not real/normal object files. It should save a couple of 100MBs when installing the debuginfo for the .ko files. (As said above we really should figure out a way to combine this with dwz processing so we can safe even more space.)
+%global _missing_build_ids_terminate_build 1
Yes please. It should show if any ELF file rpm processes doesn't have a build-id, which would be a bug in the toolchain.
I am only poking you because it seems like you spent a lot of time cleaning up some of the rpm macros in the rpm pkg.
Thanks. I am really interested in this. I did see one of the earlier patches, but missed that I had been CCed on the followups. My email setup is a bit of a mess currently. My employer recently decided on short notice that my engineering group didn't need normal email, removed all our filters and forced us to adopt a "email provider" that "helpfully" removes any emails it believes you have already received and that hides everything behind a proprietary web frontend. Sigh. So I had to switch email addresses. Sorry.
Cheers,
Mark _______________________________________________ kernel mailing list -- kernel@lists.fedoraproject.org To unsubscribe send an email to kernel-leave@lists.fedoraproject.org
Hi Don,
On Wed, 2017-04-12 at 13:45 -0400, Don Zickus wrote:
Thanks for responding. Laura already provided some good feedback. I was curious if some of this would correctly for our non-kernel package stuff we build. I might be reading Laura's patch wrong, but I think some of the options applied to the kernel _and_ the userspace applications too.
Yes, it would be nice if the user space sub-packages didn't use any of the "special settings". But at least the splitting of the sub-debuginfo package needs special handling for now (hopefully not needed in the future, see below). But the settings should really hurt either. The user space packages just wouldn't have have all the features they could have. But the mini symtab and dwz compression for those small tools are IMHO not major features.
Also, I was trying to understand Laura's previous patch with the -p changes. It looked like the output of the application's debuginfo binaries did not match with other programs. I was curious if you had some thoughts on that patch (it was split out of this one; you would have to see the original patch to understand).
Yes, that is for the subpackage debuginfo splitting. We are working on that in a more generic way for all packages, so hopefully that will all become easier in the future: https://fedoraproject.org/wiki/Changes/SubpackageAndSourceDebuginfo (Yes, that was originally proposed for Fedora 26, obviously we missed that...)
Cheers,
Mark
kernel@lists.fedoraproject.org
-
Don Zickus -
Laura Abbott -
Mark Wielaard -
Mark Wielaard