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

[Tom de Vries <tdevries at suse dot de>]

On 13-05-19 12:47, Tom de Vries wrote:
> 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.
> 

OK for trunk?

Thanks,
- Tom
> 
> 0003-gdb-contrib-Fix-gdb-contrib-gdb-add-index.sh-for-dwz-m-ed-execs.patch
> 
> [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
>