[pacman-dev,v3] libmakepkg: when stripping split debug symbols, warn on duplicate paths

Message ID 20200116173127.188484-1-eschwartz@archlinux.org
State New
Headers show
Series
  • [pacman-dev,v3] libmakepkg: when stripping split debug symbols, warn on duplicate paths
Related show

Commit Message

Eli Schwartz Jan. 16, 2020, 5:31 p.m. UTC
Trying to strip multiple files installed to the same filepath (in
different components of a split package) can produce surprising results
if those files are built using different options. Per FS#63356 this is
not a supported PKGBUILD configuration. Instead, warn about such cases
when producing debug packages.

Fixes FS#63366

Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
---

v3: mention symbols are only included for the first copy

 scripts/libmakepkg/tidy/strip.sh.in | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

Comments

Allan McRae Feb. 3, 2020, 3:32 a.m. UTC | #1
On 17/1/20 3:31 am, Eli Schwartz wrote:
> Trying to strip multiple files installed to the same filepath (in
> different components of a split package) can produce surprising results
> if those files are built using different options. Per FS#63356 this is
> not a supported PKGBUILD configuration. Instead, warn about such cases
> when producing debug packages.
> 
> Fixes FS#63366
> 
> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
> ---
> 
> v3: mention symbols are only included for the first copy

I have been sitting on this for a while...   Looking at gdb docs:

Suppose you ask gdb to debug /usr/bin/ls, which has a debug link that
specifies the file ls.debug, and a build ID whose value in hex is
abcdef1234. If the global debug directory is /usr/lib/debug, then gdb
will look for the following debug information files, in the indicated order:

    /usr/lib/debug/.build-id/ab/cdef1234.debug
    /usr/bin/ls.debug
    /usr/bin/.debug/ls.debug
    /usr/lib/debug/usr/bin/ls.debug.

Currently we strip files to (e.g.) /usr/lib/debug/usr/bin/ls.debug and
symlink to /usr/lib/debug/.build-id/ab/cdef1234.debug.

So I think we are doing this wrong!  We should strip to
/usr/lib/debug/.build-id/, then symlink into the /usr/lib/debug/<path>/
tree.  That way, if we have a file path conflict across split-packages,
we still get debug files for both binaries.  In the case of the
conflict, we can leave the /usr/lib/debug/<path> symlink on a first come
wins basis, or delete it and have not file provide it.  With warning
message...

Allan


>  scripts/libmakepkg/tidy/strip.sh.in | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/libmakepkg/tidy/strip.sh.in b/scripts/libmakepkg/tidy/strip.sh.in
> index 301d1989..d69fd755 100644
> --- a/scripts/libmakepkg/tidy/strip.sh.in
> +++ b/scripts/libmakepkg/tidy/strip.sh.in
> @@ -40,13 +40,21 @@ source_files() {
>  		awk '/DW_AT_name +:/{name=$8}/DW_AT_comp_dir +:/{{if (name == "<artificial>") next}{if (name !~ /^[<\/]/) {printf "%s/", $8}}{print name}}'
>  }
>  
> +# strip a file and append to global array "duplicate_buildids" if a file with
> +# an identical id was already stripped
>  strip_file() {
>  	local binary=$1; shift
>  
>  	if check_option "debug" "y"; then
>  		local bid=$(build_id "$binary")
>  
> -		# has this file already been stripped
> +		# has this filepath been stripped before under a different Build Id
> +		if [[ -f "$dbgdir/$binary.debug" ]]; then
> +			if [[ -n $bid && $bid != $(build_id "$dbgdir/$binary.debug") ]]; then
> +				duplicate_buildids+=("${binary#./}")
> +			fi
> +		fi
> +		# has this file's hardlink already been stripped
>  		if [[ -n "$bid" ]]; then
>  			if [[ -f "$dbgdir/.build-id/${bid:0:2}/${bid:2}.debug" ]]; then
>  				return
> @@ -95,6 +103,7 @@ strip_file() {
>  
>  
>  tidy_strip() {
> +	local duplicate_buildids=()
>  	if check_option "strip" "y"; then
>  		msg2 "$(gettext "Stripping unneeded symbols from binaries and libraries...")"
>  		# make sure library stripping variables are defined to prevent excess stripping
> @@ -110,7 +119,7 @@ tidy_strip() {
>  		fi
>  
>  		local binary strip_flags
> -		find . -type f -perm -u+w -print0 2>/dev/null | while IFS= read -rd '' binary ; do
> +		while IFS= read -rd '' binary ; do
>  			case "$(LC_ALL=C readelf -h "$binary" 2>/dev/null)" in
>  				*Type:*'DYN (Shared object file)'*) # Libraries (.so) or Relocatable binaries
>  					strip_flags="$STRIP_SHARED";;
> @@ -129,6 +138,11 @@ tidy_strip() {
>  					continue ;;
>  			esac
>  			strip_file "$binary" ${strip_flags}
> -		done
> +		done < <(find . -type f -perm -u+w -print0 2>/dev/null)
> +		if (( ${#duplicate_buildids[@]} > 0 )); then
> +			warning "Split packages contain multiple copies of the same named file but different Build Ids"
> +			warning "Debug symbols are included for the first copy only"
> +			printf '%s\n' "${duplicate_buildids[@]}" >&2
> +		fi
>  	fi
>  }
>

Patch

diff --git a/scripts/libmakepkg/tidy/strip.sh.in b/scripts/libmakepkg/tidy/strip.sh.in
index 301d1989..d69fd755 100644
--- a/scripts/libmakepkg/tidy/strip.sh.in
+++ b/scripts/libmakepkg/tidy/strip.sh.in
@@ -40,13 +40,21 @@  source_files() {
 		awk '/DW_AT_name +:/{name=$8}/DW_AT_comp_dir +:/{{if (name == "<artificial>") next}{if (name !~ /^[<\/]/) {printf "%s/", $8}}{print name}}'
 }
 
+# strip a file and append to global array "duplicate_buildids" if a file with
+# an identical id was already stripped
 strip_file() {
 	local binary=$1; shift
 
 	if check_option "debug" "y"; then
 		local bid=$(build_id "$binary")
 
-		# has this file already been stripped
+		# has this filepath been stripped before under a different Build Id
+		if [[ -f "$dbgdir/$binary.debug" ]]; then
+			if [[ -n $bid && $bid != $(build_id "$dbgdir/$binary.debug") ]]; then
+				duplicate_buildids+=("${binary#./}")
+			fi
+		fi
+		# has this file's hardlink already been stripped
 		if [[ -n "$bid" ]]; then
 			if [[ -f "$dbgdir/.build-id/${bid:0:2}/${bid:2}.debug" ]]; then
 				return
@@ -95,6 +103,7 @@  strip_file() {
 
 
 tidy_strip() {
+	local duplicate_buildids=()
 	if check_option "strip" "y"; then
 		msg2 "$(gettext "Stripping unneeded symbols from binaries and libraries...")"
 		# make sure library stripping variables are defined to prevent excess stripping
@@ -110,7 +119,7 @@  tidy_strip() {
 		fi
 
 		local binary strip_flags
-		find . -type f -perm -u+w -print0 2>/dev/null | while IFS= read -rd '' binary ; do
+		while IFS= read -rd '' binary ; do
 			case "$(LC_ALL=C readelf -h "$binary" 2>/dev/null)" in
 				*Type:*'DYN (Shared object file)'*) # Libraries (.so) or Relocatable binaries
 					strip_flags="$STRIP_SHARED";;
@@ -129,6 +138,11 @@  tidy_strip() {
 					continue ;;
 			esac
 			strip_file "$binary" ${strip_flags}
-		done
+		done < <(find . -type f -perm -u+w -print0 2>/dev/null)
+		if (( ${#duplicate_buildids[@]} > 0 )); then
+			warning "Split packages contain multiple copies of the same named file but different Build Ids"
+			warning "Debug symbols are included for the first copy only"
+			printf '%s\n' "${duplicate_buildids[@]}" >&2
+		fi
 	fi
 }