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

["Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org>]

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.
> >>
>
>