Re: [PATCH v6 3/4] Share fork_inferior et al with gdbserver

[Pedro Alves <palves@redhat.com>]

On 06/07/2017 11:15 AM, Pedro Alves wrote:
> On 05/31/2017 04:43 AM, Sergio Durigan Junior wrote:
>>
>>>>>> index 4ea7913..8aa85db 100644
>>>>>> --- a/gdb/gdbserver/configure.ac
>>>>>> +++ b/gdb/gdbserver/configure.ac
>>>>>> @@ -462,7 +462,9 @@ esac],
>>>>>>  
>>>>>>  if $want_ipa ; then
>>>>>>     if $have_ipa ; then
>>>>>> -     IPA_DEPFILES="$ipa_obj"
>>>>>> +     # Needed because safe_strerror's definition is host-dependent
>>>>
>>>> Why do we end up needing safe_strerror in the IPA in the first place?
>> This is needed because I moved the definition of
>> trace_start_error_with_name from the old gdb/fork-child.c to
>> common/common-utils.c.  This function which uses safe_strerror, and
>> common/common-utils.c is compiled by IPA.
>>
>> An option would be to keep these trace_start_error.* functions in
>> nat/fork-inferior.c, but I think it is more logical to keep them on
>> common-utils.c.
> 
> I'd rather not add them to the IPA.  The least unnecessary code
> is included in that library the better, because it is injected
> into the target process.  So keeping them in fork-inferior.c
> sounds better.
> 

I tried it against v7, and that works.  The think to keep
in mind is that these trace_start_error functions
are only called from the fork child, so they're very much
fork-inferior.c related.

>>>>
>>>>>> -#if defined(__UCLIBC__) && defined(HAS_NOMMU)
>>>>>> -  pid = vfork ();
>>>>
>>>> Does fork_inferior end up always using vfork on no-MMU
>>>> ports somehow?
>> Sorry, I am not sure.  How would I go about finding that?
> 
> I'm not sure either.  configure.ac for anything fork related?
> Look at git blame history around those lines, see what other
> bits were touched at the same time?  gdbserver clearly cares about
> being built for no-MMU ports, so the new code must too.  We can't
> just delete that support without an alternative.

I think we just need to add the HAVE_MMU etc. check around
vfork in fork-inferior.c.

> 
>> Now, something that came up while I was testing things with mingw here.
>> gdb/gdbserver/server.c now calls startup_inferior (define in
>> nat/fork-inferior.c) directly, instead of doing things on its own.  This
>> is one of the goals of this series, but for targets that don't compile
>> fork-inferior.c (like Windows) this is an obvious problem.  So here's
>> what I'm doing for the new version of the series: I'll move
>> startup_inferior to common/ (probably common/common-fork-inferior.c or
>> some such), so that all targets can have access to it (it's
>> target-agnostic anyway).  If you have any comments about this, please
>> let me know.
> 
> This sounds quite contorted, but I'll take a better look at it in v7.

I think that we just need to create a gdbserver/fork-child.c that is
very much like gdb's own gdb/fork-child.c ends up looking like,
and then only building fork-child.o on hosts that also build
nat/fork-inferior.o.  We can get rid of the function pointer
parameter this way.

I'll send you a patch on top of v7 in reply to v7, to show
what I mean.

Thanks,
Pedro Alves