Re: [PATCHv3 7/7] gdb: some process_stratum_target should not be shared

[Andrew Burgess <aburgess@redhat.com>]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> For things like the Linux and FreeBSD native targets, as well as the
> Andrew> remote target, this is absolutely true (or was made true).  But some
> Andrew> targets were never updated to be shareable, for example, the
> Andrew> core_target, which is used when reading core-files, stores some of its
> Andrew> state in the program_space, but also, the core-file and the executable
> Andrew> being debugged are closely related.
>
> Andrew> As each add-inferior call creates an inferior with a new
> Andrew> program_space, and doesn't automatically copy the executable, or the
> Andrew> current core-file, I don't think it really makes sense to "share"
> Andrew> core_target objects between inferiors.
>
> Yeah, probably the core target just doesn't even make sense to share.
>
> I guess if we did want to share it, we could move the state into the
> core target.  Maybe this is worthwhile to do anyway?  I see a bunch of
> uses of core_bfd (which is a #define reaching into the program space),
> but some, e.g. in linux_read_core_file_mappings, seem like they could be
> replaced with a parameter.
>
> Andrew> I think this behaviour might be confusing, so I'd like to have GDB not
> Andrew> initially share the core connection.  Instead, when the user tries to
> Andrew> add the new inferior a warning is given, and the new inferior is
> Andrew> created without a connection, like this:
>
> This makes sense to me.
>
> Andrew> +  /* The core_target only works for the inferior in which it was initially
> Andrew> +     opened, and can't be copied to some other inferior's target_stack.  */
> Andrew> +  bool is_shareable () override
> Andrew> +  { return false; }

Tom,

Thanks for all the reviews on this series.  Sorry it's taken me so long
to look at these patches again.

>
> However, why only mark the core target this way?

Mostly, because that was the only target I'd run into problems with.

> I think there are a lot of other targets that can't be
> shared... remote-sim, all the trace targets, even I think windows-nat,
> since it isn't multi-inferior-capable yet.
>
> So maybe the default implementation should be 'return false' and then
> specific known-good targets should override it?

Mostly I'm just nervous about making such sweeping changes.  As a result
I tend to go smaller and try to only change the minimum needed to fix
the issue I'm seeing.  But I think you're right, I should probably
consider if the default makes more sense the other way around.

I've fixed all the issues you pointed out with patches 1->6, so I'm
going to go ahead and merge them, then take another look at patch 7, and
I'll post an updated version.

Thanks,
Andrew