[PATCH] gdb: run 'maint selftest' with an executable loaded

[Luis Machado]

On 5/20/21 12:41 PM, Andrew Burgess wrote:
> * Luis Machado <luis.machado@linaro.org> [2021-05-20 11:29:07 -0300]:
> 
>> On 5/20/21 10:29 AM, Andrew Burgess wrote:
>>> I spotted that 'maint selftest' with an executable loaded into GDB,
>>> would (when GDB was compiled for all targets) crash GDB.  I fixed this
>>> with a commit to bfd:
>>
>> I don't understand the purpose of being able to run selftests with an
>> executable loaded, or, more generally, running selftests when any GDB state
>> is capable of interfering with the tests.
>>
>> Should we force the selftests to cleanup the state before we run them? Or
>> not allow running selftests when there is a debugging session already
>> stablished?
> 
> I'm not trying to claim such a thing is useful, but I do feel pretty
> strongly that running 'maint selftest' (with an executable loaded)
> shouldn't cause GDB to segfault.  To ensure it doesn't get broken
> again in the future I'd suggest we need add this configuration to the
> testsuite.
> 
> I guess one possible solution would have been to add a check within
> the 'maint selftest' command that throws an error if an executable is
> loaded, but, that would have meant we missed a real bug (I claim) in
> bfd.
> 
> You'll need to check out commit 427e4066afd1 for details of the issue
> I found, but it could be reproduced (though I admit via a rather weird
> set of steps) outside of 'maint selftest'.
> 
> I understand your concerns about the selftests potentially depending
> on GDB being in a vanilla state, which might be corrupted by loading
> an executable, right now this doesn't seem to be the case (too much, I
> guess the ARM failure is an example of this happening).
> 
> If in the future we get more such cases, I'd have no problems with us
> increasing the number of acceptable selftest failures.

Right. I'm not against the fix, but this shows the selftest design in 
GDB is kinda broken in this regard. It is not self-contained and seems 
to rely on external state.

Just wanted to point out that we shouldn't need to handle running the 
selftests with an executable loaded. It makes me wonder what other 
options we can tweak when GDB is running that will cause the selftests 
to hit some unpredictable situations.

> 
>>
>>>
>>>     commit 427e4066afd13d1bf52c849849475f536e285d66
>>>     Date:   Thu May 20 09:16:41 2021 +0100
>>>
>>>         gdb/bfd: avoid crash when architecture is forced to csky or riscv
>>>
>>> However, this issue was not spotted as we currently only run 'maint
>>> selftest' without an executable loaded.
>>>
>>> This commit extends the testsuite to run 'maint selftest' both with
>>> and without an executable loaded into GDB.
>>>
>>> Currently, when no executable is loaded into GDB all of the selftest
>>> pass (i.e. the fail count is 0), however, when running with an
>>> executable loaded, I am seeing 1 failure (on an x86-64 GNU/Linux
>>> host).
>>>
>>> This failure is from the ARM disassembler tests, it appears that the
>>> disassembler somehow gets itself into a state where it thinks it is in
>>> thumb mode; when running the same test without an executable loaded
>>> this doesn't happen.
>>
>> Do you have more information about that?
> 
> Not really, the problem is the 'force_thumb' static global in
> opcode/arm-dis.c.  When running without an executable loaded this
> variable remains 'false'.  When run with an executable loaded I'm
> seeing this get set 'true' during an earlier test, then when we get to
> the print_one_insn tests we end up disassembling in thumb mode.
> 
> At this point I stopped looking, for exactly the reasons you raised
> above.

Thanks, that's useful to know.