From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 104558 invoked by alias); 24 Jan 2020 16:02:19 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 104546 invoked by uid 89); 24 Jan 2020 16:02:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-29.4 required=5.0 tests=AWL,BAYES_00,ENV_AND_HDR_SPF_MATCH,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS,USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy= X-HELO: mail-oi1-f194.google.com Received: from mail-oi1-f194.google.com (HELO mail-oi1-f194.google.com) (209.85.167.194) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 24 Jan 2020 16:02:16 +0000 Received: by mail-oi1-f194.google.com with SMTP id 18so2268509oin.9 for ; Fri, 24 Jan 2020 08:02:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=M01ByBMwBe4vQyeNZj665qAdyJB7BdsNJmv8eqtYZzA=; b=Zquq4PWzEuMBj/cqIz3ksprLcNvVtyx9mH8kkT+A80q78oKRCqwlcEechhkhmIzrmv 87L5+ckSpyrVuY1WvQDPIx2s04uhuY9yoVJmZr+HcY8doKIhRccTozTnthTGpdnCmef4 TtMrNDgLSun5aKNQxV+OttF3fqZ9/mtOYZtrJBPOA0p590SRpoK41ePd503eQcgvTu+B V3WBo3NSZ2JspM6JvLqpQmMwZuRku7ZDn9Fey/3c7FDT3KhaiYQX8dEb5VFnFnCtSd0v kSRxrs2Y0sTQPJZkG1C5Br0Pk0lccNPll97dmJzd32EssH+pAnOyZ7spjnhGyiaj4ZQN lHSg== MIME-Version: 1.0 References: <20200124141458.171392-3-cbiesinger@chromium.org> <20200124141818.172490-1-cbiesinger@chromium.org> <2afe5687-5be2-7650-d4e3-3aceed3f68f2@gmx.com> <7432896e-39ec-4a99-cc07-77c684b71644@gmx.com> <8126c811-3416-a4d4-5a01-17776b0df999@gmx.com> In-Reply-To: <8126c811-3416-a4d4-5a01-17776b0df999@gmx.com> From: "Christian Biesinger via gdb-patches" Reply-To: Christian Biesinger Date: Fri, 24 Jan 2020 16:25:00 -0000 Message-ID: Subject: Re: [PATCH 2/3 v2] Define _KMEMUSER in arm-nbsd-nat.c To: Kamil Rytarowski Cc: gdb-patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2020-01/txt/msg00811.txt.bz2 On Fri, Jan 24, 2020 at 4:49 PM Kamil Rytarowski wrote: > > On 24.01.2020 16:35, Christian Biesinger via gdb-patches wrote: > > On Fri, Jan 24, 2020 at 4:23 PM Kamil Rytarowski 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 wrote: > >>>> > >>>> On 24.01.2020 15:18, cbiesinger@chromium.org wrote: > >>>>> From: Christian Biesinger > >>>>> > >>>>> 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 > >>>>> > >>>>> * 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 . */ > >>>>> > >>>>> +/* 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. > >>>> > >> > >> > >