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

[Simon Marchi <simon dot marchi at polymtl dot ca>]

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