On Tue, Apr 04, 2017 at 02:36:48PM -0700, Laura Abbott wrote:
> From: Laura Abbott <labb...@redhat.com>
>
> 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.
> ---
> Bringing this out from the previous thread. This drops the special invocations
> of find-debuginfo.sh and debugedit. The results seem to be reasonable as far
> as I can tell.
Looking at the /usr/lib/rpm/macros and the find_debuginfo stuff in
particular, those pieces of your patch makes sense to drop and replace with
the find_debuginfo_opts[1].
I am curious about the AFTER_LINK patch. What is the reason to drop it?
The patch probably needs to be dropped as either stale or integrated
differently upstream, just thought it would be nice to have it documented.
Can I assume your testing was install the debuginfo package and have an
application like gdb or crash read in the symbols to verify the contents?
I think this patch is a good direction forward, just a little nitpick in
[1].
Cheers,
Don
[1] - Adding the VERSION and RELEASE stuff to find_debuginfo_opts must have
been painful. It also makes it hard to read. I was curious if a macro
would work there, where we pass a list of files and the macro spits out the
files with the VERSION, RELEASE, _arch, _debug junk appended to it?
This might make it easier to add to later and maintain?
> ---
> kbuild-AFTER_LINK.patch | 126
> ------------------------------------------------
> kernel.spec | 40 +++------------
> 2 files changed, 6 insertions(+), 160 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 ab738c6..0000000
> --- a/kbuild-AFTER_LINK.patch
> +++ /dev/null
> @@ -1,126 +0,0 @@
> -From 649d991ca7737dd227f2a1ca4f30247daf6a7b4b Mon Sep 17 00:00:00 2001
> -From: Roland McGrath <rol...@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 <rol...@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 19a8ad1..b7653e4 100644
> --- a/kernel.spec
> +++ b/kernel.spec
> @@ -183,9 +183,6 @@ Summary: The Linux kernel
> %define _enable_debug_packages 0
> %endif
> %define debuginfodir /usr/lib/debug
> -# Needed because we override almost everything involving build-ids
> -# and debuginfo generation. Currently we rely on the old alldebug setting.
> -%global _build_id_links alldebug
>
> # kernel PAE is only built on i686 and ARMv7.
> %ifnarch i686 armv7hl
> @@ -395,7 +392,8 @@ BuildRequires: pciutils-devel gettext ncurses-devel
> BuildConflicts: rhbuildsys(DiskFree) < 500Mb
> %if %{with_debuginfo}
> BuildRequires: rpm-build, elfutils
> -%define debuginfo_args --strict-build-id -r
> +%undefine _include_minidebuginfo
> +%undefine _find_debuginfo_dwz_opts
> %endif
>
> %if %{signkernel}%{signmodules}
> @@ -492,9 +490,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 +700,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-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.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 +721,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-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?|XXX'
> -o python-perf-debuginfo.list}
>
>
> %endif # with_perf
> @@ -781,7 +776,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-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?|.*%%{_bindir}/powernow-k8-decode-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?|.*%%{_bindir}/cpupower-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?|.*%%{_libdir}/libcpupower.*|.*%%{_bindir}/turbostat-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?|.*%%{_bindir}/x86_energy_perf_policy-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?|.*%%{_bindir}/tmon-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?|.*%%{_bindir}/lsgpio-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?|.*%%{_bindir}/gpio-hammer-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?|.*%%{_bindir}/gpio-event-mon-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?|.*%%{_bindir}/iio_event_monitor-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?|.*%%{_bindir}/iio_generic_buffer-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?|.*%%{_bindir}/lsiio-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?|XXX'
> -o kernel-tools-debuginfo.list}
>
> %endif # with_tools
>
> @@ -801,7 +796,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}}-%%{VERSION}-%%{RELEASE}.%%{_arch}(\.debug)?'
> -o debuginfo%{?1}.list}\
> %{nil}
>
> #
> @@ -1282,18 +1277,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 +1488,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 +1723,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}
> --
> 2.7.4
> _______________________________________________
> kernel mailing list -- kernel@lists.fedoraproject.org
> To unsubscribe send an email to kernel-le...@lists.fedoraproject.org
_______________________________________________
kernel mailing list -- kernel@lists.fedoraproject.org
To unsubscribe send an email to kernel-le...@lists.fedoraproject.org
