This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
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. Andreas
Attachment:
gdb_ppc_20121127-1.diff
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |