From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 79000 invoked by alias); 10 Jun 2019 18:41:47 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 78870 invoked by uid 89); 10 Jun 2019 18:41:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy=batch, incorporated, H*Ad:U*palves X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 10 Jun 2019 18:41:45 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 10AFAAF99; Mon, 10 Jun 2019 18:41:43 +0000 (UTC) Subject: [PATCH, gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs From: Tom de Vries To: Simon Marchi , gdb-patches@sourceware.org, Pedro Alves References: <20190507144207.GA17626@delia> <852d3210-f59a-1540-ed23-19f31829f465@polymtl.ca> <4899dcce-cb5c-27a7-9f21-d230e13485cd@suse.de> Message-ID: <754e9866-69a8-8f20-e325-789096e6b14f@suse.de> Date: Mon, 10 Jun 2019 18:41:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <4899dcce-cb5c-27a7-9f21-d230e13485cd@suse.de> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2019-06/txt/msg00198.txt.bz2 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 > > 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 >