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

[Andrew Burgess]

* Luis Machado <luis.machado@linaro.org> [2021-05-20 13:06:04 -0300]:

> 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.

You'll have to excuse me for being a little slow today.  I believe I
understand your concerns, which I think are:

  Selftest was not designed to be run in any situation other than in a
  vanilla GDB, with no executable loaded, so it's not surprising that
  it broke when we did "other" things.

What I'm struggling with is to translate your concerns into what
action you think I should take.

I think I see a couple of possibilities:

  1. Continue with the test I suggest (running selftest with an
  executable loaded), but we acknowledge that we might have to accept
  some selftest failures in this case, or

  2. Modify 'maint selftest' so it throws an error if GDB has an
  executable loaded, write a new test that checks this error occurs,
  and, write an additional test that reveals the bug I originally saw
  (via 'maint selftest') but through the alternative route.

  3. Maybe you have a different suggestion?

Honestly, I'm prepare to go with any approach, it's just, I found a
way to crash GDB, I fixed it, and I think we should make sure the same
failure can't be introduced again in the future.

Thanks for all your input,

Andrew