[PATCH 0/3] Fix some ARM NetBSD compile errors

[PATCH 0/3] Fix some ARM NetBSD compile errors

[cbiesinger]

From: Christian Biesinger <cbiesinger@google.com>

I tried compiling GDB for ARM NetBSD (long story) and found some compile
errors.  This fixes some of them.

Christian Biesinger (3):
  Support the NetBSD version of pthread_setname_np
  Define _KMEMUSER in arm-nbsd-nat.c
  Make the class name in the definition match the declaration

 gdb/arm-nbsd-nat.c       |  5 +++--
 gdbsupport/thread-pool.c | 12 ++++++++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.25.0.341.g760bfbb309-goog

[PATCH 3/3] Make the class name in the definition match the declaration

[cbiesinger]

From: Christian Biesinger <cbiesinger@google.com>

Fixes a compile error because the class is actually called
arm_netbsd_nat_target.

gdb/ChangeLog:

2020-01-24  Christian Biesinger  <cbiesinger@google.com>

	* arm-nbsd-nat.c (arm_nbsd_nat_target::fetch_registers): Rename to...
	(arm_netbsd_nat_target::fetch_registers): ...this.
	(arm_nbsd_nat_target::store_registers): Rename to...
	(arm_netbsd_nat_target::store_registers): ...this.

Change-Id: Ibebfab9edeff48f54c32d0745afda1d74d31de92
---
 gdb/arm-nbsd-nat.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
index 31fe66c724..5072bd09ec 100644
--- a/gdb/arm-nbsd-nat.c
+++ b/gdb/arm-nbsd-nat.c
@@ -191,7 +191,7 @@ fetch_fp_regs (struct regcache *regcache)
 }
 
 void
-arm_nbsd_nat_target::fetch_registers (struct regcache *regcache, int regno)
+arm_netbsd_nat_target::fetch_registers (struct regcache *regcache, int regno)
 {
   if (regno >= 0)
     {
@@ -380,7 +380,7 @@ store_fp_regs (const struct regcache *regcache)
 }
 
 void
-arm_nbsd_nat_target::store_registers (struct regcache *regcache, int regno)
+arm_netbsd_nat_target::store_registers (struct regcache *regcache, int regno)
 {
   if (regno >= 0)
     {
-- 
2.25.0.341.g760bfbb309-goog

Re: [PATCH 3/3] Make the class name in the definition match the declaration

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 1521 bytes --]

On 24.01.2020 15:14, cbiesinger@chromium.org wrote:
> From: Christian Biesinger <cbiesinger@google.com>
> 
> Fixes a compile error because the class is actually called
> arm_netbsd_nat_target.
> 
> gdb/ChangeLog:
> 
> 2020-01-24  Christian Biesinger  <cbiesinger@google.com>
> 
> 	* arm-nbsd-nat.c (arm_nbsd_nat_target::fetch_registers): Rename to...
> 	(arm_netbsd_nat_target::fetch_registers): ...this.
> 	(arm_nbsd_nat_target::store_registers): Rename to...
> 	(arm_netbsd_nat_target::store_registers): ...this.
> 

Looks fine.

Personally I am for unwinding the 'nbsd' term in GDB more widely in more
places.

> Change-Id: Ibebfab9edeff48f54c32d0745afda1d74d31de92
> ---
>  gdb/arm-nbsd-nat.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
> index 31fe66c724..5072bd09ec 100644
> --- a/gdb/arm-nbsd-nat.c
> +++ b/gdb/arm-nbsd-nat.c
> @@ -191,7 +191,7 @@ fetch_fp_regs (struct regcache *regcache)
>  }
>  
>  void
> -arm_nbsd_nat_target::fetch_registers (struct regcache *regcache, int regno)
> +arm_netbsd_nat_target::fetch_registers (struct regcache *regcache, int regno)
>  {
>    if (regno >= 0)
>      {
> @@ -380,7 +380,7 @@ store_fp_regs (const struct regcache *regcache)
>  }
>  
>  void
> -arm_nbsd_nat_target::store_registers (struct regcache *regcache, int regno)
> +arm_netbsd_nat_target::store_registers (struct regcache *regcache, int regno)
>  {
>    if (regno >= 0)
>      {
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/3] Make the class name in the definition match the declaration

[Tom Tromey]

>>>>> ">" == cbiesinger  <cbiesinger@chromium.org> writes:

>> 2020-01-24  Christian Biesinger  <cbiesinger@google.com>

>> 	* arm-nbsd-nat.c (arm_nbsd_nat_target::fetch_registers): Rename to...
>> 	(arm_netbsd_nat_target::fetch_registers): ...this.
>> 	(arm_nbsd_nat_target::store_registers): Rename to...
>> 	(arm_netbsd_nat_target::store_registers): ...this.

This one counts as obvious IMO.
Thank you for doing this.

Tom

Re: [PATCH 3/3] Make the class name in the definition match the declaration

[Christian Biesinger via gdb-patches]

On Fri, Jan 24, 2020 at 4:04 PM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> ">" == cbiesinger  <cbiesinger@chromium.org> writes:
>
> >> 2020-01-24  Christian Biesinger  <cbiesinger@google.com>
>
> >>      * arm-nbsd-nat.c (arm_nbsd_nat_target::fetch_registers): Rename to...
> >>      (arm_netbsd_nat_target::fetch_registers): ...this.
> >>      (arm_nbsd_nat_target::store_registers): Rename to...
> >>      (arm_netbsd_nat_target::store_registers): ...this.
>
> This one counts as obvious IMO.
> Thank you for doing this.

Thanks, will push this patch series. I figured it was easier to group
all the patches together.

Christian

[PATCH 1/3] Support the NetBSD version of pthread_setname_np

[cbiesinger]

From: Christian Biesinger <cbiesinger@google.com>

On NetBSD, pthread_setname_np takes a printf-style format string plus
one argument:
https://netbsd.gw.com/cgi-bin/man-cgi?pthread_setname_np++NetBSD-current

This patch makes thread-pool.c handle that.

gdbsupport/ChangeLog:

2020-01-24  Christian Biesinger  <cbiesinger@google.com>

	* thread-pool.c (set_thread_name): Add an overload for the NetBSD
	version of pthread_setname_np.

Change-Id: I61e664a813eaa7f52b6811b1a43e08ac3082d8ef
---
 gdbsupport/thread-pool.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdbsupport/thread-pool.c b/gdbsupport/thread-pool.c
index fc83ff765f..be9ca22682 100644
--- a/gdbsupport/thread-pool.c
+++ b/gdbsupport/thread-pool.c
@@ -40,8 +40,16 @@
 #include <pthread.h>
 
 /* Handle platform discrepancies in pthread_setname_np: macOS uses a
-   single-argument form, while Linux uses a two-argument form.  This
-   wrapper handles the difference.  */
+   single-argument form, while Linux uses a two-argument form.  NetBSD
+   takes a printf-style format and an argument.  This wrapper handles the
+   difference.  */
+
+ATTRIBUTE_UNUSED static void
+set_thread_name (int (*set_name) (pthread_t, const char *, void *),
+				  const char *name)
+{
+  set_name (pthread_self (), "%s", const_cast<char *> (name));
+}
 
 ATTRIBUTE_UNUSED static void
 set_thread_name (int (*set_name) (pthread_t, const char *), const char *name)
-- 
2.25.0.341.g760bfbb309-goog

Re: [PATCH 1/3] Support the NetBSD version of pthread_setname_np

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 1654 bytes --]

On 24.01.2020 15:14, cbiesinger@chromium.org wrote:
> From: Christian Biesinger <cbiesinger@google.com>
> 
> On NetBSD, pthread_setname_np takes a printf-style format string plus
> one argument:
> https://netbsd.gw.com/cgi-bin/man-cgi?pthread_setname_np++NetBSD-current
> 
> This patch makes thread-pool.c handle that.
> 
> gdbsupport/ChangeLog:
> 
> 2020-01-24  Christian Biesinger  <cbiesinger@google.com>
> 
> 	* thread-pool.c (set_thread_name): Add an overload for the NetBSD
> 	version of pthread_setname_np.
> 

Looks fine.

> Change-Id: I61e664a813eaa7f52b6811b1a43e08ac3082d8ef
> ---
>  gdbsupport/thread-pool.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/gdbsupport/thread-pool.c b/gdbsupport/thread-pool.c
> index fc83ff765f..be9ca22682 100644
> --- a/gdbsupport/thread-pool.c
> +++ b/gdbsupport/thread-pool.c
> @@ -40,8 +40,16 @@
>  #include <pthread.h>
>  
>  /* Handle platform discrepancies in pthread_setname_np: macOS uses a
> -   single-argument form, while Linux uses a two-argument form.  This
> -   wrapper handles the difference.  */
> +   single-argument form, while Linux uses a two-argument form.  NetBSD
> +   takes a printf-style format and an argument.  This wrapper handles the
> +   difference.  */
> +
> +ATTRIBUTE_UNUSED static void
> +set_thread_name (int (*set_name) (pthread_t, const char *, void *),
> +				  const char *name)
> +{
> +  set_name (pthread_self (), "%s", const_cast<char *> (name));
> +}
>  
>  ATTRIBUTE_UNUSED static void
>  set_thread_name (int (*set_name) (pthread_t, const char *), const char *name)
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/3] Support the NetBSD version of pthread_setname_np

[Tom Tromey]

>>>>> ">" == cbiesinger  <cbiesinger@chromium.org> writes:

>> From: Christian Biesinger <cbiesinger@google.com>
>> On NetBSD, pthread_setname_np takes a printf-style format string plus
>> one argument:
>> https://netbsd.gw.com/cgi-bin/man-cgi?pthread_setname_np++NetBSD-current

>> This patch makes thread-pool.c handle that.

>> gdbsupport/ChangeLog:

>> 2020-01-24  Christian Biesinger  <cbiesinger@google.com>

>> 	* thread-pool.c (set_thread_name): Add an overload for the NetBSD
>> 	version of pthread_setname_np.

Thank you.  This is ok.

Tom

[PATCH 2/3] Define _KMEMUSER in arm-nbsd-nat.c

[cbiesinger]

From: Christian Biesinger <cbiesinger@google.com>

Fixes the below compile error on ARM NetBSD 9.0_RC1 (the only version I
tested).  types.h does not define register_t by default.

We already use this define elsewhere, notably in bsd-kvm.c.

In file included from ../../gdb/arm-nbsd-nat.c:28:
/usr/include/machine/frame.h:54:2: error: unknown type name 'register_t'; did you mean '__register_t'?
        register_t tf_spsr;
        ^
/usr/include/machine/types.h:77:14: note: '__register_t' declared here
typedef int             __register_t;
                        ^

There are other compile errors that this does not fix.

gdb/ChangeLog:

2020-01-24  Christian Biesinger  <cbiesinger@google.com>

	* arm-nbsd-nat.c: Define _KMEMUSER to get the declaration of
	register_t.

Change-Id: I82c21d38189ee59ea0af2538ba84b771d268722e
---
 gdb/arm-nbsd-nat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
index 00f919194b..31fe66c724 100644
--- a/gdb/arm-nbsd-nat.c
+++ b/gdb/arm-nbsd-nat.c
@@ -17,6 +17,7 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+
 #include "defs.h"
 #include "gdbcore.h"
 #include "inferior.h"
-- 
2.25.0.341.g760bfbb309-goog

[PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c

[cbiesinger]

From: Christian Biesinger <cbiesinger@google.com>

Fixes the below compile error on ARM NetBSD 9.0_RC1 (the only version I
tested).  types.h does not define register_t by default.

We already use this define elsewhere, notably in bsd-kvm.c.

In file included from ../../gdb/arm-nbsd-nat.c:28:
/usr/include/machine/frame.h:54:2: error: unknown type name 'register_t'; did you mean '__register_t'?
        register_t tf_spsr;
        ^
/usr/include/machine/types.h:77:14: note: '__register_t' declared here
typedef int             __register_t;
                        ^

There are other compile errors that this does not fix.

gdb/ChangeLog:

2020-01-24  Christian Biesinger  <cbiesinger@google.com>

	* arm-nbsd-nat.c: Define _KMEMUSER to get the declaration of
	register_t.

Change-Id: I82c21d38189ee59ea0af2538ba84b771d268722e
---
 gdb/arm-nbsd-nat.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
index 00f919194b..4844b51a3c 100644
--- a/gdb/arm-nbsd-nat.c
+++ b/gdb/arm-nbsd-nat.c
@@ -17,6 +17,8 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+/* We define this to get types like register_t.  */
+#define _KMEMUSER
 #include "defs.h"
 #include "gdbcore.h"
 #include "inferior.h"
-- 
2.25.0.341.g760bfbb309-goog

Re: [PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 1659 bytes --]

On 24.01.2020 15:18, cbiesinger@chromium.org wrote:
> From: Christian Biesinger <cbiesinger@google.com>
> 
> Fixes the below compile error on ARM NetBSD 9.0_RC1 (the only version I
> tested).  types.h does not define register_t by default.
> 
> We already use this define elsewhere, notably in bsd-kvm.c.
> 
> In file included from ../../gdb/arm-nbsd-nat.c:28:
> /usr/include/machine/frame.h:54:2: error: unknown type name 'register_t'; did you mean '__register_t'?
>         register_t tf_spsr;
>         ^
> /usr/include/machine/types.h:77:14: note: '__register_t' declared here
> typedef int             __register_t;
>                         ^
> 
> There are other compile errors that this does not fix.
> 
> gdb/ChangeLog:
> 
> 2020-01-24  Christian Biesinger  <cbiesinger@google.com>
> 
> 	* arm-nbsd-nat.c: Define _KMEMUSER to get the declaration of
> 	register_t.
> 
> Change-Id: I82c21d38189ee59ea0af2538ba84b771d268722e
> ---
>  gdb/arm-nbsd-nat.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
> index 00f919194b..4844b51a3c 100644
> --- a/gdb/arm-nbsd-nat.c
> +++ b/gdb/arm-nbsd-nat.c
> @@ -17,6 +17,8 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
> +/* We define this to get types like register_t.  */
> +#define _KMEMUSER
>  #include "defs.h"
>  #include "gdbcore.h"
>  #include "inferior.h"
> 

While gdb is the right user for _KMEMUSER, here we should probably go
for -D_KERNTYPES as it is the canonical symbol for register_t.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c

[Christian Biesinger via gdb-patches]

On Fri, Jan 24, 2020 at 3:29 PM Kamil Rytarowski <n54@gmx.com> wrote:
>
> On 24.01.2020 15:18, cbiesinger@chromium.org wrote:
> > From: Christian Biesinger <cbiesinger@google.com>
> >
> > Fixes the below compile error on ARM NetBSD 9.0_RC1 (the only version I
> > tested).  types.h does not define register_t by default.
> >
> > We already use this define elsewhere, notably in bsd-kvm.c.
> >
> > In file included from ../../gdb/arm-nbsd-nat.c:28:
> > /usr/include/machine/frame.h:54:2: error: unknown type name 'register_t'; did you mean '__register_t'?
> >         register_t tf_spsr;
> >         ^
> > /usr/include/machine/types.h:77:14: note: '__register_t' declared here
> > typedef int             __register_t;
> >                         ^
> >
> > There are other compile errors that this does not fix.
> >
> > gdb/ChangeLog:
> >
> > 2020-01-24  Christian Biesinger  <cbiesinger@google.com>
> >
> >       * arm-nbsd-nat.c: Define _KMEMUSER to get the declaration of
> >       register_t.
> >
> > Change-Id: I82c21d38189ee59ea0af2538ba84b771d268722e
> > ---
> >  gdb/arm-nbsd-nat.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
> > index 00f919194b..4844b51a3c 100644
> > --- a/gdb/arm-nbsd-nat.c
> > +++ b/gdb/arm-nbsd-nat.c
> > @@ -17,6 +17,8 @@
> >     You should have received a copy of the GNU General Public License
> >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> >
> > +/* We define this to get types like register_t.  */
> > +#define _KMEMUSER
> >  #include "defs.h"
> >  #include "gdbcore.h"
> >  #include "inferior.h"
> >
>
> While gdb is the right user for _KMEMUSER, here we should probably go
> for -D_KERNTYPES as it is the canonical symbol for register_t.

Ah thank you! I was hoping someone who knows this stuff would respond.
I will send a new patch shortly.

Christian

[PATCH v3 2/3] Define _KERNTYPES in arm-nbsd-nat.c

[cbiesinger]

From: Christian Biesinger <cbiesinger@google.com>

Fixes the below compile error on ARM NetBSD 9.0_RC1 (the only version I
tested).  types.h does not define register_t by default.

We already use this define elsewhere, notably in bsd-kvm.c.

In file included from ../../gdb/arm-nbsd-nat.c:28:
/usr/include/machine/frame.h:54:2: error: unknown type name 'register_t'; did you mean '__register_t'?
        register_t tf_spsr;
        ^
/usr/include/machine/types.h:77:14: note: '__register_t' declared here
typedef int             __register_t;
                        ^

There are other compile errors that this does not fix.

gdb/ChangeLog:

2020-01-24  Christian Biesinger  <cbiesinger@google.com>

	* arm-nbsd-nat.c: Define _KERNTYPES to get the declaration of
	register_t.

Change-Id: I82c21d38189ee59ea0af2538ba84b771d268722e
---
 gdb/arm-nbsd-nat.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
index 00f919194b..33ae790063 100644
--- a/gdb/arm-nbsd-nat.c
+++ b/gdb/arm-nbsd-nat.c
@@ -17,6 +17,8 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+/* We define this to get types like register_t.  */
+#define _KERNTYPES
 #include "defs.h"
 #include "gdbcore.h"
 #include "inferior.h"
-- 
2.25.0.341.g760bfbb309-goog

Re: [PATCH v3 2/3] Define _KERNTYPES in arm-nbsd-nat.c

[Tom Tromey]

>>>>> ">" == cbiesinger  <cbiesinger@chromium.org> writes:

>> 2020-01-24  Christian Biesinger  <cbiesinger@google.com>

>> 	* arm-nbsd-nat.c: Define _KERNTYPES to get the declaration of
>> 	register_t.

Thanks, this is ok.

Tom

Re: [PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c

[Christian Biesinger via gdb-patches]

Hi Kamil,

I have a related question. NetBSD applied this patch:
https://www.mail-archive.com/tech@openbsd.org/msg44100.html

Do you know which NetBSD version that shipped in? Can we apply that
patch to GDB as-is or should we attempt to support the older struct
layout as well?

Thanks,
Christian

On Fri, Jan 24, 2020 at 3:29 PM Kamil Rytarowski <n54@gmx.com> wrote:
>
> On 24.01.2020 15:18, cbiesinger@chromium.org wrote:
> > From: Christian Biesinger <cbiesinger@google.com>
> >
> > Fixes the below compile error on ARM NetBSD 9.0_RC1 (the only version I
> > tested).  types.h does not define register_t by default.
> >
> > We already use this define elsewhere, notably in bsd-kvm.c.
> >
> > In file included from ../../gdb/arm-nbsd-nat.c:28:
> > /usr/include/machine/frame.h:54:2: error: unknown type name 'register_t'; did you mean '__register_t'?
> >         register_t tf_spsr;
> >         ^
> > /usr/include/machine/types.h:77:14: note: '__register_t' declared here
> > typedef int             __register_t;
> >                         ^
> >
> > There are other compile errors that this does not fix.
> >
> > gdb/ChangeLog:
> >
> > 2020-01-24  Christian Biesinger  <cbiesinger@google.com>
> >
> >       * arm-nbsd-nat.c: Define _KMEMUSER to get the declaration of
> >       register_t.
> >
> > Change-Id: I82c21d38189ee59ea0af2538ba84b771d268722e
> > ---
> >  gdb/arm-nbsd-nat.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
> > index 00f919194b..4844b51a3c 100644
> > --- a/gdb/arm-nbsd-nat.c
> > +++ b/gdb/arm-nbsd-nat.c
> > @@ -17,6 +17,8 @@
> >     You should have received a copy of the GNU General Public License
> >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> >
> > +/* We define this to get types like register_t.  */
> > +#define _KMEMUSER
> >  #include "defs.h"
> >  #include "gdbcore.h"
> >  #include "inferior.h"
> >
>
> While gdb is the right user for _KMEMUSER, here we should probably go
> for -D_KERNTYPES as it is the canonical symbol for register_t.
>

Re: [PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 3100 bytes --]

On 24.01.2020 15:53, Christian Biesinger via gdb-patches wrote:
> Hi Kamil,
> 
> I have a related question. NetBSD applied this patch:
> https://www.mail-archive.com/tech@openbsd.org/msg44100.html
> 

Is this the right link?

> Do you know which NetBSD version that shipped in? Can we apply that
> patch to GDB as-is or should we attempt to support the older struct
> layout as well?

Please go for the current FPU layout on NetBSD. Massive ptrace(2) fixes
were introduced in NetBSD-8 and later. Soon NetBSD 7.x will go EOL
(after releasing 9.0, rc2 is planned soon).

In LLDB we support NetBSD 9.0 or newer. In GDB we should keep the same
minimal requirements and deal with older NetBSD versions (if at all)
with downstream patches.

We have got a pile of local GDB patches.

There is also a functional gdbserver implementation on NetBSD/amd64 and
I intend to upstream it. (Help wanted! Would you be interested in this
and in upstreaming?)

The patches are located here:

https://github.com/NetBSD/pkgsrc-wip/tree/master/gdb-netbsd/patches

* with core/basic features... but it is difficult as there is no OS with
finished transition...
https://sourceware.org/gdb/wiki/LocalRemoteFeatureParity

> 
> Thanks,
> Christian
> 
> On Fri, Jan 24, 2020 at 3:29 PM Kamil Rytarowski <n54@gmx.com> wrote:
>>
>> On 24.01.2020 15:18, cbiesinger@chromium.org wrote:
>>> From: Christian Biesinger <cbiesinger@google.com>
>>>
>>> Fixes the below compile error on ARM NetBSD 9.0_RC1 (the only version I
>>> tested).  types.h does not define register_t by default.
>>>
>>> We already use this define elsewhere, notably in bsd-kvm.c.
>>>
>>> In file included from ../../gdb/arm-nbsd-nat.c:28:
>>> /usr/include/machine/frame.h:54:2: error: unknown type name 'register_t'; did you mean '__register_t'?
>>>         register_t tf_spsr;
>>>         ^
>>> /usr/include/machine/types.h:77:14: note: '__register_t' declared here
>>> typedef int             __register_t;
>>>                         ^
>>>
>>> There are other compile errors that this does not fix.
>>>
>>> gdb/ChangeLog:
>>>
>>> 2020-01-24  Christian Biesinger  <cbiesinger@google.com>
>>>
>>>       * arm-nbsd-nat.c: Define _KMEMUSER to get the declaration of
>>>       register_t.
>>>
>>> Change-Id: I82c21d38189ee59ea0af2538ba84b771d268722e
>>> ---
>>>  gdb/arm-nbsd-nat.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
>>> index 00f919194b..4844b51a3c 100644
>>> --- a/gdb/arm-nbsd-nat.c
>>> +++ b/gdb/arm-nbsd-nat.c
>>> @@ -17,6 +17,8 @@
>>>     You should have received a copy of the GNU General Public License
>>>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>>
>>> +/* We define this to get types like register_t.  */
>>> +#define _KMEMUSER
>>>  #include "defs.h"
>>>  #include "gdbcore.h"
>>>  #include "inferior.h"
>>>
>>
>> While gdb is the right user for _KMEMUSER, here we should probably go
>> for -D_KERNTYPES as it is the canonical symbol for register_t.
>>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c

[Christian Biesinger via gdb-patches]

On Fri, Jan 24, 2020 at 4:23 PM Kamil Rytarowski <n54@gmx.com> wrote:
>
> On 24.01.2020 15:53, Christian Biesinger via gdb-patches wrote:
> > Hi Kamil,
> >
> > I have a related question. NetBSD applied this patch:
> > https://www.mail-archive.com/tech@openbsd.org/msg44100.html
> >
>
> Is this the right link?

Yeah -- that patch changes a system header at the top and patches GDB
at the bottom.

> > Do you know which NetBSD version that shipped in? Can we apply that
> > patch to GDB as-is or should we attempt to support the older struct
> > layout as well?
>
> Please go for the current FPU layout on NetBSD. Massive ptrace(2) fixes
> were introduced in NetBSD-8 and later. Soon NetBSD 7.x will go EOL
> (after releasing 9.0, rc2 is planned soon).

OK, great. Thanks.

> In LLDB we support NetBSD 9.0 or newer. In GDB we should keep the same
> minimal requirements and deal with older NetBSD versions (if at all)
> with downstream patches.
>
> We have got a pile of local GDB patches.

OK. Maybe I should look through those at some point...  I was
surprised that NetBSD apparently has an oldish GDB if
http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/devel/gdb/README.html
is correct (8.1)

> There is also a functional gdbserver implementation on NetBSD/amd64 and
> I intend to upstream it. (Help wanted! Would you be interested in this
> and in upstreaming?)
>
> The patches are located here:
>
> https://github.com/NetBSD/pkgsrc-wip/tree/master/gdb-netbsd/patches
>
> * with core/basic features... but it is difficult as there is no OS with
> finished transition...
> https://sourceware.org/gdb/wiki/LocalRemoteFeatureParity

I can definitely not commit to upstreaming the gdbserver. I am only
looking at NetBSD because I wanted to remove a deprecated function in
GDB, and one of the two callers is in NetBSD ARM code. So, I wanted to
build ARM NetBSD first so I can test if it still works after that
change. But I can't commit to any further NetBSD work.

BTW, is there a reason why your patches have one .patch per changed
file? I usually find it easier to follow them if they are instead
grouped by some kind of topic per patch.

Thanks,
Christian

>
> >
> > Thanks,
> > Christian
> >
> > On Fri, Jan 24, 2020 at 3:29 PM Kamil Rytarowski <n54@gmx.com> wrote:
> >>
> >> On 24.01.2020 15:18, cbiesinger@chromium.org wrote:
> >>> From: Christian Biesinger <cbiesinger@google.com>
> >>>
> >>> Fixes the below compile error on ARM NetBSD 9.0_RC1 (the only version I
> >>> tested).  types.h does not define register_t by default.
> >>>
> >>> We already use this define elsewhere, notably in bsd-kvm.c.
> >>>
> >>> In file included from ../../gdb/arm-nbsd-nat.c:28:
> >>> /usr/include/machine/frame.h:54:2: error: unknown type name 'register_t'; did you mean '__register_t'?
> >>>         register_t tf_spsr;
> >>>         ^
> >>> /usr/include/machine/types.h:77:14: note: '__register_t' declared here
> >>> typedef int             __register_t;
> >>>                         ^
> >>>
> >>> There are other compile errors that this does not fix.
> >>>
> >>> gdb/ChangeLog:
> >>>
> >>> 2020-01-24  Christian Biesinger  <cbiesinger@google.com>
> >>>
> >>>       * arm-nbsd-nat.c: Define _KMEMUSER to get the declaration of
> >>>       register_t.
> >>>
> >>> Change-Id: I82c21d38189ee59ea0af2538ba84b771d268722e
> >>> ---
> >>>  gdb/arm-nbsd-nat.c | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
> >>> index 00f919194b..4844b51a3c 100644
> >>> --- a/gdb/arm-nbsd-nat.c
> >>> +++ b/gdb/arm-nbsd-nat.c
> >>> @@ -17,6 +17,8 @@
> >>>     You should have received a copy of the GNU General Public License
> >>>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> >>>
> >>> +/* We define this to get types like register_t.  */
> >>> +#define _KMEMUSER
> >>>  #include "defs.h"
> >>>  #include "gdbcore.h"
> >>>  #include "inferior.h"
> >>>
> >>
> >> While gdb is the right user for _KMEMUSER, here we should probably go
> >> for -D_KERNTYPES as it is the canonical symbol for register_t.
> >>
>
>

Re: [PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 4498 bytes --]

On 24.01.2020 16:35, Christian Biesinger via gdb-patches wrote:
> On Fri, Jan 24, 2020 at 4:23 PM Kamil Rytarowski <n54@gmx.com> wrote:
>>
>> On 24.01.2020 15:53, Christian Biesinger via gdb-patches wrote:
>>> Hi Kamil,
>>>
>>> I have a related question. NetBSD applied this patch:
>>> https://www.mail-archive.com/tech@openbsd.org/msg44100.html
>>>
>>
>> Is this the right link?
> 
> Yeah -- that patch changes a system header at the top and patches GDB
> at the bottom.
> 

This is not a change in NetBSD, so it is unrelated.

>>> Do you know which NetBSD version that shipped in? Can we apply that
>>> patch to GDB as-is or should we attempt to support the older struct
>>> layout as well?
>>
>> Please go for the current FPU layout on NetBSD. Massive ptrace(2) fixes
>> were introduced in NetBSD-8 and later. Soon NetBSD 7.x will go EOL
>> (after releasing 9.0, rc2 is planned soon).
> 
> OK, great. Thanks.
> 
>> In LLDB we support NetBSD 9.0 or newer. In GDB we should keep the same
>> minimal requirements and deal with older NetBSD versions (if at all)
>> with downstream patches.
>>
>> We have got a pile of local GDB patches.
> 
> OK. Maybe I should look through those at some point...  I was
> surprised that NetBSD apparently has an oldish GDB if
> http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/devel/gdb/README.html
> is correct (8.1)
> 
>> There is also a functional gdbserver implementation on NetBSD/amd64 and
>> I intend to upstream it. (Help wanted! Would you be interested in this
>> and in upstreaming?)
>>
>> The patches are located here:
>>
>> https://github.com/NetBSD/pkgsrc-wip/tree/master/gdb-netbsd/patches
>>
>> * with core/basic features... but it is difficult as there is no OS with
>> finished transition...
>> https://sourceware.org/gdb/wiki/LocalRemoteFeatureParity
> 
> I can definitely not commit to upstreaming the gdbserver. I am only
> looking at NetBSD because I wanted to remove a deprecated function in
> GDB, and one of the two callers is in NetBSD ARM code. So, I wanted to
> build ARM NetBSD first so I can test if it still works after that
> change. But I can't commit to any further NetBSD work.
> 

OK, thanks!

> BTW, is there a reason why your patches have one .patch per changed
> file? I usually find it easier to follow them if they are instead
> grouped by some kind of topic per patch.
> 

This is a convention in pkgsrc and it is practical for its use-case.

> Thanks,
> Christian
> 
>>
>>>
>>> Thanks,
>>> Christian
>>>
>>> On Fri, Jan 24, 2020 at 3:29 PM Kamil Rytarowski <n54@gmx.com> wrote:
>>>>
>>>> On 24.01.2020 15:18, cbiesinger@chromium.org wrote:
>>>>> From: Christian Biesinger <cbiesinger@google.com>
>>>>>
>>>>> Fixes the below compile error on ARM NetBSD 9.0_RC1 (the only version I
>>>>> tested).  types.h does not define register_t by default.
>>>>>
>>>>> We already use this define elsewhere, notably in bsd-kvm.c.
>>>>>
>>>>> In file included from ../../gdb/arm-nbsd-nat.c:28:
>>>>> /usr/include/machine/frame.h:54:2: error: unknown type name 'register_t'; did you mean '__register_t'?
>>>>>         register_t tf_spsr;
>>>>>         ^
>>>>> /usr/include/machine/types.h:77:14: note: '__register_t' declared here
>>>>> typedef int             __register_t;
>>>>>                         ^
>>>>>
>>>>> There are other compile errors that this does not fix.
>>>>>
>>>>> gdb/ChangeLog:
>>>>>
>>>>> 2020-01-24  Christian Biesinger  <cbiesinger@google.com>
>>>>>
>>>>>       * arm-nbsd-nat.c: Define _KMEMUSER to get the declaration of
>>>>>       register_t.
>>>>>
>>>>> Change-Id: I82c21d38189ee59ea0af2538ba84b771d268722e
>>>>> ---
>>>>>  gdb/arm-nbsd-nat.c | 2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
>>>>> index 00f919194b..4844b51a3c 100644
>>>>> --- a/gdb/arm-nbsd-nat.c
>>>>> +++ b/gdb/arm-nbsd-nat.c
>>>>> @@ -17,6 +17,8 @@
>>>>>     You should have received a copy of the GNU General Public License
>>>>>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>>>>
>>>>> +/* We define this to get types like register_t.  */
>>>>> +#define _KMEMUSER
>>>>>  #include "defs.h"
>>>>>  #include "gdbcore.h"
>>>>>  #include "inferior.h"
>>>>>
>>>>
>>>> While gdb is the right user for _KMEMUSER, here we should probably go
>>>> for -D_KERNTYPES as it is the canonical symbol for register_t.
>>>>
>>
>>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c

[Christian Biesinger via gdb-patches]

On Fri, Jan 24, 2020 at 4:49 PM Kamil Rytarowski <n54@gmx.com> wrote:
>
> On 24.01.2020 16:35, Christian Biesinger via gdb-patches wrote:
> > On Fri, Jan 24, 2020 at 4:23 PM Kamil Rytarowski <n54@gmx.com> wrote:
> >>
> >> On 24.01.2020 15:53, Christian Biesinger via gdb-patches wrote:
> >>> Hi Kamil,
> >>>
> >>> I have a related question. NetBSD applied this patch:
> >>> https://www.mail-archive.com/tech@openbsd.org/msg44100.html
> >>>
> >>
> >> Is this the right link?
> >
> > Yeah -- that patch changes a system header at the top and patches GDB
> > at the bottom.
> >
>
> This is not a change in NetBSD, so it is unrelated.

My apologies, I completely misread that. I'll see if I can find where
NetBSD changed their FP register data structure, or perhaps your
downstream patch will still work (although that probably has to come
from one of y'all for copyright reasons?)

> >>> Do you know which NetBSD version that shipped in? Can we apply that
> >>> patch to GDB as-is or should we attempt to support the older struct
> >>> layout as well?
> >>
> >> Please go for the current FPU layout on NetBSD. Massive ptrace(2) fixes
> >> were introduced in NetBSD-8 and later. Soon NetBSD 7.x will go EOL
> >> (after releasing 9.0, rc2 is planned soon).
> >
> > OK, great. Thanks.
> >
> >> In LLDB we support NetBSD 9.0 or newer. In GDB we should keep the same
> >> minimal requirements and deal with older NetBSD versions (if at all)
> >> with downstream patches.
> >>
> >> We have got a pile of local GDB patches.
> >
> > OK. Maybe I should look through those at some point...  I was
> > surprised that NetBSD apparently has an oldish GDB if
> > http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/devel/gdb/README.html
> > is correct (8.1)
> >
> >> There is also a functional gdbserver implementation on NetBSD/amd64 and
> >> I intend to upstream it. (Help wanted! Would you be interested in this
> >> and in upstreaming?)
> >>
> >> The patches are located here:
> >>
> >> https://github.com/NetBSD/pkgsrc-wip/tree/master/gdb-netbsd/patches
> >>
> >> * with core/basic features... but it is difficult as there is no OS with
> >> finished transition...
> >> https://sourceware.org/gdb/wiki/LocalRemoteFeatureParity
> >
> > I can definitely not commit to upstreaming the gdbserver. I am only
> > looking at NetBSD because I wanted to remove a deprecated function in
> > GDB, and one of the two callers is in NetBSD ARM code. So, I wanted to
> > build ARM NetBSD first so I can test if it still works after that
> > change. But I can't commit to any further NetBSD work.
> >
>
> OK, thanks!
>
> > BTW, is there a reason why your patches have one .patch per changed
> > file? I usually find it easier to follow them if they are instead
> > grouped by some kind of topic per patch.
> >
>
> This is a convention in pkgsrc and it is practical for its use-case.

OK. Some of those patches should be upstreamable very easily. For some
others it would be helpful to have a description & I guess a copyright
assignment.

I also noticed that some of those files are 0 bytes and could probably
be deleted? And this does not seem necessary:
https://github.com/NetBSD/pkgsrc-wip/blob/master/gdb-netbsd/patches/patch-bfd_netbsd-core.c

> >>>
> >>> Thanks,
> >>> Christian
> >>>
> >>> On Fri, Jan 24, 2020 at 3:29 PM Kamil Rytarowski <n54@gmx.com> wrote:
> >>>>
> >>>> On 24.01.2020 15:18, cbiesinger@chromium.org wrote:
> >>>>> From: Christian Biesinger <cbiesinger@google.com>
> >>>>>
> >>>>> Fixes the below compile error on ARM NetBSD 9.0_RC1 (the only version I
> >>>>> tested).  types.h does not define register_t by default.
> >>>>>
> >>>>> We already use this define elsewhere, notably in bsd-kvm.c.
> >>>>>
> >>>>> In file included from ../../gdb/arm-nbsd-nat.c:28:
> >>>>> /usr/include/machine/frame.h:54:2: error: unknown type name 'register_t'; did you mean '__register_t'?
> >>>>>         register_t tf_spsr;
> >>>>>         ^
> >>>>> /usr/include/machine/types.h:77:14: note: '__register_t' declared here
> >>>>> typedef int             __register_t;
> >>>>>                         ^
> >>>>>
> >>>>> There are other compile errors that this does not fix.
> >>>>>
> >>>>> gdb/ChangeLog:
> >>>>>
> >>>>> 2020-01-24  Christian Biesinger  <cbiesinger@google.com>
> >>>>>
> >>>>>       * arm-nbsd-nat.c: Define _KMEMUSER to get the declaration of
> >>>>>       register_t.
> >>>>>
> >>>>> Change-Id: I82c21d38189ee59ea0af2538ba84b771d268722e
> >>>>> ---
> >>>>>  gdb/arm-nbsd-nat.c | 2 ++
> >>>>>  1 file changed, 2 insertions(+)
> >>>>>
> >>>>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
> >>>>> index 00f919194b..4844b51a3c 100644
> >>>>> --- a/gdb/arm-nbsd-nat.c
> >>>>> +++ b/gdb/arm-nbsd-nat.c
> >>>>> @@ -17,6 +17,8 @@
> >>>>>     You should have received a copy of the GNU General Public License
> >>>>>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> >>>>>
> >>>>> +/* We define this to get types like register_t.  */
> >>>>> +#define _KMEMUSER
> >>>>>  #include "defs.h"
> >>>>>  #include "gdbcore.h"
> >>>>>  #include "inferior.h"
> >>>>>
> >>>>
> >>>> While gdb is the right user for _KMEMUSER, here we should probably go
> >>>> for -D_KERNTYPES as it is the canonical symbol for register_t.
> >>>>
> >>
> >>
>
>

Re: [PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 5874 bytes --]

On 24.01.2020 17:01, Christian Biesinger wrote:
> On Fri, Jan 24, 2020 at 4:49 PM Kamil Rytarowski <n54@gmx.com> wrote:
>>
>> On 24.01.2020 16:35, Christian Biesinger via gdb-patches wrote:
>>> On Fri, Jan 24, 2020 at 4:23 PM Kamil Rytarowski <n54@gmx.com> wrote:
>>>>
>>>> On 24.01.2020 15:53, Christian Biesinger via gdb-patches wrote:
>>>>> Hi Kamil,
>>>>>
>>>>> I have a related question. NetBSD applied this patch:
>>>>> https://www.mail-archive.com/tech@openbsd.org/msg44100.html
>>>>>
>>>>
>>>> Is this the right link?
>>>
>>> Yeah -- that patch changes a system header at the top and patches GDB
>>> at the bottom.
>>>
>>
>> This is not a change in NetBSD, so it is unrelated.
> 
> My apologies, I completely misread that. I'll see if I can find where
> NetBSD changed their FP register data structure, or perhaps your
> downstream patch will still work (although that probably has to come
> from one of y'all for copyright reasons?)
> 

Please cherry-pick what you need and I will find the original author.
Many people in NetBSD have FSF papers done.

>>>>> Do you know which NetBSD version that shipped in? Can we apply that
>>>>> patch to GDB as-is or should we attempt to support the older struct
>>>>> layout as well?
>>>>
>>>> Please go for the current FPU layout on NetBSD. Massive ptrace(2) fixes
>>>> were introduced in NetBSD-8 and later. Soon NetBSD 7.x will go EOL
>>>> (after releasing 9.0, rc2 is planned soon).
>>>
>>> OK, great. Thanks.
>>>
>>>> In LLDB we support NetBSD 9.0 or newer. In GDB we should keep the same
>>>> minimal requirements and deal with older NetBSD versions (if at all)
>>>> with downstream patches.
>>>>
>>>> We have got a pile of local GDB patches.
>>>
>>> OK. Maybe I should look through those at some point...  I was
>>> surprised that NetBSD apparently has an oldish GDB if
>>> http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/devel/gdb/README.html
>>> is correct (8.1)
>>>
>>>> There is also a functional gdbserver implementation on NetBSD/amd64 and
>>>> I intend to upstream it. (Help wanted! Would you be interested in this
>>>> and in upstreaming?)
>>>>
>>>> The patches are located here:
>>>>
>>>> https://github.com/NetBSD/pkgsrc-wip/tree/master/gdb-netbsd/patches
>>>>
>>>> * with core/basic features... but it is difficult as there is no OS with
>>>> finished transition...
>>>> https://sourceware.org/gdb/wiki/LocalRemoteFeatureParity
>>>
>>> I can definitely not commit to upstreaming the gdbserver. I am only
>>> looking at NetBSD because I wanted to remove a deprecated function in
>>> GDB, and one of the two callers is in NetBSD ARM code. So, I wanted to
>>> build ARM NetBSD first so I can test if it still works after that
>>> change. But I can't commit to any further NetBSD work.
>>>
>>
>> OK, thanks!
>>
>>> BTW, is there a reason why your patches have one .patch per changed
>>> file? I usually find it easier to follow them if they are instead
>>> grouped by some kind of topic per patch.
>>>
>>
>> This is a convention in pkgsrc and it is practical for its use-case.
> 
> OK. Some of those patches should be upstreamable very easily. For some
> others it would be helpful to have a description & I guess a copyright
> assignment.
> 

My intention was to revamp the general NetBSD support from native-only
to gdbserver... (same like lldb-server).. and it would result in deep
rewrite of the current code, but there is work in progress status in
Linux so it does not help.

> I also noticed that some of those files are 0 bytes and could probably
> be deleted? And this does not seem necessary:
> https://github.com/NetBSD/pkgsrc-wip/blob/master/gdb-netbsd/patches/patch-bfd_netbsd-core.c
> 

Good catch.

>>>>>
>>>>> Thanks,
>>>>> Christian
>>>>>
>>>>> On Fri, Jan 24, 2020 at 3:29 PM Kamil Rytarowski <n54@gmx.com> wrote:
>>>>>>
>>>>>> On 24.01.2020 15:18, cbiesinger@chromium.org wrote:
>>>>>>> From: Christian Biesinger <cbiesinger@google.com>
>>>>>>>
>>>>>>> Fixes the below compile error on ARM NetBSD 9.0_RC1 (the only version I
>>>>>>> tested).  types.h does not define register_t by default.
>>>>>>>
>>>>>>> We already use this define elsewhere, notably in bsd-kvm.c.
>>>>>>>
>>>>>>> In file included from ../../gdb/arm-nbsd-nat.c:28:
>>>>>>> /usr/include/machine/frame.h:54:2: error: unknown type name 'register_t'; did you mean '__register_t'?
>>>>>>>         register_t tf_spsr;
>>>>>>>         ^
>>>>>>> /usr/include/machine/types.h:77:14: note: '__register_t' declared here
>>>>>>> typedef int             __register_t;
>>>>>>>                         ^
>>>>>>>
>>>>>>> There are other compile errors that this does not fix.
>>>>>>>
>>>>>>> gdb/ChangeLog:
>>>>>>>
>>>>>>> 2020-01-24  Christian Biesinger  <cbiesinger@google.com>
>>>>>>>
>>>>>>>       * arm-nbsd-nat.c: Define _KMEMUSER to get the declaration of
>>>>>>>       register_t.
>>>>>>>
>>>>>>> Change-Id: I82c21d38189ee59ea0af2538ba84b771d268722e
>>>>>>> ---
>>>>>>>  gdb/arm-nbsd-nat.c | 2 ++
>>>>>>>  1 file changed, 2 insertions(+)
>>>>>>>
>>>>>>> diff --git a/gdb/arm-nbsd-nat.c b/gdb/arm-nbsd-nat.c
>>>>>>> index 00f919194b..4844b51a3c 100644
>>>>>>> --- a/gdb/arm-nbsd-nat.c
>>>>>>> +++ b/gdb/arm-nbsd-nat.c
>>>>>>> @@ -17,6 +17,8 @@
>>>>>>>     You should have received a copy of the GNU General Public License
>>>>>>>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>>>>>>>
>>>>>>> +/* We define this to get types like register_t.  */
>>>>>>> +#define _KMEMUSER
>>>>>>>  #include "defs.h"
>>>>>>>  #include "gdbcore.h"
>>>>>>>  #include "inferior.h"
>>>>>>>
>>>>>>
>>>>>> While gdb is the right user for _KMEMUSER, here we should probably go
>>>>>> for -D_KERNTYPES as it is the canonical symbol for register_t.
>>>>>>
>>>>
>>>>
>>
>>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c

[Christian Biesinger via gdb-patches]

On Fri, Jan 24, 2020 at 5:37 PM Kamil Rytarowski <n54@gmx.com> wrote:
>
> On 24.01.2020 17:01, Christian Biesinger wrote:
> > On Fri, Jan 24, 2020 at 4:49 PM Kamil Rytarowski <n54@gmx.com> wrote:
> >>
> >> On 24.01.2020 16:35, Christian Biesinger via gdb-patches wrote:
> >>> On Fri, Jan 24, 2020 at 4:23 PM Kamil Rytarowski <n54@gmx.com> wrote:
> >>>>
> >>>> On 24.01.2020 15:53, Christian Biesinger via gdb-patches wrote:
> >>>>> Hi Kamil,
> >>>>>
> >>>>> I have a related question. NetBSD applied this patch:
> >>>>> https://www.mail-archive.com/tech@openbsd.org/msg44100.html
> >>>>>
> >>>>
> >>>> Is this the right link?
> >>>
> >>> Yeah -- that patch changes a system header at the top and patches GDB
> >>> at the bottom.
> >>>
> >>
> >> This is not a change in NetBSD, so it is unrelated.
> >
> > My apologies, I completely misread that. I'll see if I can find where
> > NetBSD changed their FP register data structure, or perhaps your
> > downstream patch will still work (although that probably has to come
> > from one of y'all for copyright reasons?)
> >
>
> Please cherry-pick what you need and I will find the original author.
> Many people in NetBSD have FSF papers done.

Thanks. I think this patch would probably be the most important since
it fixes a compile error:
https://github.com/NetBSD/pkgsrc-wip/blob/master/gdb-netbsd/patches/patch-gdb_arm-nbsd-nat.c

However, I don't understand why arm_netbsd_core_fns /
fetch_core_registers was added. As far as I can tell, because that
struct uses bfd_target_unknown_flavour and default_core_sniffer, it
will never be used.

Christian

Re: [PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c

[Christian Biesinger via gdb-patches]

On Mon, Jan 27, 2020 at 7:38 AM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> On Fri, Jan 24, 2020 at 5:37 PM Kamil Rytarowski <n54@gmx.com> wrote:
> >
> > On 24.01.2020 17:01, Christian Biesinger wrote:
> > > On Fri, Jan 24, 2020 at 4:49 PM Kamil Rytarowski <n54@gmx.com> wrote:
> > >>
> > >> On 24.01.2020 16:35, Christian Biesinger via gdb-patches wrote:
> > >>> On Fri, Jan 24, 2020 at 4:23 PM Kamil Rytarowski <n54@gmx.com> wrote:
> > >>>>
> > >>>> On 24.01.2020 15:53, Christian Biesinger via gdb-patches wrote:
> > >>>>> Hi Kamil,
> > >>>>>
> > >>>>> I have a related question. NetBSD applied this patch:
> > >>>>> https://www.mail-archive.com/tech@openbsd.org/msg44100.html
> > >>>>>
> > >>>>
> > >>>> Is this the right link?
> > >>>
> > >>> Yeah -- that patch changes a system header at the top and patches GDB
> > >>> at the bottom.
> > >>>
> > >>
> > >> This is not a change in NetBSD, so it is unrelated.
> > >
> > > My apologies, I completely misread that. I'll see if I can find where
> > > NetBSD changed their FP register data structure, or perhaps your
> > > downstream patch will still work (although that probably has to come
> > > from one of y'all for copyright reasons?)
> > >
> >
> > Please cherry-pick what you need and I will find the original author.
> > Many people in NetBSD have FSF papers done.
>
> Thanks. I think this patch would probably be the most important since
> it fixes a compile error:
> https://github.com/NetBSD/pkgsrc-wip/blob/master/gdb-netbsd/patches/patch-gdb_arm-nbsd-nat.c
>
> However, I don't understand why arm_netbsd_core_fns /
> fetch_core_registers was added. As far as I can tell, because that
> struct uses bfd_target_unknown_flavour and default_core_sniffer, it
> will never be used.

Hi Kamil,

I also filed https://github.com/NetBSD/pkgsrc-wip/issues/17 on some
patches that can be dropped, and I upstreamed a couple of
trivial/obvious patches you had.

Christian

Re: [PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 2096 bytes --]

On 05.02.2020 19:02, Christian Biesinger via gdb-patches wrote:
> On Mon, Jan 27, 2020 at 7:38 AM Christian Biesinger
> <cbiesinger@google.com> wrote:
>>
>> On Fri, Jan 24, 2020 at 5:37 PM Kamil Rytarowski <n54@gmx.com> wrote:
>>>
>>> On 24.01.2020 17:01, Christian Biesinger wrote:
>>>> On Fri, Jan 24, 2020 at 4:49 PM Kamil Rytarowski <n54@gmx.com> wrote:
>>>>>
>>>>> On 24.01.2020 16:35, Christian Biesinger via gdb-patches wrote:
>>>>>> On Fri, Jan 24, 2020 at 4:23 PM Kamil Rytarowski <n54@gmx.com> wrote:
>>>>>>>
>>>>>>> On 24.01.2020 15:53, Christian Biesinger via gdb-patches wrote:
>>>>>>>> Hi Kamil,
>>>>>>>>
>>>>>>>> I have a related question. NetBSD applied this patch:
>>>>>>>> https://www.mail-archive.com/tech@openbsd.org/msg44100.html
>>>>>>>>
>>>>>>>
>>>>>>> Is this the right link?
>>>>>>
>>>>>> Yeah -- that patch changes a system header at the top and patches GDB
>>>>>> at the bottom.
>>>>>>
>>>>>
>>>>> This is not a change in NetBSD, so it is unrelated.
>>>>
>>>> My apologies, I completely misread that. I'll see if I can find where
>>>> NetBSD changed their FP register data structure, or perhaps your
>>>> downstream patch will still work (although that probably has to come
>>>> from one of y'all for copyright reasons?)
>>>>
>>>
>>> Please cherry-pick what you need and I will find the original author.
>>> Many people in NetBSD have FSF papers done.
>>
>> Thanks. I think this patch would probably be the most important since
>> it fixes a compile error:
>> https://github.com/NetBSD/pkgsrc-wip/blob/master/gdb-netbsd/patches/patch-gdb_arm-nbsd-nat.c
>>
>> However, I don't understand why arm_netbsd_core_fns /
>> fetch_core_registers was added. As far as I can tell, because that
>> struct uses bfd_target_unknown_flavour and default_core_sniffer, it
>> will never be used.
> 
> Hi Kamil,
> 
> I also filed https://github.com/NetBSD/pkgsrc-wip/issues/17 on some
> patches that can be dropped, and I upstreamed a couple of
> trivial/obvious patches you had.
> 
> Christian
> 

Thank you for your contribution!


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 2390 bytes --]

On 27.01.2020 13:38, Christian Biesinger via gdb-patches wrote:
> On Fri, Jan 24, 2020 at 5:37 PM Kamil Rytarowski <n54@gmx.com> wrote:
>>
>> On 24.01.2020 17:01, Christian Biesinger wrote:
>>> On Fri, Jan 24, 2020 at 4:49 PM Kamil Rytarowski <n54@gmx.com> wrote:
>>>>
>>>> On 24.01.2020 16:35, Christian Biesinger via gdb-patches wrote:
>>>>> On Fri, Jan 24, 2020 at 4:23 PM Kamil Rytarowski <n54@gmx.com> wrote:
>>>>>>
>>>>>> On 24.01.2020 15:53, Christian Biesinger via gdb-patches wrote:
>>>>>>> Hi Kamil,
>>>>>>>
>>>>>>> I have a related question. NetBSD applied this patch:
>>>>>>> https://www.mail-archive.com/tech@openbsd.org/msg44100.html
>>>>>>>
>>>>>>
>>>>>> Is this the right link?
>>>>>
>>>>> Yeah -- that patch changes a system header at the top and patches GDB
>>>>> at the bottom.
>>>>>
>>>>
>>>> This is not a change in NetBSD, so it is unrelated.
>>>
>>> My apologies, I completely misread that. I'll see if I can find where
>>> NetBSD changed their FP register data structure, or perhaps your
>>> downstream patch will still work (although that probably has to come
>>> from one of y'all for copyright reasons?)
>>>
>>
>> Please cherry-pick what you need and I will find the original author.
>> Many people in NetBSD have FSF papers done.
> 
> Thanks. I think this patch would probably be the most important since
> it fixes a compile error:
> https://github.com/NetBSD/pkgsrc-wip/blob/master/gdb-netbsd/patches/patch-gdb_arm-nbsd-nat.c
> 
> However, I don't understand why arm_netbsd_core_fns /
> fetch_core_registers was added. As far as I can tell, because that
> struct uses bfd_target_unknown_flavour and default_core_sniffer, it
> will never be used.
> 
> Christian
> 

This looks like a leftover after a.out. It was removed upstream and we
by a mistake keep it locally as a local modification.

commit 1736a7bd96e8927c3f889a35f9153df4fd19d833
Author: Pedro Alves <palves@redhat.com>
Date:   Fri Dec 9 16:08:49 2016 +0000

    gdb: Remove support for obsolete OSABIs and a.out

    gdb/ChangeLog:
    2016-12-09  Pedro Alves  <palves@redhat.com>

a.out should be removed.

We switched to ELF back in NetBSD 1.6/2.0 and don't have toolchain for it.

We still support running old a.out binaries and plan to do it in
foreseeable future, but attaching a debugger to these processes is not
our priority.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]