Re: [RFC, gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs

[Tom de Vries <tdevries@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2759 bytes --]

On 12-05-19 21:49, Simon Marchi wrote:
> On 2019-05-07 12:13 p.m., Tom de Vries wrote:
>> [ was: Re: [PATCH][gdb] Write index for dwz -m file ]
>>
>>
>> Hi,
>>
>> This is a follow-up patch for "[gdb] Write index for dwz -m file".
>>
>> Any comments on the updated gdb/contrib/gdb-add-index.sh script?
>>
>> In particular, I'd like some advice on whether I should add shell
>> variables (as I've done for readelf) for grep, tail and sed.
>>
>> Thanks,
>> - Tom
>>
> 
> Hi Tom,
> 
> I think it can be useful to override gdb, readelf and objcopy, as it is likely
> that people will want to use specific versions of these (either newer, or
> specific to their architectures).
> 
> But it's not very likely for grep, tail and sed, as long as we make sure that
> we are compatible with the BSD versions of these tools.  That would mean making
> sure we only use features defined by POSIX.
> 

Ack, thanks for the insight.

> One case that isn't handled correct by GDB and/or the script (with both my and
> your patch applied) is running the script on two executables that share the same
> external dwz file.  It will fail adding the index to the dwz file the second time.
> This use case is kind of important, because the point of having dwz files is to
> share it between multiple executables.
> 
> This is what I get the second time:
> 
> $ GDB="/home/simark/build/binutils-gdb/gdb/gdb --data-directory=/home/simark/build/binutils-gdb/gdb/data-directory" ~/src/binutils-gdb/gdb/contrib/gdb-add-index.sh b
> + objcopy --add-section .gdb_index=shared-debug-info.dwz.gdb-index --set-section-flags .gdb_index=readonly shared-debug-info.dwz shared-debug-info.dwz
> objcopy:std9IdCI: can't add section '.gdb_index': file in wrong format
> 
> I haven't looked in more details for this problem.
> 

That's fixed now.

[ In a way, it's a known problem. Running gdb-add-index twice on the
same (regular, non-dwz-ed) executable, gives the same kind of error. ]

> There's a buglet in the clean up code causing the dwz file's index
> (shared-debug-info.dwz.gdb-index in my case) to be left in the current directory after
> running the script (even successfully).  This fixes it:
> 
> ---
> diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
> index afedce0c848d..2b3af2e84f71 100755
> --- a/gdb/contrib/gdb-add-index.sh
> +++ b/gdb/contrib/gdb-add-index.sh
> @@ -70,7 +70,7 @@ for f in "$file" "$dwz_file"; do
>      if [ "$f" = "" ]; then
>  	continue
>      fi
> -    set_files "$file"
> +    set_files "$f"
>      tmp_files="$tmp_files $index4 $index5 $debugstr $debugstrmerge $debugstrerr"
>  done

Ok, I've incorporated that fix, as well as making the handle_file
$dwz_file conditional on $dwz_file != "".

Updated version attached.

Thanks,
- Tom

[-- Attachment #2: 0003-gdb-contrib-Fix-gdb-contrib-gdb-add-index.sh-for-dwz-m-ed-execs.patch --]
[-- Type: text/x-patch, Size: 5204 bytes --]

[gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs

Atm gdb-add-index.exp fails with target board cc-with-dwz-m.

Fix this by updating gdb/contrib/gdb-add-index.sh to handle a dwz-m-ed
executable.

Tested on x86_64-linux.

gdb/ChangeLog:

2019-05-07  Tom de Vries  <tdevries@suse.de>

	PR gdb/24445
	* contrib/gdb-add-index.sh: Update to handle dwz-m-ed executable.

---
 gdb/contrib/gdb-add-index.sh | 138 ++++++++++++++++++++++++++++---------------
 1 file changed, 91 insertions(+), 47 deletions(-)

diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
index efaad1dce7..5a37020656 100755
--- a/gdb/contrib/gdb-add-index.sh
+++ b/gdb/contrib/gdb-add-index.sh
@@ -20,6 +20,7 @@
 # If not, or you want others, pass the following in the environment
 GDB=${GDB:=gdb}
 OBJCOPY=${OBJCOPY:=objcopy}
+READELF=${READELF:=readelf}
 
 myname="${0##*/}"
 
@@ -43,15 +44,44 @@ fi
 
 dir="${file%/*}"
 test "$dir" = "$file" && dir="."
-index4="${file}.gdb-index"
-index5="${file}.debug_names"
-debugstr="${file}.debug_str"
-debugstrmerge="${file}.debug_str.merge"
-debugstrerr="${file}.debug_str.err"
 
-rm -f $index4 $index5 $debugstr $debugstrmerge $debugstrerr
+dwz_file=""
+if $READELF -S "$file" | grep -q " \.gnu_debugaltlink "; then
+    dwz_file=$($READELF --string-dump=.gnu_debugaltlink "$file" \
+		   | grep -A1  "'\.gnu_debugaltlink':" \
+		   | tail -n +2 \
+		   | sed 's/.*]//')
+    dwz_file=$(echo $dwz_file)
+    if $READELF -S "$dwz_file" | grep -E -q " \.(gdb_index|debug_names) "; then
+	# Already has an index, skip it.
+	dwz_file=""
+    fi
+fi
+
+set_files ()
+{
+    local file="$1"
+
+    index4="${file}.gdb-index"
+    index5="${file}.debug_names"
+    debugstr="${file}.debug_str"
+    debugstrmerge="${file}.debug_str.merge"
+    debugstrerr="${file}.debug_str.err"
+}
+
+tmp_files=
+for f in "$file" "$dwz_file"; do
+    if [ "$f" = "" ]; then
+	continue
+    fi
+    set_files "$f"
+    tmp_files="$tmp_files $index4 $index5 $debugstr $debugstrmerge $debugstrerr"
+done
+
+rm -f $tmp_files
+
 # Ensure intermediate index file is removed when we exit.
-trap "rm -f $index4 $index5 $debugstr $debugstrmerge $debugstrerr" 0
+trap "rm -f $tmp_files" 0
 
 $GDB --batch -nx -iex 'set auto-load no' \
     -ex "file $file" -ex "save gdb-index $dwarf5 $dir" || {
@@ -67,50 +97,64 @@ $GDB --batch -nx -iex 'set auto-load no' \
 # already stripped binary, it's a no-op.
 status=0
 
-if test -f "$index4" -a -f "$index5"; then
-    echo "$myname: Both index types were created for $file" 1>&2
-    status=1
-elif test -f "$index4" -o -f "$index5"; then
-    if test -f "$index4"; then
-	index="$index4"
-	section=".gdb_index"
-    else
-	index="$index5"
-	section=".debug_names"
-    fi
-    debugstradd=false
-    debugstrupdate=false
-    if test -s "$debugstr"; then
-	if ! $OBJCOPY --dump-section .debug_str="$debugstrmerge" "$file" /dev/null \
-		 2>$debugstrerr; then
-	    cat >&2 $debugstrerr
-	    exit 1
-	fi
-	if grep -q "can't dump section '.debug_str' - it does not exist" \
-		  $debugstrerr; then
-	    debugstradd=true
+handle_file ()
+{
+    local file
+    file="$1"
+
+    set_files "$file"
+
+    if test -f "$index4" -a -f "$index5"; then
+	echo "$myname: Both index types were created for $file" 1>&2
+	status=1
+    elif test -f "$index4" -o -f "$index5"; then
+	if test -f "$index4"; then
+	    index="$index4"
+	    section=".gdb_index"
 	else
-	    debugstrupdate=true
-	    cat >&2 $debugstrerr
+	    index="$index5"
+	    section=".debug_names"
+	fi
+	debugstradd=false
+	debugstrupdate=false
+	if test -s "$debugstr"; then
+	    if ! $OBJCOPY --dump-section .debug_str="$debugstrmerge" "$file" \
+		 /dev/null 2>$debugstrerr; then
+		cat >&2 $debugstrerr
+		exit 1
+	    fi
+	    if grep -q "can't dump section '.debug_str' - it does not exist" \
+		    $debugstrerr; then
+		debugstradd=true
+	    else
+		debugstrupdate=true
+		cat >&2 $debugstrerr
+	    fi
+	    cat "$debugstr" >>"$debugstrmerge"
 	fi
-	cat "$debugstr" >>"$debugstrmerge"
-    fi
 
-    $OBJCOPY --add-section $section="$index" \
-	--set-section-flags $section=readonly \
-	$(if $debugstradd; then \
-	      echo --add-section .debug_str="$debugstrmerge"; \
-	      echo --set-section-flags .debug_str=readonly; \
-	  fi; \
-	  if $debugstrupdate; then \
-	      echo --update-section .debug_str="$debugstrmerge"; \
-	  fi) \
-	"$file" "$file"
+	$OBJCOPY --add-section $section="$index" \
+		 --set-section-flags $section=readonly \
+		 $(if $debugstradd; then \
+		       echo --add-section .debug_str="$debugstrmerge"; \
+		       echo --set-section-flags .debug_str=readonly; \
+		   fi; \
+		   if $debugstrupdate; then \
+		       echo --update-section .debug_str="$debugstrmerge"; \
+		   fi) \
+		 "$file" "$file"
+
+	status=$?
+    else
+	echo "$myname: No index was created for $file" 1>&2
+	echo "$myname: [Was there no debuginfo? Was there already an index?]" \
+	     1>&2
+    fi
+}
 
-    status=$?
-else
-    echo "$myname: No index was created for $file" 1>&2
-    echo "$myname: [Was there no debuginfo? Was there already an index?]" 1>&2
+handle_file "$file"
+if [ "$dwz_file" != "" ]; then
+    handle_file "$dwz_file"
 fi
 
 exit $status