* [PATCH] nat/fork-inferior: include linux-ptrace.h
@ 2018-06-25 8:05 Thomas Petazzoni
2018-06-27 14:25 ` Simon Marchi
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Petazzoni @ 2018-06-25 8:05 UTC (permalink / raw)
To: gdb-patches; +Cc: Thomas Petazzoni
To decide whether fork() or vfork() should be used, fork-inferior.c
uses the following test:
#if !(defined(__UCLIBC__) && defined(HAS_NOMMU))
However, HAS_NOMMU is never defined, because it gets defined in
linux-ptrace.h, which is not included by fork-inferior.c. Due to this,
gdbserver fails to build on noMMU architectures. This commit fixes
that by simply including linux-ptrace.h.
This bug was introduced by commit
2090129c36c7e582943b7d300968d19b46160d84 ("Share fork_inferior et al
with gdbserver"). Indeed, the same fork()/vfork() selection was done,
but in another file where linux-ptrace.h was included.
Fixes the following build issue:
../nat/fork-inferior.c: In function 'pid_t fork_inferior(const char*, const string&, char**, void (*)(), void (*)(int), void (*)(), const char*, void (*)(const char*, char* const*, char* const*))':
../nat/fork-inferior.c:376:11: error: 'fork' was not declared in this scope
pid = fork ();
^~~~
../nat/fork-inferior.c:376:11: note: suggested alternative: 'vfork'
pid = fork ();
^~~~
vfork
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
gdb/nat/fork-inferior.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index 8b59387fa5..05167628a6 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -26,6 +26,7 @@
#include "common-gdbthread.h"
#include "signals-state-save-restore.h"
#include "gdb_tilde_expand.h"
+#include "linux-ptrace.h"
#include <vector>
extern char **environ;
--
2.14.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nat/fork-inferior: include linux-ptrace.h
2018-06-25 8:05 [PATCH] nat/fork-inferior: include linux-ptrace.h Thomas Petazzoni
@ 2018-06-27 14:25 ` Simon Marchi
2018-06-27 14:30 ` Pedro Alves
2018-06-27 14:31 ` Thomas Petazzoni
0 siblings, 2 replies; 7+ messages in thread
From: Simon Marchi @ 2018-06-27 14:25 UTC (permalink / raw)
To: Thomas Petazzoni; +Cc: gdb-patches
On 2018-06-25 04:05, Thomas Petazzoni wrote:
> To decide whether fork() or vfork() should be used, fork-inferior.c
> uses the following test:
>
> #if !(defined(__UCLIBC__) && defined(HAS_NOMMU))
>
> However, HAS_NOMMU is never defined, because it gets defined in
> linux-ptrace.h, which is not included by fork-inferior.c. Due to this,
> gdbserver fails to build on noMMU architectures. This commit fixes
> that by simply including linux-ptrace.h.
>
> This bug was introduced by commit
> 2090129c36c7e582943b7d300968d19b46160d84 ("Share fork_inferior et al
> with gdbserver"). Indeed, the same fork()/vfork() selection was done,
> but in another file where linux-ptrace.h was included.
>
> Fixes the following build issue:
>
> ../nat/fork-inferior.c: In function 'pid_t fork_inferior(const char*,
> const string&, char**, void (*)(), void (*)(int), void (*)(), const
> char*, void (*)(const char*, char* const*, char* const*))':
> ../nat/fork-inferior.c:376:11: error: 'fork' was not declared in this
> scope
> pid = fork ();
> ^~~~
> ../nat/fork-inferior.c:376:11: note: suggested alternative: 'vfork'
> pid = fork ();
> ^~~~
> vfork
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
> gdb/nat/fork-inferior.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
> index 8b59387fa5..05167628a6 100644
> --- a/gdb/nat/fork-inferior.c
> +++ b/gdb/nat/fork-inferior.c
> @@ -26,6 +26,7 @@
> #include "common-gdbthread.h"
> #include "signals-state-save-restore.h"
> #include "gdb_tilde_expand.h"
> +#include "linux-ptrace.h"
> #include <vector>
>
> extern char **environ;
Hi Thomas,
fork-inferior.c is also included in native builds for BSDs, AIX, Solaris
and Darwin (see gdb/configure.nat). I am a bit concerned that
linux-ptrace.h could use some Linux-specific things, and thus would
break the other builds. However, I built-tested on FreeBSD and it seems
fine. Worst case, we can probably wrap this include in "#ifdef
__linux__" if that becomes a problem.
Do you have push access, or do you prefer if I push the patch for you?
I suppose that error was caught by a Buildroot autobuilder? Would it be
possible to have the config, so I can add a similar configuration to my
collection of cross-compiled GDB builds I use for build-testing?
Thanks,
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nat/fork-inferior: include linux-ptrace.h
2018-06-27 14:25 ` Simon Marchi
@ 2018-06-27 14:30 ` Pedro Alves
2018-06-27 14:31 ` Thomas Petazzoni
1 sibling, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2018-06-27 14:30 UTC (permalink / raw)
To: Simon Marchi, Thomas Petazzoni; +Cc: gdb-patches
On 06/27/2018 03:24 PM, Simon Marchi wrote:
> On 2018-06-25 04:05, Thomas Petazzoni wrote:
>> To decide whether fork() or vfork() should be used, fork-inferior.c
>> uses the following test:
>>
>> Â #if !(defined(__UCLIBC__) && defined(HAS_NOMMU))
>>
>> However, HAS_NOMMU is never defined, because it gets defined in
>> linux-ptrace.h, which is not included by fork-inferior.c. Due to this,
>> gdbserver fails to build on noMMU architectures. This commit fixes
>> that by simply including linux-ptrace.h.
>>
>> This bug was introduced by commit
>> 2090129c36c7e582943b7d300968d19b46160d84 ("Share fork_inferior et al
>> with gdbserver"). Indeed, the same fork()/vfork() selection was done,
>> but in another file where linux-ptrace.h was included.
>>
>> Fixes the following build issue:
>>
>> ../nat/fork-inferior.c: In function 'pid_t fork_inferior(const char*,
>> const string&, char**, void (*)(), void (*)(int), void (*)(), const
>> char*, void (*)(const char*, char* const*, char* const*))':
>> ../nat/fork-inferior.c:376:11: error: 'fork' was not declared in this scope
>> Â Â Â Â pid = fork ();
>> Â Â Â Â Â Â Â Â Â Â ^~~~
>> ../nat/fork-inferior.c:376:11: note: suggested alternative: 'vfork'
>> Â Â Â Â pid = fork ();
>> Â Â Â Â Â Â Â Â Â Â ^~~~
>> Â Â Â Â Â Â Â Â Â Â vfork
>>
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>> ---
>> Â gdb/nat/fork-inferior.c | 1 +
>> Â 1 file changed, 1 insertion(+)
>>
>> diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
>> index 8b59387fa5..05167628a6 100644
>> --- a/gdb/nat/fork-inferior.c
>> +++ b/gdb/nat/fork-inferior.c
>> @@ -26,6 +26,7 @@
>> Â #include "common-gdbthread.h"
>> Â #include "signals-state-save-restore.h"
>> Â #include "gdb_tilde_expand.h"
>> +#include "linux-ptrace.h"
>> Â #include <vector>
>>
>> Â extern char **environ;
>
> Hi Thomas,
>
> fork-inferior.c is also included in native builds for BSDs, AIX, Solaris and Darwin (see gdb/configure.nat). I am a bit concerned that linux-ptrace.h could use some Linux-specific things, and thus would break the other builds. However, I built-tested on FreeBSD and it seems fine. Worst case, we can probably wrap this include in "#ifdef __linux__" if that becomes a problem.
Please don't. It seems very wrong to me to include this header on other hosts.
It is full of Linux-only details.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nat/fork-inferior: include linux-ptrace.h
2018-06-27 14:25 ` Simon Marchi
2018-06-27 14:30 ` Pedro Alves
@ 2018-06-27 14:31 ` Thomas Petazzoni
2018-06-27 14:34 ` Simon Marchi
1 sibling, 1 reply; 7+ messages in thread
From: Thomas Petazzoni @ 2018-06-27 14:31 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
Hello Simon,
On Wed, 27 Jun 2018 10:24:07 -0400, Simon Marchi wrote:
> > diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
> > index 8b59387fa5..05167628a6 100644
> > --- a/gdb/nat/fork-inferior.c
> > +++ b/gdb/nat/fork-inferior.c
> > @@ -26,6 +26,7 @@
> > #include "common-gdbthread.h"
> > #include "signals-state-save-restore.h"
> > #include "gdb_tilde_expand.h"
> > +#include "linux-ptrace.h"
> > #include <vector>
> >
> > extern char **environ;
>
> Hi Thomas,
>
> fork-inferior.c is also included in native builds for BSDs, AIX, Solaris
> and Darwin (see gdb/configure.nat). I am a bit concerned that
> linux-ptrace.h could use some Linux-specific things, and thus would
> break the other builds. However, I built-tested on FreeBSD and it seems
> fine. Worst case, we can probably wrap this include in "#ifdef
> __linux__" if that becomes a problem.
Or better, this horrible mess of __UCLIBC__ and HAS_NOMMU macros should
be replaced by a proper autoconf check testing for the availability of
fork().
> Do you have push access, or do you prefer if I push the patch for you?
I don't have push access.
> I suppose that error was caught by a Buildroot autobuilder? Would it be
> possible to have the config, so I can add a similar configuration to my
> collection of cross-compiled GDB builds I use for build-testing?
The error wasn't caught by the autobuilders, but by the automated build
logic we use for http://toolchains.bootlin.com to build toolchains for
a wide range of CPU architectures.
The failure was
https://gitlab.com/free-electrons/toolchains-builder/-/jobs/77021057,
and the Buildroot configuration being built was:
BR2_m68k=y
BR2_m68k_cf5208=y
BR2_WGET="wget --passive-ftp -nd -t 3 --no-check-certificate"
BR2_HOST_DIR="/opt/m68k-coldfire--uclibc--bleeding-edge-2018.06-1"
BR2_GNU_MIRROR="http://ftp.gnu.org/pub/gnu"
BR2_KERNEL_HEADERS_4_14=y
BR2_TOOLCHAIN_BUILDROOT_LOCALE=y
BR2_PTHREAD_DEBUG=y
BR2_BINUTILS_VERSION_2_30_X=y
BR2_GCC_VERSION_8_X=y
BR2_TOOLCHAIN_BUILDROOT_CXX=y
BR2_PACKAGE_HOST_GDB=y
BR2_GDB_VERSION_8_1=y
BR2_INIT_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_GDB=y
# BR2_TARGET_ROOTFS_TAR is not set
Note that with Buildroot's latest master, this configuration will build
fine, because we already merged the fix for the fork/vfork issue.
Best regards,
Thomas Petazzoni
--
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nat/fork-inferior: include linux-ptrace.h
2018-06-27 14:31 ` Thomas Petazzoni
@ 2018-06-27 14:34 ` Simon Marchi
2018-06-27 16:29 ` Joel Brobecker
2018-06-27 16:32 ` Pedro Alves
0 siblings, 2 replies; 7+ messages in thread
From: Simon Marchi @ 2018-06-27 14:34 UTC (permalink / raw)
To: Thomas Petazzoni; +Cc: gdb-patches, Pedro Alves
On 2018-06-27 10:31, Thomas Petazzoni wrote:
>> fork-inferior.c is also included in native builds for BSDs, AIX,
>> Solaris
>> and Darwin (see gdb/configure.nat). I am a bit concerned that
>> linux-ptrace.h could use some Linux-specific things, and thus would
>> break the other builds. However, I built-tested on FreeBSD and it
>> seems
>> fine. Worst case, we can probably wrap this include in "#ifdef
>> __linux__" if that becomes a problem.
>
> Or better, this horrible mess of __UCLIBC__ and HAS_NOMMU macros should
> be replaced by a proper autoconf check testing for the availability of
> fork().
Agreed, and I think Pedro will agree too (unless you can think of a
simpler solution that doesn't involve autoconf?).
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nat/fork-inferior: include linux-ptrace.h
2018-06-27 14:34 ` Simon Marchi
@ 2018-06-27 16:29 ` Joel Brobecker
2018-06-27 16:32 ` Pedro Alves
1 sibling, 0 replies; 7+ messages in thread
From: Joel Brobecker @ 2018-06-27 16:29 UTC (permalink / raw)
To: Simon Marchi; +Cc: Thomas Petazzoni, gdb-patches, Pedro Alves
> > > fork-inferior.c is also included in native builds for BSDs, AIX,
> > > Solaris
> > > and Darwin (see gdb/configure.nat). I am a bit concerned that
> > > linux-ptrace.h could use some Linux-specific things, and thus would
> > > break the other builds. However, I built-tested on FreeBSD and it
> > > seems
> > > fine. Worst case, we can probably wrap this include in "#ifdef
> > > __linux__" if that becomes a problem.
> >
> > Or better, this horrible mess of __UCLIBC__ and HAS_NOMMU macros should
> > be replaced by a proper autoconf check testing for the availability of
> > fork().
>
> Agreed, and I think Pedro will agree too (unless you can think of a simpler
> solution that doesn't involve autoconf?).
We already autoconf for "fork" (see HAVE_FORK in config.in).
Does that help?
--
Joel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] nat/fork-inferior: include linux-ptrace.h
2018-06-27 14:34 ` Simon Marchi
2018-06-27 16:29 ` Joel Brobecker
@ 2018-06-27 16:32 ` Pedro Alves
1 sibling, 0 replies; 7+ messages in thread
From: Pedro Alves @ 2018-06-27 16:32 UTC (permalink / raw)
To: Simon Marchi, Thomas Petazzoni; +Cc: gdb-patches
On 06/27/2018 03:34 PM, Simon Marchi wrote:
> On 2018-06-27 10:31, Thomas Petazzoni wrote:
>>> fork-inferior.c is also included in native builds for BSDs, AIX, Solaris
>>> and Darwin (see gdb/configure.nat). I am a bit concerned that
>>> linux-ptrace.h could use some Linux-specific things, and thus would
>>> break the other builds. However, I built-tested on FreeBSD and it seems
>>> fine. Worst case, we can probably wrap this include in "#ifdef
>>> __linux__" if that becomes a problem.
>>
>> Or better, this horrible mess of __UCLIBC__ and HAS_NOMMU macros should
>> be replaced by a proper autoconf check testing for the availability of
>> fork().
>
> Agreed, and I think Pedro will agree too (unless you can think of a simpler solution that doesn't involve autoconf?).
That does sound better.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-06-27 16:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-25 8:05 [PATCH] nat/fork-inferior: include linux-ptrace.h Thomas Petazzoni
2018-06-27 14:25 ` Simon Marchi
2018-06-27 14:30 ` Pedro Alves
2018-06-27 14:31 ` Thomas Petazzoni
2018-06-27 14:34 ` Simon Marchi
2018-06-27 16:29 ` Joel Brobecker
2018-06-27 16:32 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).