Re: [gentoo-portage-dev] [PATCH] estrip: rework hard link logic in save_elf_debug
On Wed, Oct 27, 2021 at 10:51 AM Alec Warner wrote: > Can we do better than X and Y here? The patch has gone through several revisions on Github. https://github.com/gentoo/portage/pull/763
Re: [gentoo-portage-dev] [PATCH] estrip: rework hard link logic in save_elf_debug
On Mon, Oct 25, 2021 at 10:10 AM Mike Gilbert wrote:
>
> GDB loads debug files based on the file name given in the .gnu_debuglink
> section, prepended with /usr/lib/debug/${dirname}, where dirname is the
> absolute path to the parent directory of the binary being executed.
>
> For each unique inode as input, we need a link to the debug file with
> the GNU debuglink as its basename. A link to the debug file should exist
> for each directory in which the input inode exists.
>
> The debug link names should be based on the .gnu_debuglink value instead
> of the name of the file we are processing as input.
>
> The .gnu_debuglink value is based on the name of the first link
> processed for each inode. We save this value as a symlink, and then read
> it back as we process subsequent links.
>
> For example, given the following input:
>
> INODE PATH
> 1 /usr/bin/git
> 1 /usr/libexec/git-core/git-add
> 2 /usr/bin/git-shell
> 2 /usr/libexec/git-core/git-shell
>
> We generate the following inodes for the debug files:
>
> INODE DEBUGLINK
> 3 git.debug
> 4 git-shell.debug
>
> We should generate the following links:
>
> INODE PATH
> 3 /usr/lib/debug/usr/bin/git.debug
> 3 /usr/lib/debug/usr/libexec/git-core/git.debug
> 4 /usr/bin/debug/usr/bin/git-shell.debug
> 4 /usr/bin/debug/usr/libexec/git-core/git-shell.debug
>
> The previous code would have generated this broken output:
>
> INODE PATH
> 3 /usr/lib/debug/usr/bin/git.debug
> 3 /usr/lib/debug/usr/libexec/git-core/git-add.debug (*)
> 4 /usr/bin/debug/usr/bin/git-shell.debug
> 4 /usr/bin/debug/usr/libexec/git-core/git-shell.debug
>
> (*) This link has the wrong name.
>
> Bug: https://bugs.gentoo.org/820107
> Signed-off-by: Mike Gilbert
> ---
> bin/estrip | 31 ++-
> 1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/bin/estrip b/bin/estrip
> index abe523fff..8134020fd 100755
> --- a/bin/estrip
> +++ b/bin/estrip
> @@ -201,17 +201,29 @@ save_elf_debug() {
> local x=$1
> local inode_debug=$2
> local splitdebug=$3
> - local d_noslash=${D%/}
> - local y=${ED%/}/usr/lib/debug/${x:${#d_noslash}}.debug
> +
> + local x_basename=${x##*/}
> + local x_dirname=${x%/*}
> + local y_dirname=${ED%/}/usr/lib/debug/${x_dirname#${D%/}/}
> + local y_basename y
Can we do better than X and Y here?
>
> # dont save debug info twice
> [[ ${x} == *".debug" ]] && return 0
>
> - mkdir -p "${y%/*}"
> + mkdir -p "${y_dirname}" || die
> +
> + if [[ -L ${inode_debug} ]] ; then
> + y_basename=$(readlink "${inode_debug}") || die
> + y_basename=${y_basename##*/}
> + y=${y_dirname}/${y_basename}
>
> - if [ -f "${inode_debug}" ] ; then
> - ln "${inode_debug}" "${y}" || die "ln failed unexpectedly"
> + if [[ ! -e ${y} ]]; then
> + ln -L "${inode_debug}" "${y}" || die
> + fi
> else
> + y_basename=${x_basename}.debug
> + y=${y_dirname}/${y_basename}
> +
> if [[ -n ${splitdebug} ]] ; then
> mv "${splitdebug}" "${y}"
> else
> @@ -222,11 +234,12 @@ save_elf_debug() {
> fi
> # Only do the following if the debug file was
> # successfully created (see bug #446774).
> - if [ $? -eq 0 ] ; then
> + if [[ $? -eq 0 ]] ; then
> local args="a-x,o-w"
> [[ -g ${x} || -u ${x} ]] && args+=",go-r"
> chmod ${args} "${y}"
> - ln "${y}" "${inode_debug}" || die "ln failed
> unexpectedly"
> + # symlink so we can read the name back.
> + ln -s "${y}" "${inode_debug}" || die
> fi
> fi
>
> @@ -239,8 +252,8 @@ save_elf_debug() {
> local
> buildid_dir="${ED%/}/usr/lib/debug/.build-id/${buildid:0:2}"
> local buildid_file="${buildid_dir}/${buildid:2}"
> mkdir -p "${buildid_dir}"
> - [ -L "${buildid_file}".debug ] || ln -s
> "../../${x:$((${#d_noslash} + 1))}.debug" "${buildid_file}.debug"
> - [ -L "${buildid_file}" ] || ln -s "/${x:$((${#d_noslash} +
> 1))}" "${buildid_file}"
> + [[ -L "${buildid_file}".debug ]] || ln -s
> "../../${x#${D%/}/}.debug" "${buildid_file}.debug"
> + [[ -L "${buildid_file}" ]] || ln -s
> "../../../../../${x#${D%/}/}" "${buildid_file}"
> fi
> }
>
> --
> 2.33.1
>
>
[gentoo-portage-dev] [PATCH] estrip: rework hard link logic in save_elf_debug
GDB loads debug files based on the file name given in the .gnu_debuglink
section, prepended with /usr/lib/debug/${dirname}, where dirname is the
absolute path to the parent directory of the binary being executed.
For each unique inode as input, we need a link to the debug file with
the GNU debuglink as its basename. A link to the debug file should exist
for each directory in which the input inode exists.
The debug link names should be based on the .gnu_debuglink value instead
of the name of the file we are processing as input.
The .gnu_debuglink value is based on the name of the first link
processed for each inode. We save this value as a symlink, and then read
it back as we process subsequent links.
For example, given the following input:
INODE PATH
1 /usr/bin/git
1 /usr/libexec/git-core/git-add
2 /usr/bin/git-shell
2 /usr/libexec/git-core/git-shell
We generate the following inodes for the debug files:
INODE DEBUGLINK
3 git.debug
4 git-shell.debug
We should generate the following links:
INODE PATH
3 /usr/lib/debug/usr/bin/git.debug
3 /usr/lib/debug/usr/libexec/git-core/git.debug
4 /usr/bin/debug/usr/bin/git-shell.debug
4 /usr/bin/debug/usr/libexec/git-core/git-shell.debug
The previous code would have generated this broken output:
INODE PATH
3 /usr/lib/debug/usr/bin/git.debug
3 /usr/lib/debug/usr/libexec/git-core/git-add.debug (*)
4 /usr/bin/debug/usr/bin/git-shell.debug
4 /usr/bin/debug/usr/libexec/git-core/git-shell.debug
(*) This link has the wrong name.
Bug: https://bugs.gentoo.org/820107
Signed-off-by: Mike Gilbert
---
bin/estrip | 31 ++-
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/bin/estrip b/bin/estrip
index abe523fff..8134020fd 100755
--- a/bin/estrip
+++ b/bin/estrip
@@ -201,17 +201,29 @@ save_elf_debug() {
local x=$1
local inode_debug=$2
local splitdebug=$3
- local d_noslash=${D%/}
- local y=${ED%/}/usr/lib/debug/${x:${#d_noslash}}.debug
+
+ local x_basename=${x##*/}
+ local x_dirname=${x%/*}
+ local y_dirname=${ED%/}/usr/lib/debug/${x_dirname#${D%/}/}
+ local y_basename y
# dont save debug info twice
[[ ${x} == *".debug" ]] && return 0
- mkdir -p "${y%/*}"
+ mkdir -p "${y_dirname}" || die
+
+ if [[ -L ${inode_debug} ]] ; then
+ y_basename=$(readlink "${inode_debug}") || die
+ y_basename=${y_basename##*/}
+ y=${y_dirname}/${y_basename}
- if [ -f "${inode_debug}" ] ; then
- ln "${inode_debug}" "${y}" || die "ln failed unexpectedly"
+ if [[ ! -e ${y} ]]; then
+ ln -L "${inode_debug}" "${y}" || die
+ fi
else
+ y_basename=${x_basename}.debug
+ y=${y_dirname}/${y_basename}
+
if [[ -n ${splitdebug} ]] ; then
mv "${splitdebug}" "${y}"
else
@@ -222,11 +234,12 @@ save_elf_debug() {
fi
# Only do the following if the debug file was
# successfully created (see bug #446774).
- if [ $? -eq 0 ] ; then
+ if [[ $? -eq 0 ]] ; then
local args="a-x,o-w"
[[ -g ${x} || -u ${x} ]] && args+=",go-r"
chmod ${args} "${y}"
- ln "${y}" "${inode_debug}" || die "ln failed
unexpectedly"
+ # symlink so we can read the name back.
+ ln -s "${y}" "${inode_debug}" || die
fi
fi
@@ -239,8 +252,8 @@ save_elf_debug() {
local
buildid_dir="${ED%/}/usr/lib/debug/.build-id/${buildid:0:2}"
local buildid_file="${buildid_dir}/${buildid:2}"
mkdir -p "${buildid_dir}"
- [ -L "${buildid_file}".debug ] || ln -s
"../../${x:$((${#d_noslash} + 1))}.debug" "${buildid_file}.debug"
- [ -L "${buildid_file}" ] || ln -s "/${x:$((${#d_noslash} +
1))}" "${buildid_file}"
+ [[ -L "${buildid_file}".debug ]] || ln -s
"../../${x#${D%/}/}.debug" "${buildid_file}.debug"
+ [[ -L "${buildid_file}" ]] || ln -s
"../../../../../${x#${D%/}/}" "${buildid_file}"
fi
}
--
2.33.1
