From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1032 invoked by alias); 11 Dec 2012 15:48:29 -0000 Received: (qmail 1022 invoked by uid 22791); 11 Dec 2012 15:48:27 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RP_MATCHES_RCVD,TBC X-Spam-Check-By: sourceware.org Received: from mail.fgznet.ch (HELO smtp.fgznet.ch) (81.92.96.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 11 Dec 2012 15:48:19 +0000 Received: from deuterium.andreas.nets (dhclient-91-190-14-19.flashcable.ch [91.190.14.19]) by smtp.fgznet.ch (8.13.8/8.13.8/Submit_SMTPAUTH) with ESMTP id qBBFm9Np021099; Tue, 11 Dec 2012 16:48:12 +0100 (CET) (envelope-from andreast-list@fgznet.ch) Message-ID: <50C755B8.5000209@fgznet.ch> Date: Tue, 11 Dec 2012 15:48:00 -0000 From: Andreas Tobler User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:16.0) Gecko/20121026 Thunderbird/16.0.2 MIME-Version: 1.0 To: Pedro Alves , gdb-patches@sourceware.org Subject: PING Re: [PATCH] FreeBSD powerpc support References: <50AAA80B.1000509@fgznet.ch> <50AFCF95.1080809@redhat.com> <50B4FC29.9050006@fgznet.ch> In-Reply-To: <50B4FC29.9050006@fgznet.ch> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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 X-SW-Source: 2012-12/txt/msg00340.txt.bz2 On 27.11.12 18:45, Andreas Tobler wrote: > Hello again, > > took a bit longer. > > On 23.11.12 20:33, Pedro Alves wrote: > >>> The attached patch adds support for FreeBSD PowerPC, 32-bit and 64-bit. >>> It is derived from ppcobsd* and ppc-linux with FreeBSD additions. >>> >>> There is room for improvement :) And I will continue, but I'd like to >>> see this patch in the gdb cvs because it will be much easier for me fix >>> outstanding issues when I can work with cvs iso. of local revisions. >> >> Eh. If it's easier, then maybe you're not using the proper tools; there's >> always quilt. :-) Or better nowadays, you could also put it >> in a public git repo somewhere. We have a git mirror of cvs. >> That said, I'm really not against putting it in early, if it's not riddled >> with hacks. > > Might be that I do not use the latest and greatest tools. > The room for improvement, above, is in the direction of general FreeBSD > stuff, not only limited to this particular port. > >>> Also, other people might have a use of this work, even if not complete. >>> >>> Currently missing/incomplete: >>> - Altivec support, kernel bits are missing. >>> - HW watchpoints, also kernel bits are missing. >>> - thread support. >>> - tls support. >>> - some sig tests. >> >> I've skimmed the patch, and didn't notice anything horrible. >> Then again, I'm on the low end of the scale that measures >> PowerPC or FreeBSD expertness... >> >> - Please make sure there's a blank line between introductory comment >> and function. > > I hope I didn't miss them. > >> - I noticed that a few functions don't have introductory comment. > > Also, I put one in where I could. > >> - If the function implements of a target/gdbarch/etc. method, then >> comment it as such. E.g., >> >> /* This is the implementation of gdbarch method FOOBAR. */ >> >> - I noticed some functions with long comments are copies of existing >> code of other ports. I wonder if we could perhaps share more code. > > True, here I do not know how to share, maybe a common ppc64-tdep-common.c? > >>> +/* Read a PPC instruction from memory. PPC instructions are always >>> + * big-endian, no matter what endianness the program is running in, so >>> + * we can't use read_memory_integer or one of its friends here. >> >> read_memory_unsigned_integer nowadays has a byte_order parameter, >> so just pass it BFD_ENDIAN_BIG, and you're set. > > Done. Even tested on ppc64-linux. I might send a patch for > ppc-linux-tdep.c as well. Likewise for the below. > >>> +#define PPC64_STANDARD_LINKAGE2_LEN \ >>> + (sizeof (ppc64_standard_linkage2) / sizeof (ppc64_standard_linkage2[0])) >>> + >> >> Use the existing ARRAY_SIZE macro. > > Done. > >> +/* Signal trampolines. */ >>> + >>> +static int >>> +ppcfbsd_sigtramp_frame_sniffer (const struct frame_unwind *self, >>> + struct frame_info *this_frame, >>> + void **this_cache) >>> +{ >>> + struct gdbarch *gdbarch = get_frame_arch (this_frame); >>> + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); >>> + CORE_ADDR pc = get_frame_pc (this_frame); >>> + CORE_ADDR start_pc = (pc & ~(ppcfbsd_page_size - 1)); >>> + const int *offset; >>> + const char *name; >>> + >>> + find_pc_partial_function (pc, &name, NULL, NULL); >>> + if (name) >>> + return 0; >> >> For some reason this bailing out if name is not null jumped at me. >> It's not obvious to me what that means, so it felt like it deserves >> a comment. > > Also done, I hope I match the expectations. > >> On 11/19/2012 09:43 PM, Andreas Tobler wrote: >>> --- configure.host 30 May 2012 19:41:34 -0000 1.107 >>> +++ configure.host 19 Nov 2012 21:24:15 -0000 >>> @@ -125,6 +125,7 @@ >>> >>> powerpc-*-aix* | rs6000-*-*) >>> gdb_host=aix ;; >>> +powerpc*-*-freebsd*) gdb_host=fbsd ;; >> >> This seems to be 'powerpc-*-freebsd*' elsewhere I looked (top level, bfd). >> Why the extra wildcard? > > > Hm, here I'm not sure. My targets report as powerpc-unknown-freebsd* > (32-bit) and powerpc64-unknown-freebsd* (64-bit). So I thought I have to > match both. Is this not correct? > > Attached a revised version of the diff. > > Pedro, again thank you very much for the feedback. ping, anything I miss? Or is it the usual pre x-mas business ;) TIA, Andreas