Re: [PATCH 3/4] gdb: pass target to thread_ptid_changed observable

Pedro Alves
2020-08-05T14:50:32Z

On 7/30/20 4:27 PM, Simon Marchi wrote:
> On 2020-07-23 4:42 p.m., Pedro Alves wrote:
>>> @@ -1817,6 +1818,58 @@ cooked_write_test (struct gdbarch *gdbarch)
>>> }
>>> }
>>>
>>> +/* Verify that when two threads with the same ptid exist (from two different
>>> + targets) and one of them changes ptid, we only update the appropriate
>>> + regcaches. */
>>> +
>>> +static void
>>> +regcache_thread_ptid_changed ()
>>> +{
>>> + /* Any arch will do. */
>>> + gdbarch *arch = current_inferior ()->gdbarch;
>>> +
>>> + /* Prepare two targets with one thread each, with the same ptid. */
>>> + scoped_mock_context<test_target_ops> target1 (arch);
>>> + scoped_mock_context<test_target_ops> target2 (arch);
>>> + target2.mock_inferior.next = &target1.mock_inferior;
>>> +
>>> + ptid_t old_ptid (111, 222);
>>> + ptid_t new_ptid (111, 333);
>>> +
>>> + target1.mock_inferior.pid = old_ptid.pid ();
>>> + target1.mock_thread.ptid = old_ptid;
>>> + target2.mock_inferior.pid = old_ptid.pid ();
>>> + target2.mock_thread.ptid = old_ptid;
>>> +
>>> + gdb_assert (regcaches.empty ());
>>
>> This will fail if you debug something, e.g., run to main,
>> and then do:
>>
>> (gdb) maint selftest regcache_thread_ptid_changed
>
> I don't know, do we really want to support this? I don't really see the point. We can design
> the selftests to make it possible, but is there any advantage? In the selftests command, we
> could ensure that you are not debugging something, and otherwise error out with "You can't run
> selftests while debugging.".

I don't see why we shouldn't. Assuming that the regcaches are empty
like in the current patch can also fail is someone inserts some test
before this new test that ends up filling in the regcache. I see it as
your new test being fragile because of that, and running the
"maint selftest" while some program is running is just a way
to trigger it. To me, not allowing selftests while the
program is running is just an admission that the tests
aren't well isolated enough.