Re: exec-file-mismatch and native-gdbserver testing

[Pedro Alves <palves@redhat.com>]

On 5/18/20 11:35 AM, Philippe Waroquiers via Gdb wrote:
> On Sun, 2020-05-17 at 22:58 +0100, Pedro Alves wrote:
>> If the executable file is modified while GDB is busy using it,
>>> could that not cause some problems ?
>>
>> How does filename detection help with this?  If GDB is busy
>> using it, you're already past the initial filename verification.
>> And if you modify the binary but don't change the filename,
>> then the filename verification doesn't help either.  There
>> are many ways to shoot yourself in the foot, I don't think
>> we need to protect against all of them.
> I think we have 2 different aspects:
>   * can GDB detect and protect against all problems ? Answer is no :).
>   * can GDB silently do something that might afterwards lead
>     to unexpected behaviour ? IMO, answer is preferably no:
>     If GDB does something (like *not* using the file that
>     a process is using), IMO, GDB should at least tell that to the user.
>     (like GDB is telling to the user that it is reloading a changed file).
>  
>>
>>>
>>>
>>>>> So, my main original use case needs filename comparison :(.
>>>>
>>>> According to:
>>>>
>>>>  https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/6/html/developer_guide/compiling-build-id
>>>>
>>>> "Each executable or shared library built with Red Hat Enterprise Linux Server 6 or later is assigned a unique identification 160-bit SHA-1 string, generated as a checksum of selected parts of the binary. "
>>>>
>>>> Maybe older gold versions didn't emit the build id by default, while
>>>> GNU ld did.  I tried it with master gold, and it emits the build id 
>>>> by default.  does explicitly specifying --build-id on the link work?
>>>> Since you're already not using the default tools, you could tweak
>>>> your build system to explicitly request a build id?
>>> I will check tomorrow if I can persuade the build system
>>> to generate a build ID.
>>> If yes (and assuming all what we have to debug but we do not build
>>> ourselves has a build ID), then build ID will cover my use case.
> I managed to have a build ID generated by adding --build-id linker arg,
> thanks for the hint of explicitly passing --build-id.
> 
> For info, these are the linker versions:
> gnatpro-20.0-20191009-L7/libexec/gcc/x86_64-pc-linux-gnu/8.3.1/ld.gold --version
> GNU gold (GNU Binutils 2.30.52.20180618) 1.15
> gnatpro-20.0-20191009-L7/libexec/gcc/x86_64-pc-linux-gnu/8.3.1/ld --version
> GNU ld (GNU Binutils) 2.30.52.20180618
> 
> 
> So, comparing build id is good enough for my @work use case.

Great, thanks for confirming that!

> 
> 
>> OK.  But I argue that the filename matching is more harmful than
>> helpful (see e.g., the remote debugging scenarios I presented).
>> I would rather remove it before a release is out with it, and
>> consider a better fallback if one is found & needed.  If
>> we keep it, we also have to come up with ways to unbreak the
>> testsuite on the different configurations that are presently
>> broken for it.  That alone would be sufficient grounds for a
>> reversion until it is fixed, IMHO.
> GDB silently keeping a wrong exec-file e.g. when using attach
> is very confusing.
> The exec-file-mismatch is supposed to avoid such confusion, and
> if that confusion can be avoided when there are no build ids,
> that is preferable IMO.
> 
> Maybe what we can do is:
>  - If build ids are equal, then no problem (and no need to compare file names).
>  - If build ids are different, then ask or warn user about mismatch
>    (and no need to compare file names).
>  - If build ids are not available, then try to detect confusing situation via
>    some fallback (such as the filename comparison).

This was actually what my patch was doing.  If we could compare build ids
and that detected a mismatch, then no filename comparison was made.
What I think is subpar is that the fallback is purely based on comparing
filenames.

> 
> Will that not solve (most of) the problems of the testsuite/remote debugging
> and detect (more) exec file mismatches in various setups/platforms 
> (like the default setup at my work) ?
> 
> If the filename fallback is giving too much problems but only
> in case of remote setup, we can maybe disable file name comparisons
> in such remote debugging.
> 
> But as said above, for my @work use case, I (selfishly :)) see that
> build ids are good enough for me.
> 
> So, if you think filename comparison will do more harm than good,
> fine for me to remove it.

Alright, I really think the filename comparison is too simplistic
and causes more harm than good (especially the querying).  I've updated
the patch over at gdb-patches to remove it for now.  I'd say, only
revisit if we can't convince people that they should enable build ids.
Over time, given that the GNU toolchain (at least) enables
it by default nowadays, I think that the issue will sort itself
out, and we can just forget about fallbacks.  :-)

> Thanks for the build id patch/investigation/help
Thanks,
Pedro Alves