* [PATCH] Disable get_ptrace_pid for NetBSD @ 2020-03-18 21:49 Kamil Rytarowski 2020-03-18 21:51 ` Kamil Rytarowski 2020-03-18 21:55 ` [PATCH v2] " Kamil Rytarowski 0 siblings, 2 replies; 23+ messages in thread From: Kamil Rytarowski @ 2020-03-18 21:49 UTC (permalink / raw) To: gdb-patches; +Cc: tom, Kamil Rytarowski Unlike most other Operating Systems, NetBSD tracks both pid and lwp. The process id on NetBSD is stored always in the pid field of ptid. gdb/ChangeLog: * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. * inf-ptrace.c: Likewise. * (gdb_ptrace): Add. * (inf_ptrace_target::resume): Update. * (inf_ptrace_target::xfer_partial): Likewise. --- gdb/ChangeLog | 8 ++++++++ gdb/inf-ptrace.c | 32 +++++++++++++++++++++++--------- gdb/inf-ptrace.h | 2 ++ 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 84964dc00ac..406414cee76 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,11 @@ +2020-03-18 Kamil Rytarowski <n54@gmx.com> + + * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. + * inf-ptrace.c: Likewise. + * (gdb_ptrace): Add. + * (inf_ptrace_target::resume): Update. + * (inf_ptrace_target::xfer_partial): Likewise. + 2020-03-17 Kamil Rytarowski <n54@gmx.com> * regformats/regdef.h: Put reg in gdb namespace. diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c index db17a76d946..304a749f3f3 100644 --- a/gdb/inf-ptrace.c +++ b/gdb/inf-ptrace.c @@ -37,6 +37,18 @@ \f +static int +gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG3 addr, + PTRACE_TYPE_ARG4 data) +{ +#ifdef __NetBSD__ + return ptrace (request, ptid.pid (), addr, data); +#else + pid_t pid = get_ptrace_pid (ptid); + return ptrace (request, pid, addr, data); +#endif +} + /* A unique_ptr helper to unpush a target. */ struct target_unpusher @@ -313,8 +325,12 @@ inf_ptrace_target::kill () target_mourn_inferior (inferior_ptid); } +#ifndef __NetBSD__ /* Return which PID to pass to ptrace in order to observe/control the - tracee identified by PTID. */ + tracee identified by PTID. + + Unlike most other Operating Systems, NetBSD tracks both pid and lwp + and avoids this function. */ pid_t get_ptrace_pid (ptid_t ptid) @@ -328,6 +344,7 @@ get_ptrace_pid (ptid_t ptid) pid = ptid.pid (); return pid; } +#endif /* Resume execution of thread PTID, or all threads if PTID is -1. If STEP is nonzero, single-step it. If SIGNAL is nonzero, give it @@ -336,15 +353,12 @@ get_ptrace_pid (ptid_t ptid) void inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) { - pid_t pid; int request; if (minus_one_ptid == ptid) /* Resume all threads. Traditionally ptrace() only supports single-threaded processes, so simply resume the inferior. */ - pid = inferior_ptid.pid (); - else - pid = get_ptrace_pid (ptid); + ptid = inferior_ptid; if (catch_syscall_enabled () > 0) request = PT_SYSCALL; @@ -365,7 +379,7 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) where it was. If GDB wanted it to start some other way, we have already written a new program counter value to the child. */ errno = 0; - ptrace (request, pid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); + gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); if (errno != 0) perror_with_name (("ptrace")); } @@ -528,7 +542,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, const gdb_byte *writebuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) { - pid_t pid = get_ptrace_pid (inferior_ptid); + ptid_t ptid = inferior_ptid; switch (object) { @@ -552,7 +566,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, piod.piod_len = len; errno = 0; - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) { /* Return the actual number of bytes read or written. */ *xfered_len = piod.piod_len; @@ -588,7 +602,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, piod.piod_len = len; errno = 0; - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) { /* Return the actual number of bytes read or written. */ *xfered_len = piod.piod_len; diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h index dd0733736f2..340d41d8beb 100644 --- a/gdb/inf-ptrace.h +++ b/gdb/inf-ptrace.h @@ -78,9 +78,11 @@ struct inf_ptrace_target : public inf_child_target void detach_success (inferior *inf); }; +#ifndef __NetBSD__ /* Return which PID to pass to ptrace in order to observe/control the tracee identified by PTID. */ extern pid_t get_ptrace_pid (ptid_t); +#endif #endif -- 2.25.0 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Disable get_ptrace_pid for NetBSD 2020-03-18 21:49 [PATCH] Disable get_ptrace_pid for NetBSD Kamil Rytarowski @ 2020-03-18 21:51 ` Kamil Rytarowski 2020-03-18 21:55 ` [PATCH v2] " Kamil Rytarowski 1 sibling, 0 replies; 23+ messages in thread From: Kamil Rytarowski @ 2020-03-18 21:51 UTC (permalink / raw) To: gdb-patches; +Cc: tom, gdb-patches@sourceware.org >> gdb-patches [-- Attachment #1.1: Type: text/plain, Size: 5060 bytes --] On 18.03.2020 22:49, Kamil Rytarowski wrote: > Unlike most other Operating Systems, NetBSD tracks both pid and lwp. > The process id on NetBSD is stored always in the pid field of ptid. > There is is one bug. I'm going to fix and submit a new version. > gdb/ChangeLog: > > * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. > * inf-ptrace.c: Likewise. > * (gdb_ptrace): Add. > * (inf_ptrace_target::resume): Update. > * (inf_ptrace_target::xfer_partial): Likewise. > --- > gdb/ChangeLog | 8 ++++++++ > gdb/inf-ptrace.c | 32 +++++++++++++++++++++++--------- > gdb/inf-ptrace.h | 2 ++ > 3 files changed, 33 insertions(+), 9 deletions(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 84964dc00ac..406414cee76 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,11 @@ > +2020-03-18 Kamil Rytarowski <n54@gmx.com> > + > + * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. > + * inf-ptrace.c: Likewise. > + * (gdb_ptrace): Add. > + * (inf_ptrace_target::resume): Update. > + * (inf_ptrace_target::xfer_partial): Likewise. > + > 2020-03-17 Kamil Rytarowski <n54@gmx.com> > > * regformats/regdef.h: Put reg in gdb namespace. > diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c > index db17a76d946..304a749f3f3 100644 > --- a/gdb/inf-ptrace.c > +++ b/gdb/inf-ptrace.c > @@ -37,6 +37,18 @@ > > \f > > +static int > +gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG3 addr, > + PTRACE_TYPE_ARG4 data) > +{ > +#ifdef __NetBSD__ > + return ptrace (request, ptid.pid (), addr, data); > +#else > + pid_t pid = get_ptrace_pid (ptid); > + return ptrace (request, pid, addr, data); > +#endif > +} > + > /* A unique_ptr helper to unpush a target. */ > > struct target_unpusher > @@ -313,8 +325,12 @@ inf_ptrace_target::kill () > target_mourn_inferior (inferior_ptid); > } > > +#ifndef __NetBSD__ > /* Return which PID to pass to ptrace in order to observe/control the > - tracee identified by PTID. */ > + tracee identified by PTID. > + > + Unlike most other Operating Systems, NetBSD tracks both pid and lwp > + and avoids this function. */ > > pid_t > get_ptrace_pid (ptid_t ptid) > @@ -328,6 +344,7 @@ get_ptrace_pid (ptid_t ptid) > pid = ptid.pid (); > return pid; > } > +#endif > > /* Resume execution of thread PTID, or all threads if PTID is -1. If > STEP is nonzero, single-step it. If SIGNAL is nonzero, give it > @@ -336,15 +353,12 @@ get_ptrace_pid (ptid_t ptid) > void > inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) > { > - pid_t pid; > int request; > > if (minus_one_ptid == ptid) > /* Resume all threads. Traditionally ptrace() only supports > single-threaded processes, so simply resume the inferior. */ > - pid = inferior_ptid.pid (); > - else > - pid = get_ptrace_pid (ptid); > + ptid = inferior_ptid; > > if (catch_syscall_enabled () > 0) > request = PT_SYSCALL; > @@ -365,7 +379,7 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) > where it was. If GDB wanted it to start some other way, we have > already written a new program counter value to the child. */ > errno = 0; > - ptrace (request, pid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); > + gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); > if (errno != 0) > perror_with_name (("ptrace")); > } > @@ -528,7 +542,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, > const gdb_byte *writebuf, > ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) > { > - pid_t pid = get_ptrace_pid (inferior_ptid); > + ptid_t ptid = inferior_ptid; > > switch (object) > { > @@ -552,7 +566,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, > piod.piod_len = len; > > errno = 0; > - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) > + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) > { > /* Return the actual number of bytes read or written. */ > *xfered_len = piod.piod_len; > @@ -588,7 +602,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, > piod.piod_len = len; > > errno = 0; > - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) > + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) > { > /* Return the actual number of bytes read or written. */ > *xfered_len = piod.piod_len; > diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h > index dd0733736f2..340d41d8beb 100644 > --- a/gdb/inf-ptrace.h > +++ b/gdb/inf-ptrace.h > @@ -78,9 +78,11 @@ struct inf_ptrace_target : public inf_child_target > void detach_success (inferior *inf); > }; > > +#ifndef __NetBSD__ > /* Return which PID to pass to ptrace in order to observe/control the > tracee identified by PTID. */ > > extern pid_t get_ptrace_pid (ptid_t); > +#endif > > #endif > -- > 2.25.0 > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2] Disable get_ptrace_pid for NetBSD 2020-03-18 21:49 [PATCH] Disable get_ptrace_pid for NetBSD Kamil Rytarowski 2020-03-18 21:51 ` Kamil Rytarowski @ 2020-03-18 21:55 ` Kamil Rytarowski 2020-03-18 22:21 ` Simon Marchi 2020-03-18 23:16 ` [PATCH v3] " Kamil Rytarowski 1 sibling, 2 replies; 23+ messages in thread From: Kamil Rytarowski @ 2020-03-18 21:55 UTC (permalink / raw) To: gdb-patches; +Cc: tom, Kamil Rytarowski Unlike most other Operating Systems, NetBSD tracks both pid and lwp. The process id on NetBSD is stored always in the pid field of ptid. gdb/ChangeLog: * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. * inf-ptrace.c: Likewise. * (gdb_ptrace): Add. * (inf_ptrace_target::resume): Update. * (inf_ptrace_target::xfer_partial): Likewise. --- gdb/ChangeLog | 8 ++++++++ gdb/inf-ptrace.c | 34 ++++++++++++++++++++++++---------- gdb/inf-ptrace.h | 2 ++ 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 84964dc00ac..406414cee76 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,11 @@ +2020-03-18 Kamil Rytarowski <n54@gmx.com> + + * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. + * inf-ptrace.c: Likewise. + * (gdb_ptrace): Add. + * (inf_ptrace_target::resume): Update. + * (inf_ptrace_target::xfer_partial): Likewise. + 2020-03-17 Kamil Rytarowski <n54@gmx.com> * regformats/regdef.h: Put reg in gdb namespace. diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c index db17a76d946..931192bd0db 100644 --- a/gdb/inf-ptrace.c +++ b/gdb/inf-ptrace.c @@ -37,6 +37,18 @@ \f +static int +gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG3 addr, + PTRACE_TYPE_ARG4 data) +{ +#ifdef __NetBSD__ + return ptrace (request, ptid.pid (), addr, data); +#else + pid_t pid = get_ptrace_pid (ptid); + return ptrace (request, pid, addr, data); +#endif +} + /* A unique_ptr helper to unpush a target. */ struct target_unpusher @@ -313,8 +325,12 @@ inf_ptrace_target::kill () target_mourn_inferior (inferior_ptid); } +#ifndef __NetBSD__ /* Return which PID to pass to ptrace in order to observe/control the - tracee identified by PTID. */ + tracee identified by PTID. + + Unlike most other Operating Systems, NetBSD tracks both pid and lwp + and avoids this function. */ pid_t get_ptrace_pid (ptid_t ptid) @@ -328,6 +344,7 @@ get_ptrace_pid (ptid_t ptid) pid = ptid.pid (); return pid; } +#endif /* Resume execution of thread PTID, or all threads if PTID is -1. If STEP is nonzero, single-step it. If SIGNAL is nonzero, give it @@ -336,15 +353,12 @@ get_ptrace_pid (ptid_t ptid) void inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) { - pid_t pid; int request; if (minus_one_ptid == ptid) /* Resume all threads. Traditionally ptrace() only supports single-threaded processes, so simply resume the inferior. */ - pid = inferior_ptid.pid (); - else - pid = get_ptrace_pid (ptid); + ptid = inferior_ptid; if (catch_syscall_enabled () > 0) request = PT_SYSCALL; @@ -365,7 +379,7 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) where it was. If GDB wanted it to start some other way, we have already written a new program counter value to the child. */ errno = 0; - ptrace (request, pid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); + gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); if (errno != 0) perror_with_name (("ptrace")); } @@ -528,7 +542,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, const gdb_byte *writebuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) { - pid_t pid = get_ptrace_pid (inferior_ptid); + ptid_t ptid = inferior_ptid; switch (object) { @@ -552,7 +566,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, piod.piod_len = len; errno = 0; - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) { /* Return the actual number of bytes read or written. */ *xfered_len = piod.piod_len; @@ -565,7 +579,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, return TARGET_XFER_EOF; } #endif - *xfered_len = inf_ptrace_peek_poke (pid, readbuf, writebuf, + *xfered_len = inf_ptrace_peek_poke (ptid.pid (), readbuf, writebuf, offset, len); return *xfered_len != 0 ? TARGET_XFER_OK : TARGET_XFER_EOF; @@ -588,7 +602,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, piod.piod_len = len; errno = 0; - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) { /* Return the actual number of bytes read or written. */ *xfered_len = piod.piod_len; diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h index dd0733736f2..340d41d8beb 100644 --- a/gdb/inf-ptrace.h +++ b/gdb/inf-ptrace.h @@ -78,9 +78,11 @@ struct inf_ptrace_target : public inf_child_target void detach_success (inferior *inf); }; +#ifndef __NetBSD__ /* Return which PID to pass to ptrace in order to observe/control the tracee identified by PTID. */ extern pid_t get_ptrace_pid (ptid_t); +#endif #endif -- 2.25.0 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] Disable get_ptrace_pid for NetBSD 2020-03-18 21:55 ` [PATCH v2] " Kamil Rytarowski @ 2020-03-18 22:21 ` Simon Marchi 2020-03-18 23:30 ` Kamil Rytarowski 2020-03-18 23:16 ` [PATCH v3] " Kamil Rytarowski 1 sibling, 1 reply; 23+ messages in thread From: Simon Marchi @ 2020-03-18 22:21 UTC (permalink / raw) To: Kamil Rytarowski, gdb-patches; +Cc: tom On 2020-03-18 5:55 p.m., Kamil Rytarowski wrote: > @@ -313,8 +325,12 @@ inf_ptrace_target::kill () > target_mourn_inferior (inferior_ptid); > } > > +#ifndef __NetBSD__ > /* Return which PID to pass to ptrace in order to observe/control the > - tracee identified by PTID. */ > + tracee identified by PTID. > + > + Unlike most other Operating Systems, NetBSD tracks both pid and lwp > + and avoids this function. */ To align this function with our current way of documenting functions, please move this comment to the declaration of get_ptrace_pid, in inf-ptrace.h, and put this here: /* See inf-ptrace.h. */ > > pid_t > get_ptrace_pid (ptid_t ptid) > @@ -328,6 +344,7 @@ get_ptrace_pid (ptid_t ptid) > pid = ptid.pid (); > return pid; > } > +#endif > > /* Resume execution of thread PTID, or all threads if PTID is -1. If > STEP is nonzero, single-step it. If SIGNAL is nonzero, give it > @@ -336,15 +353,12 @@ get_ptrace_pid (ptid_t ptid) > void > inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) > { > - pid_t pid; > int request; > > if (minus_one_ptid == ptid) > /* Resume all threads. Traditionally ptrace() only supports > single-threaded processes, so simply resume the inferior. */ > - pid = inferior_ptid.pid (); > - else > - pid = get_ptrace_pid (ptid); > + ptid = inferior_ptid; I'm not certain about this part (which I suggested). With the existing code, if inferior_ptid is {pid=10, lwp=20}, we'll call ptrace with pid=10. With the patch, we'll call ptrace with pid=20. I'm not entirely sure if/when resume can be called with ptid == minus_one_ptid. And if it's called with minus_one_ptid, is it possible that ptid.pid != ptid.lwp? In any case, it's probably safer to do: ptid = ptid_t (inferior_ptid.pid ()); to retain the existing behavior. WDYT? > > if (catch_syscall_enabled () > 0) > request = PT_SYSCALL; > @@ -365,7 +379,7 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) > where it was. If GDB wanted it to start some other way, we have > already written a new program counter value to the child. */ > errno = 0; > - ptrace (request, pid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); > + gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); > if (errno != 0) > perror_with_name (("ptrace")); > } > @@ -528,7 +542,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, > const gdb_byte *writebuf, > ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) > { > - pid_t pid = get_ptrace_pid (inferior_ptid); > + ptid_t ptid = inferior_ptid; > > switch (object) > { > @@ -552,7 +566,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, > piod.piod_len = len; > > errno = 0; > - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) > + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) > { > /* Return the actual number of bytes read or written. */ > *xfered_len = piod.piod_len; > @@ -565,7 +579,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, > return TARGET_XFER_EOF; > } > #endif > - *xfered_len = inf_ptrace_peek_poke (pid, readbuf, writebuf, > + *xfered_len = inf_ptrace_peek_poke (ptid.pid (), readbuf, writebuf, > offset, len); > return *xfered_len != 0 ? TARGET_XFER_OK : TARGET_XFER_EOF; I'm pretty sure that this change is not right. For non-NetBSD targets, this used to pass `inferior_ptid.lwp` (the result of get_ptrace_pid). Now, we pass `inferior_ptid.pid`. I think you'll need to pass the ptid to inf_ptrace_peek_poke and use gdb_ptrace inside that function too. Simon ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2] Disable get_ptrace_pid for NetBSD 2020-03-18 22:21 ` Simon Marchi @ 2020-03-18 23:30 ` Kamil Rytarowski 0 siblings, 0 replies; 23+ messages in thread From: Kamil Rytarowski @ 2020-03-18 23:30 UTC (permalink / raw) To: Simon Marchi, gdb-patches; +Cc: tom [-- Attachment #1.1: Type: text/plain, Size: 4060 bytes --] On 18.03.2020 23:21, Simon Marchi wrote: > On 2020-03-18 5:55 p.m., Kamil Rytarowski wrote: >> @@ -313,8 +325,12 @@ inf_ptrace_target::kill () >> target_mourn_inferior (inferior_ptid); >> } >> >> +#ifndef __NetBSD__ >> /* Return which PID to pass to ptrace in order to observe/control the >> - tracee identified by PTID. */ >> + tracee identified by PTID. >> + >> + Unlike most other Operating Systems, NetBSD tracks both pid and lwp >> + and avoids this function. */ > > To align this function with our current way of documenting functions, please move > this comment to the declaration of get_ptrace_pid, in inf-ptrace.h, and put this here: > > /* See inf-ptrace.h. */ > Done. >> >> pid_t >> get_ptrace_pid (ptid_t ptid) >> @@ -328,6 +344,7 @@ get_ptrace_pid (ptid_t ptid) >> pid = ptid.pid (); >> return pid; >> } >> +#endif >> >> /* Resume execution of thread PTID, or all threads if PTID is -1. If >> STEP is nonzero, single-step it. If SIGNAL is nonzero, give it >> @@ -336,15 +353,12 @@ get_ptrace_pid (ptid_t ptid) >> void >> inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) >> { >> - pid_t pid; >> int request; >> >> if (minus_one_ptid == ptid) >> /* Resume all threads. Traditionally ptrace() only supports >> single-threaded processes, so simply resume the inferior. */ >> - pid = inferior_ptid.pid (); >> - else >> - pid = get_ptrace_pid (ptid); >> + ptid = inferior_ptid; > > I'm not certain about this part (which I suggested). With the existing code, > if inferior_ptid is {pid=10, lwp=20}, we'll call ptrace with pid=10. With the > patch, we'll call ptrace with pid=20. > > I'm not entirely sure if/when resume can be called with ptid == minus_one_ptid. > And if it's called with minus_one_ptid, is it possible that ptid.pid != ptid.lwp? > > In any case, it's probably safer to do: > > ptid = ptid_t (inferior_ptid.pid ()); > > to retain the existing behavior. WDYT? > It's OK. >> >> if (catch_syscall_enabled () > 0) >> request = PT_SYSCALL; >> @@ -365,7 +379,7 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) >> where it was. If GDB wanted it to start some other way, we have >> already written a new program counter value to the child. */ >> errno = 0; >> - ptrace (request, pid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); >> + gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); >> if (errno != 0) >> perror_with_name (("ptrace")); >> } >> @@ -528,7 +542,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, >> const gdb_byte *writebuf, >> ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) >> { >> - pid_t pid = get_ptrace_pid (inferior_ptid); >> + ptid_t ptid = inferior_ptid; >> >> switch (object) >> { >> @@ -552,7 +566,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, >> piod.piod_len = len; >> >> errno = 0; >> - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) >> + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) >> { >> /* Return the actual number of bytes read or written. */ >> *xfered_len = piod.piod_len; >> @@ -565,7 +579,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, >> return TARGET_XFER_EOF; >> } >> #endif >> - *xfered_len = inf_ptrace_peek_poke (pid, readbuf, writebuf, >> + *xfered_len = inf_ptrace_peek_poke (ptid.pid (), readbuf, writebuf, >> offset, len); >> return *xfered_len != 0 ? TARGET_XFER_OK : TARGET_XFER_EOF; > > I'm pretty sure that this change is not right. For non-NetBSD targets, this > used to pass `inferior_ptid.lwp` (the result of get_ptrace_pid). Now, we pass > `inferior_ptid.pid`. > OK. > I think you'll need to pass the ptid to inf_ptrace_peek_poke and use gdb_ptrace > inside that function too. > Done. > Simon > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3] Disable get_ptrace_pid for NetBSD 2020-03-18 21:55 ` [PATCH v2] " Kamil Rytarowski 2020-03-18 22:21 ` Simon Marchi @ 2020-03-18 23:16 ` Kamil Rytarowski 2020-03-19 1:13 ` Simon Marchi 2020-03-19 12:28 ` [PATCH v4] " Kamil Rytarowski 1 sibling, 2 replies; 23+ messages in thread From: Kamil Rytarowski @ 2020-03-18 23:16 UTC (permalink / raw) To: gdb-patches; +Cc: tom, Kamil Rytarowski Unlike most other Operating Systems, NetBSD tracks both pid and lwp. The process id on NetBSD is stored always in the pid field of ptid. gdb/ChangeLog: * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. * inf-ptrace.c: Likewise. * (gdb_ptrace): Add. * (inf_ptrace_target::resume): Update. * (inf_ptrace_target::xfer_partial): Likewise. * (inf_ptrace_peek_poke): Change argument `pid' to `ptid'. * (inf_ptrace_peek_poke): Update. --- gdb/ChangeLog | 10 ++++++++++ gdb/inf-ptrace.c | 45 +++++++++++++++++++++++++++++---------------- gdb/inf-ptrace.h | 7 ++++++- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 84964dc00ac..f09b4d154c5 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,13 @@ +2020-03-18 Kamil Rytarowski <n54@gmx.com> + + * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. + * inf-ptrace.c: Likewise. + * (gdb_ptrace): Add. + * (inf_ptrace_target::resume): Update. + * (inf_ptrace_target::xfer_partial): Likewise. + * (inf_ptrace_peek_poke): Change argument `pid' to `ptid'. + * (inf_ptrace_peek_poke): Update. + 2020-03-17 Kamil Rytarowski <n54@gmx.com> * regformats/regdef.h: Put reg in gdb namespace. diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c index db17a76d946..4003b888616 100644 --- a/gdb/inf-ptrace.c +++ b/gdb/inf-ptrace.c @@ -37,6 +37,18 @@ \f +static int +gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG3 addr, + PTRACE_TYPE_ARG4 data) +{ +#ifdef __NetBSD__ + return ptrace (request, ptid.pid (), addr, data); +#else + pid_t pid = get_ptrace_pid (ptid); + return ptrace (request, pid, addr, data); +#endif +} + /* A unique_ptr helper to unpush a target. */ struct target_unpusher @@ -313,8 +325,9 @@ inf_ptrace_target::kill () target_mourn_inferior (inferior_ptid); } -/* Return which PID to pass to ptrace in order to observe/control the - tracee identified by PTID. */ +#ifndef __NetBSD__ + +/* See inf-ptrace.h. */ pid_t get_ptrace_pid (ptid_t ptid) @@ -328,6 +341,7 @@ get_ptrace_pid (ptid_t ptid) pid = ptid.pid (); return pid; } +#endif /* Resume execution of thread PTID, or all threads if PTID is -1. If STEP is nonzero, single-step it. If SIGNAL is nonzero, give it @@ -336,15 +350,14 @@ get_ptrace_pid (ptid_t ptid) void inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) { - pid_t pid; int request; if (minus_one_ptid == ptid) /* Resume all threads. Traditionally ptrace() only supports single-threaded processes, so simply resume the inferior. */ - pid = inferior_ptid.pid (); + ptid = inferior_ptid; else - pid = get_ptrace_pid (ptid); + ptid = ptid_t (inferior_ptid.pid ()); if (catch_syscall_enabled () > 0) request = PT_SYSCALL; @@ -365,7 +378,7 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) where it was. If GDB wanted it to start some other way, we have already written a new program counter value to the child. */ errno = 0; - ptrace (request, pid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); + gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); if (errno != 0) perror_with_name (("ptrace")); } @@ -460,7 +473,7 @@ inf_ptrace_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus, be non-null. Return the number of transferred bytes. */ static ULONGEST -inf_ptrace_peek_poke (pid_t pid, gdb_byte *readbuf, +inf_ptrace_peek_poke (ptid_t ptid, gdb_byte *readbuf, const gdb_byte *writebuf, ULONGEST addr, ULONGEST len) { @@ -491,8 +504,8 @@ inf_ptrace_peek_poke (pid_t pid, gdb_byte *readbuf, if (readbuf != NULL || chunk < sizeof (PTRACE_TYPE_RET)) { errno = 0; - buf.word = ptrace (PT_READ_I, pid, - (PTRACE_TYPE_ARG3)(uintptr_t) addr, 0); + buf.word = gdb_ptrace (PT_READ_I, ptid, + (PTRACE_TYPE_ARG3)(uintptr_t) addr, 0); if (errno != 0) break; if (readbuf != NULL) @@ -502,15 +515,15 @@ inf_ptrace_peek_poke (pid_t pid, gdb_byte *readbuf, { memcpy (buf.byte + skip, writebuf + n, chunk); errno = 0; - ptrace (PT_WRITE_D, pid, (PTRACE_TYPE_ARG3)(uintptr_t) addr, + gdb_ptrace (PT_WRITE_D, ptid, (PTRACE_TYPE_ARG3)(uintptr_t) addr, buf.word); if (errno != 0) { /* Using the appropriate one (I or D) is necessary for Gould NP1, at least. */ errno = 0; - ptrace (PT_WRITE_I, pid, (PTRACE_TYPE_ARG3)(uintptr_t) addr, - buf.word); + gdb_ptrace (PT_WRITE_I, ptid, (PTRACE_TYPE_ARG3)(uintptr_t) addr, + buf.word); if (errno != 0) break; } @@ -528,7 +541,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, const gdb_byte *writebuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) { - pid_t pid = get_ptrace_pid (inferior_ptid); + ptid_t ptid = inferior_ptid; switch (object) { @@ -552,7 +565,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, piod.piod_len = len; errno = 0; - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) { /* Return the actual number of bytes read or written. */ *xfered_len = piod.piod_len; @@ -565,7 +578,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, return TARGET_XFER_EOF; } #endif - *xfered_len = inf_ptrace_peek_poke (pid, readbuf, writebuf, + *xfered_len = inf_ptrace_peek_poke (ptid, readbuf, writebuf, offset, len); return *xfered_len != 0 ? TARGET_XFER_OK : TARGET_XFER_EOF; @@ -588,7 +601,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, piod.piod_len = len; errno = 0; - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) { /* Return the actual number of bytes read or written. */ *xfered_len = piod.piod_len; diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h index dd0733736f2..dea82d005e3 100644 --- a/gdb/inf-ptrace.h +++ b/gdb/inf-ptrace.h @@ -78,9 +78,14 @@ struct inf_ptrace_target : public inf_child_target void detach_success (inferior *inf); }; +#ifndef __NetBSD__ /* Return which PID to pass to ptrace in order to observe/control the - tracee identified by PTID. */ + tracee identified by PTID. + + Unlike most other Operating Systems, NetBSD tracks both pid and lwp + and avoids this function. */ extern pid_t get_ptrace_pid (ptid_t); +#endif #endif -- 2.25.0 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3] Disable get_ptrace_pid for NetBSD 2020-03-18 23:16 ` [PATCH v3] " Kamil Rytarowski @ 2020-03-19 1:13 ` Simon Marchi 2020-03-19 12:01 ` Kamil Rytarowski 2020-03-19 12:28 ` [PATCH v4] " Kamil Rytarowski 1 sibling, 1 reply; 23+ messages in thread From: Simon Marchi @ 2020-03-19 1:13 UTC (permalink / raw) To: Kamil Rytarowski, gdb-patches; +Cc: tom On 2020-03-18 7:16 p.m., Kamil Rytarowski wrote: > @@ -336,15 +350,14 @@ get_ptrace_pid (ptid_t ptid) > void > inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) > { > - pid_t pid; > int request; > > if (minus_one_ptid == ptid) > /* Resume all threads. Traditionally ptrace() only supports > single-threaded processes, so simply resume the inferior. */ > - pid = inferior_ptid.pid (); > + ptid = inferior_ptid; > else > - pid = get_ptrace_pid (ptid); > + ptid = ptid_t (inferior_ptid.pid ()); That's not what I meant. To keep the existing behavior, I believe it should be: if (minus_one_ptid == ptid) /* Resume all threads. Traditionally ptrace() only supports single-threaded processes, so simply resume the inferior. */ ptid = ptid_t (inferior_ptid.pid ()); > > if (catch_syscall_enabled () > 0) > request = PT_SYSCALL; > @@ -365,7 +378,7 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) > where it was. If GDB wanted it to start some other way, we have > already written a new program counter value to the child. */ > errno = 0; > - ptrace (request, pid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); > + gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); > if (errno != 0) > perror_with_name (("ptrace")); > } I'm getting this: /home/simark/src/binutils-gdb/gdb/inf-ptrace.c: In member function ‘virtual void inf_ptrace_target::resume(ptid_t, int, gdb_signal)’: /home/simark/src/binutils-gdb/gdb/inf-ptrace.c:379:15: error: invalid conversion from ‘int’ to ‘__ptrace_request’ [-fpermissive] 379 | gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); | ^~~~~~~ | | | int /home/simark/src/binutils-gdb/gdb/inf-ptrace.c:41:30: note: initializing argument 1 of ‘int gdb_ptrace(__ptrace_request, ptid_t, long int, long int)’ 41 | gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG3 addr, We would need to change the type of the variable `ret` to PTRACE_TYPE_ARG1. I'll make a test run on Linux with those changes to check if there's any regression. Simon ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3] Disable get_ptrace_pid for NetBSD 2020-03-19 1:13 ` Simon Marchi @ 2020-03-19 12:01 ` Kamil Rytarowski 0 siblings, 0 replies; 23+ messages in thread From: Kamil Rytarowski @ 2020-03-19 12:01 UTC (permalink / raw) To: Simon Marchi, gdb-patches; +Cc: tom [-- Attachment #1.1: Type: text/plain, Size: 2526 bytes --] On 19.03.2020 02:13, Simon Marchi wrote: > On 2020-03-18 7:16 p.m., Kamil Rytarowski wrote: >> @@ -336,15 +350,14 @@ get_ptrace_pid (ptid_t ptid) >> void >> inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) >> { >> - pid_t pid; >> int request; >> >> if (minus_one_ptid == ptid) >> /* Resume all threads. Traditionally ptrace() only supports >> single-threaded processes, so simply resume the inferior. */ >> - pid = inferior_ptid.pid (); >> + ptid = inferior_ptid; >> else >> - pid = get_ptrace_pid (ptid); >> + ptid = ptid_t (inferior_ptid.pid ()); > > That's not what I meant. To keep the existing behavior, I believe it should be: > > if (minus_one_ptid == ptid) > /* Resume all threads. Traditionally ptrace() only supports > single-threaded processes, so simply resume the inferior. */ > ptid = ptid_t (inferior_ptid.pid ()); > OK. >> >> if (catch_syscall_enabled () > 0) >> request = PT_SYSCALL; >> @@ -365,7 +378,7 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) >> where it was. If GDB wanted it to start some other way, we have >> already written a new program counter value to the child. */ >> errno = 0; >> - ptrace (request, pid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); >> + gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); >> if (errno != 0) >> perror_with_name (("ptrace")); >> } > > I'm getting this: > > /home/simark/src/binutils-gdb/gdb/inf-ptrace.c: In member function ‘virtual void inf_ptrace_target::resume(ptid_t, int, gdb_signal)’: > /home/simark/src/binutils-gdb/gdb/inf-ptrace.c:379:15: error: invalid conversion from ‘int’ to ‘__ptrace_request’ [-fpermissive] > 379 | gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); > | ^~~~~~~ > | | > | int > /home/simark/src/binutils-gdb/gdb/inf-ptrace.c:41:30: note: initializing argument 1 of ‘int gdb_ptrace(__ptrace_request, ptid_t, long int, long int)’ > 41 | gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG3 addr, > > > We would need to change the type of the variable `ret` to PTRACE_TYPE_ARG1. > > I'll make a test run on Linux with those changes to check if there's any regression. > I presume you mean variable `request', not `ret'. Done. > Simon > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4] Disable get_ptrace_pid for NetBSD 2020-03-18 23:16 ` [PATCH v3] " Kamil Rytarowski 2020-03-19 1:13 ` Simon Marchi @ 2020-03-19 12:28 ` Kamil Rytarowski 2020-03-19 12:55 ` Simon Marchi 2020-03-19 15:45 ` [PATCH v5] " Kamil Rytarowski 1 sibling, 2 replies; 23+ messages in thread From: Kamil Rytarowski @ 2020-03-19 12:28 UTC (permalink / raw) To: gdb-patches; +Cc: simark, tom, Kamil Rytarowski Unlike most other Operating Systems, NetBSD tracks both pid and lwp. The process id on NetBSD is stored always in the pid field of ptid. gdb/ChangeLog: * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. * inf-ptrace.c: Likewise. * (gdb_ptrace): Add. * (inf_ptrace_target::resume): Update. * (inf_ptrace_target::xfer_partial): Likewise. * (inf_ptrace_peek_poke): Change argument `pid' to `ptid'. * (inf_ptrace_peek_poke): Update. --- gdb/ChangeLog | 10 ++++++++++ gdb/inf-ptrace.c | 47 +++++++++++++++++++++++++++++------------------ gdb/inf-ptrace.h | 7 ++++++- 3 files changed, 45 insertions(+), 19 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 7f87eceaf70..78c6893cfe8 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,13 @@ +2020-03-19 Kamil Rytarowski <n54@gmx.com> + + * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. + * inf-ptrace.c: Likewise. + * (gdb_ptrace): Add. + * (inf_ptrace_target::resume): Update. + * (inf_ptrace_target::xfer_partial): Likewise. + * (inf_ptrace_peek_poke): Change argument `pid' to `ptid'. + * (inf_ptrace_peek_poke): Update. + 2020-03-19 Andrew Burgess <andrew.burgess@embecosm.com> * remote.c (remote_target::process_stop_reply): Handle events for diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c index db17a76d946..6c338cd5d14 100644 --- a/gdb/inf-ptrace.c +++ b/gdb/inf-ptrace.c @@ -37,6 +37,18 @@ \f +static int +gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG3 addr, + PTRACE_TYPE_ARG4 data) +{ +#ifdef __NetBSD__ + return ptrace (request, ptid.pid (), addr, data); +#else + pid_t pid = get_ptrace_pid (ptid); + return ptrace (request, pid, addr, data); +#endif +} + /* A unique_ptr helper to unpush a target. */ struct target_unpusher @@ -313,8 +325,9 @@ inf_ptrace_target::kill () target_mourn_inferior (inferior_ptid); } -/* Return which PID to pass to ptrace in order to observe/control the - tracee identified by PTID. */ +#ifndef __NetBSD__ + +/* See inf-ptrace.h. */ pid_t get_ptrace_pid (ptid_t ptid) @@ -328,6 +341,7 @@ get_ptrace_pid (ptid_t ptid) pid = ptid.pid (); return pid; } +#endif /* Resume execution of thread PTID, or all threads if PTID is -1. If STEP is nonzero, single-step it. If SIGNAL is nonzero, give it @@ -336,15 +350,12 @@ get_ptrace_pid (ptid_t ptid) void inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) { - pid_t pid; - int request; + PTRACE_TYPE_ARG1 request; if (minus_one_ptid == ptid) /* Resume all threads. Traditionally ptrace() only supports single-threaded processes, so simply resume the inferior. */ - pid = inferior_ptid.pid (); - else - pid = get_ptrace_pid (ptid); + ptid = ptid_t (inferior_ptid.pid ()); if (catch_syscall_enabled () > 0) request = PT_SYSCALL; @@ -365,7 +376,7 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) where it was. If GDB wanted it to start some other way, we have already written a new program counter value to the child. */ errno = 0; - ptrace (request, pid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); + gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); if (errno != 0) perror_with_name (("ptrace")); } @@ -460,7 +471,7 @@ inf_ptrace_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus, be non-null. Return the number of transferred bytes. */ static ULONGEST -inf_ptrace_peek_poke (pid_t pid, gdb_byte *readbuf, +inf_ptrace_peek_poke (ptid_t ptid, gdb_byte *readbuf, const gdb_byte *writebuf, ULONGEST addr, ULONGEST len) { @@ -491,8 +502,8 @@ inf_ptrace_peek_poke (pid_t pid, gdb_byte *readbuf, if (readbuf != NULL || chunk < sizeof (PTRACE_TYPE_RET)) { errno = 0; - buf.word = ptrace (PT_READ_I, pid, - (PTRACE_TYPE_ARG3)(uintptr_t) addr, 0); + buf.word = gdb_ptrace (PT_READ_I, ptid, + (PTRACE_TYPE_ARG3)(uintptr_t) addr, 0); if (errno != 0) break; if (readbuf != NULL) @@ -502,15 +513,15 @@ inf_ptrace_peek_poke (pid_t pid, gdb_byte *readbuf, { memcpy (buf.byte + skip, writebuf + n, chunk); errno = 0; - ptrace (PT_WRITE_D, pid, (PTRACE_TYPE_ARG3)(uintptr_t) addr, + gdb_ptrace (PT_WRITE_D, ptid, (PTRACE_TYPE_ARG3)(uintptr_t) addr, buf.word); if (errno != 0) { /* Using the appropriate one (I or D) is necessary for Gould NP1, at least. */ errno = 0; - ptrace (PT_WRITE_I, pid, (PTRACE_TYPE_ARG3)(uintptr_t) addr, - buf.word); + gdb_ptrace (PT_WRITE_I, ptid, (PTRACE_TYPE_ARG3)(uintptr_t) addr, + buf.word); if (errno != 0) break; } @@ -528,7 +539,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, const gdb_byte *writebuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) { - pid_t pid = get_ptrace_pid (inferior_ptid); + ptid_t ptid = inferior_ptid; switch (object) { @@ -552,7 +563,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, piod.piod_len = len; errno = 0; - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) { /* Return the actual number of bytes read or written. */ *xfered_len = piod.piod_len; @@ -565,7 +576,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, return TARGET_XFER_EOF; } #endif - *xfered_len = inf_ptrace_peek_poke (pid, readbuf, writebuf, + *xfered_len = inf_ptrace_peek_poke (ptid, readbuf, writebuf, offset, len); return *xfered_len != 0 ? TARGET_XFER_OK : TARGET_XFER_EOF; @@ -588,7 +599,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, piod.piod_len = len; errno = 0; - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) { /* Return the actual number of bytes read or written. */ *xfered_len = piod.piod_len; diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h index dd0733736f2..dea82d005e3 100644 --- a/gdb/inf-ptrace.h +++ b/gdb/inf-ptrace.h @@ -78,9 +78,14 @@ struct inf_ptrace_target : public inf_child_target void detach_success (inferior *inf); }; +#ifndef __NetBSD__ /* Return which PID to pass to ptrace in order to observe/control the - tracee identified by PTID. */ + tracee identified by PTID. + + Unlike most other Operating Systems, NetBSD tracks both pid and lwp + and avoids this function. */ extern pid_t get_ptrace_pid (ptid_t); +#endif #endif -- 2.25.0 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] Disable get_ptrace_pid for NetBSD 2020-03-19 12:28 ` [PATCH v4] " Kamil Rytarowski @ 2020-03-19 12:55 ` Simon Marchi 2020-03-19 15:30 ` Simon Marchi 2020-03-19 15:45 ` [PATCH v5] " Kamil Rytarowski 1 sibling, 1 reply; 23+ messages in thread From: Simon Marchi @ 2020-03-19 12:55 UTC (permalink / raw) To: Kamil Rytarowski, gdb-patches; +Cc: tom On 2020-03-19 8:28 a.m., Kamil Rytarowski wrote: > Unlike most other Operating Systems, NetBSD tracks both pid and lwp. > The process id on NetBSD is stored always in the pid field of ptid. > > gdb/ChangeLog: > > * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. > * inf-ptrace.c: Likewise. > * (gdb_ptrace): Add. > * (inf_ptrace_target::resume): Update. > * (inf_ptrace_target::xfer_partial): Likewise. > * (inf_ptrace_peek_poke): Change argument `pid' to `ptid'. > * (inf_ptrace_peek_poke): Update. Hmm, this breaks simple debugging on Linux: $ ./gdb --data-directory=data-directory a.out -ex start Reading symbols from a.out... Temporary breakpoint 1 at 0x4004da: file test.c, line 2. Starting program: /home/smarchi/build/binutils-gdb/gdb/a.out Program received signal SIGILL, Illegal instruction. 0x00007ffff7dda96d in dl_main (phdr=<optimized out>, phnum=<optimized out>, user_entry=<optimized out>, auxv=<optimized out>) at rtld.c:1517 1517 rtld.c: No such file or directory. I haven't figured out why by inspecting the code yet, I'll try to debug it later today. Simon ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] Disable get_ptrace_pid for NetBSD 2020-03-19 12:55 ` Simon Marchi @ 2020-03-19 15:30 ` Simon Marchi 2020-03-19 15:51 ` Kamil Rytarowski 2020-03-20 15:50 ` Tom Tromey 0 siblings, 2 replies; 23+ messages in thread From: Simon Marchi @ 2020-03-19 15:30 UTC (permalink / raw) To: Kamil Rytarowski, gdb-patches; +Cc: tom On 2020-03-19 8:55 a.m., Simon Marchi wrote: > On 2020-03-19 8:28 a.m., Kamil Rytarowski wrote: >> Unlike most other Operating Systems, NetBSD tracks both pid and lwp. >> The process id on NetBSD is stored always in the pid field of ptid. >> >> gdb/ChangeLog: >> >> * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. >> * inf-ptrace.c: Likewise. >> * (gdb_ptrace): Add. >> * (inf_ptrace_target::resume): Update. >> * (inf_ptrace_target::xfer_partial): Likewise. >> * (inf_ptrace_peek_poke): Change argument `pid' to `ptid'. >> * (inf_ptrace_peek_poke): Update. > > Hmm, this breaks simple debugging on Linux: > > $ ./gdb --data-directory=data-directory a.out -ex start > Reading symbols from a.out... > Temporary breakpoint 1 at 0x4004da: file test.c, line 2. > Starting program: /home/smarchi/build/binutils-gdb/gdb/a.out > > Program received signal SIGILL, Illegal instruction. > 0x00007ffff7dda96d in dl_main (phdr=<optimized out>, phnum=<optimized out>, user_entry=<optimized out>, auxv=<optimized out>) at rtld.c:1517 > 1517 rtld.c: No such file or directory. > > > I haven't figured out why by inspecting the code yet, I'll try to debug it later today. > > Simon > Ah, it's because ptrace returns long and not int on GNU/Linux. So when we want to read a 64-bits word from memory, it gets truncated. gdb_ptrace should return PTRACE_TYPE_RET. In fact, to be consistent, all these gdb_ptrace functions should be changed to return PTRACE_TYPE_RET (as a separate patch). Simon ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] Disable get_ptrace_pid for NetBSD 2020-03-19 15:30 ` Simon Marchi @ 2020-03-19 15:51 ` Kamil Rytarowski 2020-03-20 15:50 ` Tom Tromey 1 sibling, 0 replies; 23+ messages in thread From: Kamil Rytarowski @ 2020-03-19 15:51 UTC (permalink / raw) To: Simon Marchi, gdb-patches; +Cc: tom [-- Attachment #1.1: Type: text/plain, Size: 1706 bytes --] On 19.03.2020 16:30, Simon Marchi wrote: > On 2020-03-19 8:55 a.m., Simon Marchi wrote: >> On 2020-03-19 8:28 a.m., Kamil Rytarowski wrote: >>> Unlike most other Operating Systems, NetBSD tracks both pid and lwp. >>> The process id on NetBSD is stored always in the pid field of ptid. >>> >>> gdb/ChangeLog: >>> >>> * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. >>> * inf-ptrace.c: Likewise. >>> * (gdb_ptrace): Add. >>> * (inf_ptrace_target::resume): Update. >>> * (inf_ptrace_target::xfer_partial): Likewise. >>> * (inf_ptrace_peek_poke): Change argument `pid' to `ptid'. >>> * (inf_ptrace_peek_poke): Update. >> >> Hmm, this breaks simple debugging on Linux: >> >> $ ./gdb --data-directory=data-directory a.out -ex start >> Reading symbols from a.out... >> Temporary breakpoint 1 at 0x4004da: file test.c, line 2. >> Starting program: /home/smarchi/build/binutils-gdb/gdb/a.out >> >> Program received signal SIGILL, Illegal instruction. >> 0x00007ffff7dda96d in dl_main (phdr=<optimized out>, phnum=<optimized out>, user_entry=<optimized out>, auxv=<optimized out>) at rtld.c:1517 >> 1517 rtld.c: No such file or directory. >> >> >> I haven't figured out why by inspecting the code yet, I'll try to debug it later today. >> >> Simon >> > > Ah, it's because ptrace returns long and not int on GNU/Linux. So when we want to read > a 64-bits word from memory, it gets truncated. gdb_ptrace should return PTRACE_TYPE_RET. > > In fact, to be consistent, all these gdb_ptrace functions should be changed to return > PTRACE_TYPE_RET (as a separate patch). > Done in v5. I will follow up with other gdb_ptrace() instances next. > Simon > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4] Disable get_ptrace_pid for NetBSD 2020-03-19 15:30 ` Simon Marchi 2020-03-19 15:51 ` Kamil Rytarowski @ 2020-03-20 15:50 ` Tom Tromey 1 sibling, 0 replies; 23+ messages in thread From: Tom Tromey @ 2020-03-20 15:50 UTC (permalink / raw) To: Simon Marchi; +Cc: Kamil Rytarowski, gdb-patches, tom >>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: Simon> Ah, it's because ptrace returns long and not int on GNU/Linux. Simon> So when we want to read a 64-bits word from memory, it gets Simon> truncated. gdb_ptrace should return PTRACE_TYPE_RET. FWIW it's also fine to just return long from our wrapper; at least if all known ptrace implementations return some integer type. Simon> In fact, to be consistent, all these gdb_ptrace functions should Simon> be changed to return PTRACE_TYPE_RET (as a separate patch). Now that we have C++ we could probably get rid of these autoconf checks and just use the template instantiation or overloading tricks. Tom ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v5] Disable get_ptrace_pid for NetBSD 2020-03-19 12:28 ` [PATCH v4] " Kamil Rytarowski 2020-03-19 12:55 ` Simon Marchi @ 2020-03-19 15:45 ` Kamil Rytarowski 2020-03-19 19:27 ` Simon Marchi 1 sibling, 1 reply; 23+ messages in thread From: Kamil Rytarowski @ 2020-03-19 15:45 UTC (permalink / raw) To: gdb-patches; +Cc: simark, Kamil Rytarowski Unlike most other Operating Systems, NetBSD tracks both pid and lwp. The process id on NetBSD is stored always in the pid field of ptid. gdb/ChangeLog: * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. * inf-ptrace.c: Likewise. * (gdb_ptrace): Add. * (inf_ptrace_target::resume): Update. * (inf_ptrace_target::xfer_partial): Likewise. * (inf_ptrace_peek_poke): Change argument `pid' to `ptid'. * (inf_ptrace_peek_poke): Update. --- gdb/ChangeLog | 9 +++++++++ gdb/inf-ptrace.c | 47 +++++++++++++++++++++++++++++------------------ gdb/inf-ptrace.h | 7 ++++++- 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 0955d648e79..c5119224415 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,12 @@ +2020-03-19 Kamil Rytarowski <n54@gmx.com> + * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. + * inf-ptrace.c: Likewise. + * (gdb_ptrace): Add. + * (inf_ptrace_target::resume): Update. + * (inf_ptrace_target::xfer_partial): Likewise. + * (inf_ptrace_peek_poke): Change argument `pid' to `ptid'. + * (inf_ptrace_peek_poke): Update. + 2020-03-19 Kamil Rytarowski <n54@gmx.com> * x86-bsd-nat.c (gdb_ptrace): New. diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c index db17a76d946..a6a77ef9d31 100644 --- a/gdb/inf-ptrace.c +++ b/gdb/inf-ptrace.c @@ -37,6 +37,18 @@ \f +static PTRACE_TYPE_RET +gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG3 addr, + PTRACE_TYPE_ARG4 data) +{ +#ifdef __NetBSD__ + return ptrace (request, ptid.pid (), addr, data); +#else + pid_t pid = get_ptrace_pid (ptid); + return ptrace (request, pid, addr, data); +#endif +} + /* A unique_ptr helper to unpush a target. */ struct target_unpusher @@ -313,8 +325,9 @@ inf_ptrace_target::kill () target_mourn_inferior (inferior_ptid); } -/* Return which PID to pass to ptrace in order to observe/control the - tracee identified by PTID. */ +#ifndef __NetBSD__ + +/* See inf-ptrace.h. */ pid_t get_ptrace_pid (ptid_t ptid) @@ -328,6 +341,7 @@ get_ptrace_pid (ptid_t ptid) pid = ptid.pid (); return pid; } +#endif /* Resume execution of thread PTID, or all threads if PTID is -1. If STEP is nonzero, single-step it. If SIGNAL is nonzero, give it @@ -336,15 +350,12 @@ get_ptrace_pid (ptid_t ptid) void inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) { - pid_t pid; - int request; + PTRACE_TYPE_ARG1 request; if (minus_one_ptid == ptid) /* Resume all threads. Traditionally ptrace() only supports single-threaded processes, so simply resume the inferior. */ - pid = inferior_ptid.pid (); - else - pid = get_ptrace_pid (ptid); + ptid = ptid_t (inferior_ptid.pid ()); if (catch_syscall_enabled () > 0) request = PT_SYSCALL; @@ -365,7 +376,7 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) where it was. If GDB wanted it to start some other way, we have already written a new program counter value to the child. */ errno = 0; - ptrace (request, pid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); + gdb_ptrace (request, ptid, (PTRACE_TYPE_ARG3)1, gdb_signal_to_host (signal)); if (errno != 0) perror_with_name (("ptrace")); } @@ -460,7 +471,7 @@ inf_ptrace_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus, be non-null. Return the number of transferred bytes. */ static ULONGEST -inf_ptrace_peek_poke (pid_t pid, gdb_byte *readbuf, +inf_ptrace_peek_poke (ptid_t ptid, gdb_byte *readbuf, const gdb_byte *writebuf, ULONGEST addr, ULONGEST len) { @@ -491,8 +502,8 @@ inf_ptrace_peek_poke (pid_t pid, gdb_byte *readbuf, if (readbuf != NULL || chunk < sizeof (PTRACE_TYPE_RET)) { errno = 0; - buf.word = ptrace (PT_READ_I, pid, - (PTRACE_TYPE_ARG3)(uintptr_t) addr, 0); + buf.word = gdb_ptrace (PT_READ_I, ptid, + (PTRACE_TYPE_ARG3)(uintptr_t) addr, 0); if (errno != 0) break; if (readbuf != NULL) @@ -502,15 +513,15 @@ inf_ptrace_peek_poke (pid_t pid, gdb_byte *readbuf, { memcpy (buf.byte + skip, writebuf + n, chunk); errno = 0; - ptrace (PT_WRITE_D, pid, (PTRACE_TYPE_ARG3)(uintptr_t) addr, + gdb_ptrace (PT_WRITE_D, ptid, (PTRACE_TYPE_ARG3)(uintptr_t) addr, buf.word); if (errno != 0) { /* Using the appropriate one (I or D) is necessary for Gould NP1, at least. */ errno = 0; - ptrace (PT_WRITE_I, pid, (PTRACE_TYPE_ARG3)(uintptr_t) addr, - buf.word); + gdb_ptrace (PT_WRITE_I, ptid, (PTRACE_TYPE_ARG3)(uintptr_t) addr, + buf.word); if (errno != 0) break; } @@ -528,7 +539,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, const gdb_byte *writebuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) { - pid_t pid = get_ptrace_pid (inferior_ptid); + ptid_t ptid = inferior_ptid; switch (object) { @@ -552,7 +563,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, piod.piod_len = len; errno = 0; - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) { /* Return the actual number of bytes read or written. */ *xfered_len = piod.piod_len; @@ -565,7 +576,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, return TARGET_XFER_EOF; } #endif - *xfered_len = inf_ptrace_peek_poke (pid, readbuf, writebuf, + *xfered_len = inf_ptrace_peek_poke (ptid, readbuf, writebuf, offset, len); return *xfered_len != 0 ? TARGET_XFER_OK : TARGET_XFER_EOF; @@ -588,7 +599,7 @@ inf_ptrace_target::xfer_partial (enum target_object object, piod.piod_len = len; errno = 0; - if (ptrace (PT_IO, pid, (caddr_t)&piod, 0) == 0) + if (gdb_ptrace (PT_IO, ptid, (caddr_t)&piod, 0) == 0) { /* Return the actual number of bytes read or written. */ *xfered_len = piod.piod_len; diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h index dd0733736f2..dea82d005e3 100644 --- a/gdb/inf-ptrace.h +++ b/gdb/inf-ptrace.h @@ -78,9 +78,14 @@ struct inf_ptrace_target : public inf_child_target void detach_success (inferior *inf); }; +#ifndef __NetBSD__ /* Return which PID to pass to ptrace in order to observe/control the - tracee identified by PTID. */ + tracee identified by PTID. + + Unlike most other Operating Systems, NetBSD tracks both pid and lwp + and avoids this function. */ extern pid_t get_ptrace_pid (ptid_t); +#endif #endif -- 2.25.0 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5] Disable get_ptrace_pid for NetBSD 2020-03-19 15:45 ` [PATCH v5] " Kamil Rytarowski @ 2020-03-19 19:27 ` Simon Marchi 0 siblings, 0 replies; 23+ messages in thread From: Simon Marchi @ 2020-03-19 19:27 UTC (permalink / raw) To: Kamil Rytarowski, gdb-patches On 2020-03-19 11:45 a.m., Kamil Rytarowski wrote: > Unlike most other Operating Systems, NetBSD tracks both pid and lwp. > The process id on NetBSD is stored always in the pid field of ptid. I did a test run and it looks fine, this patch is ok to push. Simon ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] Disable get_ptrace_pid for NetBSD @ 2020-03-18 16:29 Kamil Rytarowski 2020-03-18 20:48 ` Tom Tromey 2020-03-18 21:15 ` Simon Marchi 0 siblings, 2 replies; 23+ messages in thread From: Kamil Rytarowski @ 2020-03-18 16:29 UTC (permalink / raw) To: gdb-patches; +Cc: simark, Kamil Rytarowski Unlike most other Operating Systems, NetBSD tracks both pid and lwp. The process id on NetBSD is stored always in the pid field of ptid. gdb/ChangeLog: * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. * inf-ptrace.c: Likewise. * (inf_ptrace_target::resume): Update. * (inf_ptrace_target::xfer_partial): Likewise. --- gdb/ChangeLog | 7 +++++++ gdb/inf-ptrace.c | 19 +++++++++++++++++-- gdb/inf-ptrace.h | 2 ++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 84964dc00ac..36e333ec8ba 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2020-03-18 Kamil Rytarowski <n54@gmx.com> + + * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. + * inf-ptrace.c: Likewise. + * (inf_ptrace_target::resume): Update. + * (inf_ptrace_target::xfer_partial): Likewise. + 2020-03-17 Kamil Rytarowski <n54@gmx.com> * regformats/regdef.h: Put reg in gdb namespace. diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c index db17a76d946..e8bf66ed45f 100644 --- a/gdb/inf-ptrace.c +++ b/gdb/inf-ptrace.c @@ -313,8 +313,12 @@ inf_ptrace_target::kill () target_mourn_inferior (inferior_ptid); } +#ifndef __NetBSD__ /* Return which PID to pass to ptrace in order to observe/control the - tracee identified by PTID. */ + tracee identified by PTID. + + Unlike most other Operating Systems, NetBSD tracks both pid and lwp + and avoids this function. */ pid_t get_ptrace_pid (ptid_t ptid) @@ -328,6 +332,7 @@ get_ptrace_pid (ptid_t ptid) pid = ptid.pid (); return pid; } +#endif /* Resume execution of thread PTID, or all threads if PTID is -1. If STEP is nonzero, single-step it. If SIGNAL is nonzero, give it @@ -344,7 +349,13 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) single-threaded processes, so simply resume the inferior. */ pid = inferior_ptid.pid (); else - pid = get_ptrace_pid (ptid); + { +#ifdef __NetBSD__ + pid = ptid. pid(); +#else + pid = get_ptrace_pid (ptid); +#endif + } if (catch_syscall_enabled () > 0) request = PT_SYSCALL; @@ -528,7 +539,11 @@ inf_ptrace_target::xfer_partial (enum target_object object, const gdb_byte *writebuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) { +#ifdef __NetBSD__ + pid_t pid = inferior_ptid. pid(); +#else pid_t pid = get_ptrace_pid (inferior_ptid); +#endif switch (object) { diff --git a/gdb/inf-ptrace.h b/gdb/inf-ptrace.h index dd0733736f2..340d41d8beb 100644 --- a/gdb/inf-ptrace.h +++ b/gdb/inf-ptrace.h @@ -78,9 +78,11 @@ struct inf_ptrace_target : public inf_child_target void detach_success (inferior *inf); }; +#ifndef __NetBSD__ /* Return which PID to pass to ptrace in order to observe/control the tracee identified by PTID. */ extern pid_t get_ptrace_pid (ptid_t); +#endif #endif -- 2.25.0 ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Disable get_ptrace_pid for NetBSD 2020-03-18 16:29 [PATCH] " Kamil Rytarowski @ 2020-03-18 20:48 ` Tom Tromey 2020-03-18 20:54 ` Tom Tromey 2020-03-18 21:15 ` Simon Marchi 1 sibling, 1 reply; 23+ messages in thread From: Tom Tromey @ 2020-03-18 20:48 UTC (permalink / raw) To: Kamil Rytarowski; +Cc: gdb-patches, simark >>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes: Kamil> Unlike most other Operating Systems, NetBSD tracks both pid and lwp. Kamil> The process id on NetBSD is stored always in the pid field of ptid. Kamil> gdb/ChangeLog: Kamil> * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. Kamil> * inf-ptrace.c: Likewise. Kamil> * (inf_ptrace_target::resume): Update. Kamil> * (inf_ptrace_target::xfer_partial): Likewise. Kamil> +#ifndef __NetBSD__ Kamil> /* Return which PID to pass to ptrace in order to observe/control the Kamil> - tracee identified by PTID. */ Kamil> + tracee identified by PTID. Kamil> + Kamil> + Unlike most other Operating Systems, NetBSD tracks both pid and lwp Kamil> + and avoids this function. */ Kamil> pid_t Kamil> get_ptrace_pid (ptid_t ptid) Kamil> @@ -328,6 +332,7 @@ get_ptrace_pid (ptid_t ptid) Kamil> pid = ptid.pid (); Kamil> return pid; Why not make just the body of this function #ifdef __NetBSD__ return ptid.pid () #else ... old code That would mean fewer #ifs. Tom ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Disable get_ptrace_pid for NetBSD 2020-03-18 20:48 ` Tom Tromey @ 2020-03-18 20:54 ` Tom Tromey 2020-03-18 21:22 ` Simon Marchi 0 siblings, 1 reply; 23+ messages in thread From: Tom Tromey @ 2020-03-18 20:54 UTC (permalink / raw) To: Tom Tromey; +Cc: Kamil Rytarowski, simark, gdb-patches >>>>> "Tom" == Tom Tromey <tom@tromey.com> writes: Tom> Why not make just the body of this function Tom> #ifdef __NetBSD__ Tom> return ptid.pid () Tom> #else Tom> ... old code Tom> That would mean fewer #ifs. I see that's what you did originally and Simon requested this version. Personally I think the new patch is uglier than the original, on the basis that use of "#if" generally makes the code harder to understand. Tom ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Disable get_ptrace_pid for NetBSD 2020-03-18 20:54 ` Tom Tromey @ 2020-03-18 21:22 ` Simon Marchi 0 siblings, 0 replies; 23+ messages in thread From: Simon Marchi @ 2020-03-18 21:22 UTC (permalink / raw) To: Tom Tromey; +Cc: Kamil Rytarowski, gdb-patches On 2020-03-18 4:54 p.m., Tom Tromey wrote: >>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes: > > Tom> Why not make just the body of this function > > Tom> #ifdef __NetBSD__ > Tom> return ptid.pid () > Tom> #else > Tom> ... old code > > Tom> That would mean fewer #ifs. > > I see that's what you did originally and Simon requested this version. > > Personally I think the new patch is uglier than the original, on the > basis that use of "#if" generally makes the code harder to understand. > > Tom > Indeed, I suggested that. I am hoping that we can isolate the ifdefs in these "gdb_ptrace" functions (we should probably find a better name for those...), to which you pass a "ptid" and do the right thing according to the current OS. I agree that adding ifdefs in the more complex code is ugly and less readable. If my suggestion doesn't work well in practice, then feel free to revert to your original solution, I certainly don't want to impose it if it turns out not to be a good idea. Simon ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Disable get_ptrace_pid for NetBSD 2020-03-18 16:29 [PATCH] " Kamil Rytarowski 2020-03-18 20:48 ` Tom Tromey @ 2020-03-18 21:15 ` Simon Marchi 2020-03-18 21:40 ` Kamil Rytarowski 1 sibling, 1 reply; 23+ messages in thread From: Simon Marchi @ 2020-03-18 21:15 UTC (permalink / raw) To: Kamil Rytarowski, gdb-patches; +Cc: Tom Tromey On 2020-03-18 12:29 p.m., Kamil Rytarowski wrote: > Unlike most other Operating Systems, NetBSD tracks both pid and lwp. > The process id on NetBSD is stored always in the pid field of ptid. > > gdb/ChangeLog: > > * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. > * inf-ptrace.c: Likewise. > * (inf_ptrace_target::resume): Update. > * (inf_ptrace_target::xfer_partial): Likewise. > --- > gdb/ChangeLog | 7 +++++++ > gdb/inf-ptrace.c | 19 +++++++++++++++++-- > gdb/inf-ptrace.h | 2 ++ > 3 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 84964dc00ac..36e333ec8ba 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,10 @@ > +2020-03-18 Kamil Rytarowski <n54@gmx.com> > + > + * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. > + * inf-ptrace.c: Likewise. > + * (inf_ptrace_target::resume): Update. > + * (inf_ptrace_target::xfer_partial): Likewise. > + > 2020-03-17 Kamil Rytarowski <n54@gmx.com> > > * regformats/regdef.h: Put reg in gdb namespace. > diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c > index db17a76d946..e8bf66ed45f 100644 > --- a/gdb/inf-ptrace.c > +++ b/gdb/inf-ptrace.c > @@ -313,8 +313,12 @@ inf_ptrace_target::kill () > target_mourn_inferior (inferior_ptid); > } > > +#ifndef __NetBSD__ > /* Return which PID to pass to ptrace in order to observe/control the > - tracee identified by PTID. */ > + tracee identified by PTID. > + > + Unlike most other Operating Systems, NetBSD tracks both pid and lwp > + and avoids this function. */ > > pid_t > get_ptrace_pid (ptid_t ptid) > @@ -328,6 +332,7 @@ get_ptrace_pid (ptid_t ptid) > pid = ptid.pid (); > return pid; > } > +#endif > > /* Resume execution of thread PTID, or all threads if PTID is -1. If > STEP is nonzero, single-step it. If SIGNAL is nonzero, give it > @@ -344,7 +349,13 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) > single-threaded processes, so simply resume the inferior. */ > pid = inferior_ptid.pid (); > else > - pid = get_ptrace_pid (ptid); > + { > +#ifdef __NetBSD__ > + pid = ptid. pid(); > +#else > + pid = get_ptrace_pid (ptid); > +#endif > + } To avoid the ifdefs in this more complex code, can't you use a gdb_ptrace function like in the other patches? This time, it would look like: static int gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG3 addr, PTRACE_TYPE_ARG4 data) { #ifdef __NetBSD__ return ptrace (request, ptid.pid (), addr, data); #else pid_t pid = get_ptrace_pid (ptid); return ptrace (request, pid, addr, data); #endif } Simon ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Disable get_ptrace_pid for NetBSD 2020-03-18 21:15 ` Simon Marchi @ 2020-03-18 21:40 ` Kamil Rytarowski 2020-03-18 21:43 ` Simon Marchi 0 siblings, 1 reply; 23+ messages in thread From: Kamil Rytarowski @ 2020-03-18 21:40 UTC (permalink / raw) To: Simon Marchi, gdb-patches; +Cc: Tom Tromey [-- Attachment #1.1: Type: text/plain, Size: 3092 bytes --] On 18.03.2020 22:15, Simon Marchi wrote: > On 2020-03-18 12:29 p.m., Kamil Rytarowski wrote: >> Unlike most other Operating Systems, NetBSD tracks both pid and lwp. >> The process id on NetBSD is stored always in the pid field of ptid. >> >> gdb/ChangeLog: >> >> * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. >> * inf-ptrace.c: Likewise. >> * (inf_ptrace_target::resume): Update. >> * (inf_ptrace_target::xfer_partial): Likewise. >> --- >> gdb/ChangeLog | 7 +++++++ >> gdb/inf-ptrace.c | 19 +++++++++++++++++-- >> gdb/inf-ptrace.h | 2 ++ >> 3 files changed, 26 insertions(+), 2 deletions(-) >> >> diff --git a/gdb/ChangeLog b/gdb/ChangeLog >> index 84964dc00ac..36e333ec8ba 100644 >> --- a/gdb/ChangeLog >> +++ b/gdb/ChangeLog >> @@ -1,3 +1,10 @@ >> +2020-03-18 Kamil Rytarowski <n54@gmx.com> >> + >> + * inf-ptrace.h: Disable get_ptrace_pid on NetBSD. >> + * inf-ptrace.c: Likewise. >> + * (inf_ptrace_target::resume): Update. >> + * (inf_ptrace_target::xfer_partial): Likewise. >> + >> 2020-03-17 Kamil Rytarowski <n54@gmx.com> >> >> * regformats/regdef.h: Put reg in gdb namespace. >> diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c >> index db17a76d946..e8bf66ed45f 100644 >> --- a/gdb/inf-ptrace.c >> +++ b/gdb/inf-ptrace.c >> @@ -313,8 +313,12 @@ inf_ptrace_target::kill () >> target_mourn_inferior (inferior_ptid); >> } >> >> +#ifndef __NetBSD__ >> /* Return which PID to pass to ptrace in order to observe/control the >> - tracee identified by PTID. */ >> + tracee identified by PTID. >> + >> + Unlike most other Operating Systems, NetBSD tracks both pid and lwp >> + and avoids this function. */ >> >> pid_t >> get_ptrace_pid (ptid_t ptid) >> @@ -328,6 +332,7 @@ get_ptrace_pid (ptid_t ptid) >> pid = ptid.pid (); >> return pid; >> } >> +#endif >> >> /* Resume execution of thread PTID, or all threads if PTID is -1. If >> STEP is nonzero, single-step it. If SIGNAL is nonzero, give it >> @@ -344,7 +349,13 @@ inf_ptrace_target::resume (ptid_t ptid, int step, enum gdb_signal signal) >> single-threaded processes, so simply resume the inferior. */ >> pid = inferior_ptid.pid (); >> else >> - pid = get_ptrace_pid (ptid); >> + { >> +#ifdef __NetBSD__ >> + pid = ptid. pid(); >> +#else >> + pid = get_ptrace_pid (ptid); >> +#endif >> + } > > To avoid the ifdefs in this more complex code, can't you use a gdb_ptrace function like in > the other patches? This time, it would look like: > > static int > gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG3 addr, > PTRACE_TYPE_ARG4 data) > { > #ifdef __NetBSD__ > return ptrace (request, ptid.pid (), addr, data); > #else > pid_t pid = get_ptrace_pid (ptid); > return ptrace (request, pid, addr, data); > #endif > } > > Simon > I will go for it, but only for a selection of ptrace () calls in this file. In certain places there would need to be need to compose ptid_t out of plain pid. Another case is PT_TRACE_ME. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Disable get_ptrace_pid for NetBSD 2020-03-18 21:40 ` Kamil Rytarowski @ 2020-03-18 21:43 ` Simon Marchi 2020-03-18 21:45 ` Kamil Rytarowski 0 siblings, 1 reply; 23+ messages in thread From: Simon Marchi @ 2020-03-18 21:43 UTC (permalink / raw) To: Kamil Rytarowski, gdb-patches; +Cc: Tom Tromey On 2020-03-18 5:40 p.m., Kamil Rytarowski wrote: > I will go for it, but only for a selection of ptrace () calls in this file. > > In certain places there would need to be need to compose ptid_t out of > plain pid. Another case is PT_TRACE_ME. My understanding for those is that there's no difference between NetBSD and the other platforms. Simon ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] Disable get_ptrace_pid for NetBSD 2020-03-18 21:43 ` Simon Marchi @ 2020-03-18 21:45 ` Kamil Rytarowski 0 siblings, 0 replies; 23+ messages in thread From: Kamil Rytarowski @ 2020-03-18 21:45 UTC (permalink / raw) To: Simon Marchi, gdb-patches; +Cc: Tom Tromey [-- Attachment #1.1: Type: text/plain, Size: 480 bytes --] On 18.03.2020 22:43, Simon Marchi wrote: > On 2020-03-18 5:40 p.m., Kamil Rytarowski wrote: >> I will go for it, but only for a selection of ptrace () calls in this file. >> >> In certain places there would need to be need to compose ptid_t out of >> plain pid. Another case is PT_TRACE_ME. > > My understanding for those is that there's no difference between NetBSD and > the other platforms. > This is correct, at least in the usage out there. > Simon > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2020-03-20 16:02 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-18 21:49 [PATCH] Disable get_ptrace_pid for NetBSD Kamil Rytarowski 2020-03-18 21:51 ` Kamil Rytarowski 2020-03-18 21:55 ` [PATCH v2] " Kamil Rytarowski 2020-03-18 22:21 ` Simon Marchi 2020-03-18 23:30 ` Kamil Rytarowski 2020-03-18 23:16 ` [PATCH v3] " Kamil Rytarowski 2020-03-19 1:13 ` Simon Marchi 2020-03-19 12:01 ` Kamil Rytarowski 2020-03-19 12:28 ` [PATCH v4] " Kamil Rytarowski 2020-03-19 12:55 ` Simon Marchi 2020-03-19 15:30 ` Simon Marchi 2020-03-19 15:51 ` Kamil Rytarowski 2020-03-20 15:50 ` Tom Tromey 2020-03-19 15:45 ` [PATCH v5] " Kamil Rytarowski 2020-03-19 19:27 ` Simon Marchi -- strict thread matches above, loose matches on Subject: below -- 2020-03-18 16:29 [PATCH] " Kamil Rytarowski 2020-03-18 20:48 ` Tom Tromey 2020-03-18 20:54 ` Tom Tromey 2020-03-18 21:22 ` Simon Marchi 2020-03-18 21:15 ` Simon Marchi 2020-03-18 21:40 ` Kamil Rytarowski 2020-03-18 21:43 ` Simon Marchi 2020-03-18 21:45 ` Kamil Rytarowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox