[PATCH] gdb: fix target_ops reference count for some cases

Simon Marchi simark@simark.ca
Mon Sep 26 14:16:19 GMT 2022


>> Just wondering, why do we need to restore explicitly the current
>> pspace, instead of using just scoped_restore_current_thread?
>>
>> scoped_restore_current_pspace_and_thread's doc says:
>>
>> /* Save/restore the current program space, thread, inferior and frame.
>>    Use this when you need to call
>>    switch_to_program_space_and_thread.  */
>>
>> ... but you are not using switch_to_program_space_and_thread here.
>> Maybe it's ok and I just don't understand.  Same in
>> ~scoped_mock_context.
> 
> I suspect the comment you quote is just out of date.
> 
> switch_to_program_space_and_thread can end up calling
> switch_to_inferior_no_thread if there are no running threads in the
> program space being switched too.  But, even if switch_to_thread does
> end up being called we:
> 
>   - set the program space,
>   - set the inferior,
>   - set the current thread,
>   - reinit the frame cache,
> 
> By comparison, switch_to_inferior_no_thread does:
> 
>   - sets the program space,
>   - sets the inferior,
>   - sets the current thread (to nullptr this time though),
>   - reinits the frame cache,
> 
> As you can see they do the same set of things, all of which I think
> should be reverted once we leave the scope, hence
> scoped_restore_current_pspace_and_thread seems like the way to go.

Hmm okay, but won't scoped_restore_current_thread always restore the
pspace one way or another?  scoped_restore_current_thread::restore will
either call switch_to_thread or switch_to_inferior_no_thread, which both
end up setting the pspace.  I just don't understand why
scoped_restore_current_pspace_and_thread exists, it seems like
scoped_restore_current_thread would always work where
scoped_restore_current_pspace_and_thread is used.  I must be missing
something, scoped_restore_current_pspace_and_thread must have been added
for a reason.

Simon


More information about the Gdb-patches mailing list