[RFC,gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs
Commit Message
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
Comments
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
>
On 2019-06-10 2:41 p.m., Tom de Vries wrote:
> 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
Hi Tom,
Thanks, this LGTM.
Simon
[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(-)
@@ -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