public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] Use the ELF class to determine the word size for FreeBSD core notes.
  2016-12-06 21:01 [PATCH v2 0/3] Add FreeBSD/mips targets to GDB John Baldwin
  2016-12-06 21:01 ` [PATCH v2 3/3] Add native target for FreeBSD/mips John Baldwin
@ 2016-12-06 21:01 ` John Baldwin
  2016-12-13 11:04   ` Nick Clifton
  2016-12-06 21:01 ` [PATCH v2 2/3] Add FreeBSD/mips architecture John Baldwin
  2016-12-16 12:50 ` [PATCH v2 0/3] Add FreeBSD/mips targets to GDB Pedro Alves
  3 siblings, 1 reply; 16+ messages in thread
From: John Baldwin @ 2016-12-06 21:01 UTC (permalink / raw)
  To: gdb-patches, binutils

FreeBSD MIPS64 core dumps contain an empty e_flags value so are assigned
to the default bfd_arch_mips which is 32-bits.

bfd/ChangeLog:

	* elf.c (elfcore_grok_freebsd_psinfo): Use ELF header class to
	determine structure sizes.
	(elfcore_grok_freebsd_prstatus): Likewise.
---
 bfd/ChangeLog |  6 ++++++
 bfd/elf.c     | 23 +++++++++++++----------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 26fb42b..dd41d1a 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@
+2016-12-06  John Baldwin  <jhb@FreeBSD.org>
+
+	* elf.c (elfcore_grok_freebsd_psinfo): Use ELF header class to
+	determine structure sizes.
+	(elfcore_grok_freebsd_prstatus): Likewise.
+
 2016-12-06  Nick Clifton  <nickc@redhat.com>
 
 	PR binutils/20931
diff --git a/bfd/elf.c b/bfd/elf.c
index 678c043..c7c7ff1 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -9714,14 +9714,14 @@ elfcore_grok_freebsd_psinfo (bfd *abfd, Elf_Internal_Note *note)
 {
   size_t offset;
 
-  switch (abfd->arch_info->bits_per_word)
+  switch (elf_elfheader (abfd)->e_ident[EI_CLASS])
     {
-    case 32:
+    case ELFCLASS32:
       if (note->descsz < 108)
 	return FALSE;
       break;
 
-    case 64:
+    case ELFCLASS64:
       if (note->descsz < 120)
 	return FALSE;
       break;
@@ -9736,7 +9736,7 @@ elfcore_grok_freebsd_psinfo (bfd *abfd, Elf_Internal_Note *note)
   offset = 4;
 
   /* Skip over pr_psinfosz. */
-  if (abfd->arch_info->bits_per_word == 32)
+  if (elf_elfheader (abfd)->e_ident[EI_CLASS] == ELFCLASS32)
     offset += 4;
   else
     {
@@ -9779,13 +9779,13 @@ elfcore_grok_freebsd_prstatus (bfd *abfd, Elf_Internal_Note *note)
   offset = 4;
 
   /* Skip over pr_statussz.  */
-  switch (abfd->arch_info->bits_per_word)
+  switch (elf_elfheader (abfd)->e_ident[EI_CLASS])
     {
-    case 32:
+    case ELFCLASS32:
       offset += 4;
       break;
 
-    case 64:
+    case ELFCLASS64:
       offset += 4;	/* Padding before pr_statussz. */
       offset += 8;
       break;
@@ -9795,13 +9795,16 @@ elfcore_grok_freebsd_prstatus (bfd *abfd, Elf_Internal_Note *note)
     }
 
   /* Extract size of pr_reg from pr_gregsetsz.  */
-  if (abfd->arch_info->bits_per_word == 32)
+  if (elf_elfheader (abfd)->e_ident[EI_CLASS] == ELFCLASS32)
     size = bfd_h_get_32 (abfd, (bfd_byte *) note->descdata + offset);
   else
     size = bfd_h_get_64 (abfd, (bfd_byte *) note->descdata + offset);
 
   /* Skip over pr_gregsetsz and pr_fpregsetsz. */
-  offset += (abfd->arch_info->bits_per_word / 8) * 2;
+  if (elf_elfheader (abfd)->e_ident[EI_CLASS] == ELFCLASS32)
+    offset += 4 * 2;
+  else
+    offset += 8 * 2;
 
   /* Skip over pr_osreldate. */
   offset += 4;
@@ -9818,7 +9821,7 @@ elfcore_grok_freebsd_prstatus (bfd *abfd, Elf_Internal_Note *note)
   offset += 4;
 
   /* Padding before pr_reg. */
-  if (abfd->arch_info->bits_per_word == 64)
+  if (elf_elfheader (abfd)->e_ident[EI_CLASS] == ELFCLASS64)
     offset += 4;
 
   /* Make a ".reg/999" section and a ".reg" section.  */
-- 
2.9.2

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 0/3] Add FreeBSD/mips targets to GDB
@ 2016-12-06 21:01 John Baldwin
  2016-12-06 21:01 ` [PATCH v2 3/3] Add native target for FreeBSD/mips John Baldwin
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: John Baldwin @ 2016-12-06 21:01 UTC (permalink / raw)
  To: gdb-patches, binutils

The first patch works around an issue where FreeBSD/mips core
dumps from the kernel include an empty ei_flags value in the ELF
header.  This causes bfd to select the default MIPS architecture
(bfd_mach_3000) even though it is a 64-bit core file.  As a result,
parsing the core dump notes doesn't work correctly since bits_per_word
is 32.  It's not clear to me if bfd shouldn't default to a 64-bit
MIPS machine if the ELF header indicates a 64-bit class instead of
this change?

Changes since the first version are to address most (but perhaps not
all) of the feedback from Luis Machado on patches 2 and 3.

John Baldwin (3):
  Use the ELF class to determine the word size for FreeBSD core notes.
  Add FreeBSD/mips architecture.
  Add native target for FreeBSD/mips.

 bfd/ChangeLog           |   6 +
 bfd/elf.c               |  23 +-
 gdb/ChangeLog           |  15 ++
 gdb/Makefile.in         |   3 +
 gdb/config/mips/fbsd.mh |   3 +
 gdb/configure.host      |   1 +
 gdb/configure.tgt       |   5 +
 gdb/mips-fbsd-nat.c     | 141 ++++++++++++
 gdb/mips-fbsd-tdep.c    | 565 ++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/mips-fbsd-tdep.h    |  28 +++
 10 files changed, 780 insertions(+), 10 deletions(-)
 create mode 100644 gdb/config/mips/fbsd.mh
 create mode 100644 gdb/mips-fbsd-nat.c
 create mode 100644 gdb/mips-fbsd-tdep.c
 create mode 100644 gdb/mips-fbsd-tdep.h

-- 
2.9.2

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 2/3] Add FreeBSD/mips architecture.
  2016-12-06 21:01 [PATCH v2 0/3] Add FreeBSD/mips targets to GDB John Baldwin
  2016-12-06 21:01 ` [PATCH v2 3/3] Add native target for FreeBSD/mips John Baldwin
  2016-12-06 21:01 ` [PATCH v2 1/3] Use the ELF class to determine the word size for FreeBSD core notes John Baldwin
@ 2016-12-06 21:01 ` John Baldwin
  2016-12-08 18:47   ` Luis Machado
  2016-12-16 12:22   ` GDB attribution policy (Re: [PATCH v2 2/3] Add FreeBSD/mips architecture.) Pedro Alves
  2016-12-16 12:50 ` [PATCH v2 0/3] Add FreeBSD/mips targets to GDB Pedro Alves
  3 siblings, 2 replies; 16+ messages in thread
From: John Baldwin @ 2016-12-06 21:01 UTC (permalink / raw)
  To: gdb-patches, binutils

This has been tested for the n64 and o32 ABIs.  Signal frame unwinders for
both ABIs are provided.  FreeBSD/mips requires custom linkmap offsets since
it contains an additional l_off member in 'struct link_map' that other
FreeBSD platforms do not have.  Support for collecting and supplying
general purpose and floating point register sets are provided.  Common
routines for working with native format register sets are exported for
use by the native target.

gdb/ChangeLog:

	* Makefile.in (ALL_TARGET_OBS): Add mips-fbsd-tdep.o.
	(ALLDEPFILES): Add mips-fbsd-tdep.c.
	* configure.tgt: Add mips*-*-freebsd*.
	* mips-fbsd-tdep.c: New file.
	* mips-fbsd-tdep.h: New file.
---
 gdb/ChangeLog        |   8 +
 gdb/Makefile.in      |   2 +
 gdb/configure.tgt    |   5 +
 gdb/mips-fbsd-tdep.c | 565 +++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/mips-fbsd-tdep.h |  28 +++
 5 files changed, 608 insertions(+)
 create mode 100644 gdb/mips-fbsd-tdep.c
 create mode 100644 gdb/mips-fbsd-tdep.h

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ea1cff3..890702e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2016-12-06  John Baldwin  <jhb@FreeBSD.org>
+
+	* Makefile.in (ALL_TARGET_OBS): Add mips-fbsd-tdep.o.
+	(ALLDEPFILES): Add mips-fbsd-tdep.c.
+	* configure.tgt: Add mips*-*-freebsd*.
+	* mips-fbsd-tdep.c: New file.
+	* mips-fbsd-tdep.h: New file.
+
 2016-12-06  Yao Qi  <yao.qi@linaro.org>
 
 	* frame.c (frame_register_unwind): Set *realnump if *lvalp is
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 2cf1b3d..e34fa4a 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -817,6 +817,7 @@ ALL_TARGET_OBS = \
 	mep-tdep.o \
 	microblaze-linux-tdep.o \
 	microblaze-tdep.o \
+	mips-fbsd-tdep.o \
 	mips-linux-tdep.o \
 	mips-nbsd-tdep.o \
 	mips-sde-tdep.o \
@@ -2541,6 +2542,7 @@ ALLDEPFILES = \
 	microblaze-linux-tdep.c \
 	microblaze-tdep.c \
 	mingw-hdep.c \
+	mips-fbsd-tdep.c \
 	mips-linux-nat.c \
 	mips-linux-tdep.c \
 	mips-nbsd-nat.c \
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 3a1ea6f..1a43604 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -369,6 +369,11 @@ mips*-*-netbsd* | mips*-*-knetbsd*-gnu)
 	gdb_target_obs="mips-tdep.o mips-nbsd-tdep.o solib-svr4.o nbsd-tdep.o"
 	gdb_sim=../sim/mips/libsim.a
 	;;
+mips*-*-freebsd*)
+	# Target: MIPS running FreeBSD
+	gdb_target_obs="mips-tdep.o mips-fbsd-tdep.o solib-svr4.o fbsd-tdep.o"
+	gdb_sim=../sim/mips/libsim.a
+	;;
 mips64*-*-openbsd*)
 	# Target: OpenBSD/mips64
 	gdb_target_obs="mips-tdep.o mips64-obsd-tdep.o obsd-tdep.o solib-svr4.o"
diff --git a/gdb/mips-fbsd-tdep.c b/gdb/mips-fbsd-tdep.c
new file mode 100644
index 0000000..be9ff18
--- /dev/null
+++ b/gdb/mips-fbsd-tdep.c
@@ -0,0 +1,565 @@
+/* Target-dependent code for FreeBSD/mips.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This software was developed by SRI International and the University
+   of Cambridge Computer Laboratory under DARPA/AFRL contract
+   FA8750-10-C-0237 ("CTSRD"), as part of the DARPA CRASH research
+   programme.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   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 "osabi.h"
+#include "regset.h"
+#include "trad-frame.h"
+#include "tramp-frame.h"
+
+#include "fbsd-tdep.h"
+#include "mips-tdep.h"
+#include "mips-fbsd-tdep.h"
+
+#include "solib-svr4.h"
+
+/* Shorthand for some register numbers used below.  */
+#define MIPS_PC_REGNUM  MIPS_EMBED_PC_REGNUM
+#define MIPS_FP0_REGNUM MIPS_EMBED_FP0_REGNUM
+#define MIPS_FSR_REGNUM MIPS_EMBED_FP0_REGNUM + 32
+
+/* Core file support. */
+
+/* Number of registers in `struct reg' from <machine/reg.h>.  The
+   first 38 follow the standard MIPS layout.  The 39th holds
+   IC_INT_REG on RM7K and RM9K processors.  The 40th is a dummy for
+   padding.  */
+#define MIPS_FBSD_NUM_GREGS	40
+
+/* Number of registers in `struct fpreg' from <machine/reg.h>.  The
+   first 32 hold floating point registers.  33 holds the FSR.  The
+   34th is a dummy for padding.  */
+#define MIPS_FBSD_NUM_FPREGS	34
+
+/* Supply a single register.  If the source register size matches the
+   size the regcache expects, this can use regcache_raw_supply().  If
+   they are different, this copies the source register into a buffer
+   that can be passed to regcache_raw_supply().  */
+
+static void
+mips_fbsd_supply_reg (struct regcache *regcache, int regnum, const void *addr,
+		      size_t len)
+{
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+
+  if (register_size (gdbarch, regnum) == len)
+    regcache_raw_supply (regcache, regnum, addr);
+  else
+    {
+      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+      gdb_byte buf[MAX_REGISTER_SIZE];
+      LONGEST val;
+
+      val = extract_signed_integer ((const gdb_byte *) addr, len, byte_order);
+      store_signed_integer (buf, register_size (gdbarch, regnum), byte_order,
+			    val);
+      regcache_raw_supply (regcache, regnum, buf);
+    }
+}
+
+/* Collect a single register.  If the destination register size
+   matches the size the regcache expects, this can use
+   regcache_raw_supply().  If they are different, this fetches the
+   register via regcache_raw_supply() into a buffer and then copies it
+   into the final destination.  */
+
+static void
+mips_fbsd_collect_reg (const struct regcache *regcache, int regnum, void *addr,
+		       size_t len)
+{
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+
+  if (register_size (gdbarch, regnum) == len)
+    regcache_raw_collect (regcache, regnum, addr);
+  else
+    {
+      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+      gdb_byte buf[MAX_REGISTER_SIZE];
+      LONGEST val;
+
+      regcache_raw_collect (regcache, regnum, buf);
+      val = extract_signed_integer (buf, register_size (gdbarch, regnum),
+				    byte_order);
+      store_signed_integer ((gdb_byte *) addr, len, byte_order, val);
+    }
+}
+
+/* Supply the floating-point registers stored in FPREGS to REGCACHE.
+   Each floating-point register in FPREGS is REGSIZE bytes in
+   length.  */
+
+void
+mips_fbsd_supply_fpregs (struct regcache *regcache, int regnum,
+			 const void *fpregs, size_t regsize)
+{
+  const gdb_byte *regs = (const gdb_byte *) fpregs;
+  int i;
+
+  for (i = MIPS_FP0_REGNUM; i <= MIPS_FSR_REGNUM; i++)
+    if (regnum == i || regnum == -1)
+      mips_fbsd_supply_reg (regcache, i,
+			    regs + (i - MIPS_FP0_REGNUM) * regsize, regsize);
+}
+
+/* Supply the general-purpose registers stored in GREGS to REGCACHE.
+   Each general-purpose register in GREGS is REGSIZE bytes in
+   length.  */
+
+void
+mips_fbsd_supply_gregs (struct regcache *regcache, int regnum,
+			const void *gregs, size_t regsize)
+{
+  const gdb_byte *regs = (const gdb_byte *) gregs;
+  int i;
+
+  for (i = 0; i <= MIPS_PC_REGNUM; i++)
+    if (regnum == i || regnum == -1)
+      mips_fbsd_supply_reg (regcache, i, regs + i * regsize, regsize);
+}
+
+/* Collect the floating-point registers from REGCACHE and store them
+   in FPREGS.  Each floating-point register in FPREGS is REGSIZE bytes
+   in length.  */
+
+void
+mips_fbsd_collect_fpregs (const struct regcache *regcache, int regnum,
+			  void *fpregs, size_t regsize)
+{
+  gdb_byte *regs = (gdb_byte *) fpregs;
+  int i;
+
+  for (i = MIPS_FP0_REGNUM; i <= MIPS_FSR_REGNUM; i++)
+    if (regnum == i || regnum == -1)
+      mips_fbsd_collect_reg (regcache, i,
+			     regs + (i - MIPS_FP0_REGNUM) * regsize, regsize);
+}
+
+/* Collect the general-purpose registers from REGCACHE and store them
+   in GREGS.  Each general-purpose register in GREGS is REGSIZE bytes
+   in length.  */
+
+void
+mips_fbsd_collect_gregs (const struct regcache *regcache, int regnum,
+			 void *gregs, size_t regsize)
+{
+  gdb_byte *regs = (gdb_byte *) gregs;
+  int i;
+
+  for (i = 0; i <= MIPS_PC_REGNUM; i++)
+    if (regnum == i || regnum == -1)
+      mips_fbsd_collect_reg (regcache, i, regs + i * regsize, regsize);
+}
+
+/* Supply register REGNUM from the buffer specified by FPREGS and LEN
+   in the floating-point register set REGSET to register cache
+   REGCACHE.  If REGNUM is -1, do this for all registers in REGSET.  */
+
+static void
+mips_fbsd_supply_fpregset (const struct regset *regset,
+			   struct regcache *regcache,
+			   int regnum, const void *fpregs, size_t len)
+{
+  size_t regsize = mips_abi_regsize (get_regcache_arch (regcache));
+
+  gdb_assert (len >= MIPS_FBSD_NUM_FPREGS * regsize);
+
+  mips_fbsd_supply_fpregs (regcache, regnum, fpregs, regsize);
+}
+
+/* Collect register REGNUM from the register cache REGCACHE and store
+   it in the buffer specified by FPREGS and LEN in the floating-point
+   register set REGSET.  If REGNUM is -1, do this for all registers in
+   REGSET.  */
+
+static void
+mips_fbsd_collect_fpregset (const struct regset *regset,
+			    const struct regcache *regcache,
+			    int regnum, void *fpregs, size_t len)
+{
+  size_t regsize = mips_abi_regsize (get_regcache_arch (regcache));
+
+  gdb_assert (len >= MIPS_FBSD_NUM_FPREGS * regsize);
+
+  mips_fbsd_collect_fpregs (regcache, regnum, fpregs, regsize);
+}
+
+/* Supply register REGNUM from the buffer specified by GREGS and LEN
+   in the general-purpose register set REGSET to register cache
+   REGCACHE.  If REGNUM is -1, do this for all registers in REGSET.  */
+
+static void
+mips_fbsd_supply_gregset (const struct regset *regset,
+			  struct regcache *regcache, int regnum,
+			  const void *gregs, size_t len)
+{
+  size_t regsize = mips_abi_regsize (get_regcache_arch (regcache));
+
+  gdb_assert (len >= MIPS_FBSD_NUM_GREGS * regsize);
+
+  mips_fbsd_supply_gregs (regcache, regnum, gregs, regsize);
+}
+
+/* Collect register REGNUM from the register cache REGCACHE and store
+   it in the buffer specified by GREGS and LEN in the general-purpose
+   register set REGSET.  If REGNUM is -1, do this for all registers in
+   REGSET.  */
+
+static void
+mips_fbsd_collect_gregset (const struct regset *regset,
+			   const struct regcache *regcache,
+			   int regnum, void *gregs, size_t len)
+{
+  size_t regsize = mips_abi_regsize (get_regcache_arch (regcache));
+
+  gdb_assert (len >= MIPS_FBSD_NUM_GREGS * regsize);
+
+  mips_fbsd_collect_gregs (regcache, regnum, gregs, regsize);
+}
+
+/* FreeBSD/mips register sets.  */
+
+static const struct regset mips_fbsd_gregset =
+{
+  NULL,
+  mips_fbsd_supply_gregset,
+  mips_fbsd_collect_gregset,
+};
+
+static const struct regset mips_fbsd_fpregset =
+{
+  NULL,
+  mips_fbsd_supply_fpregset,
+  mips_fbsd_collect_fpregset,
+};
+
+/* Iterate over core file register note sections.  */
+
+static void
+mips_fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
+					iterate_over_regset_sections_cb *cb,
+					void *cb_data,
+					const struct regcache *regcache)
+{
+  size_t regsize = mips_abi_regsize (gdbarch);
+
+  cb (".reg", MIPS_FBSD_NUM_GREGS * regsize, &mips_fbsd_gregset,
+      NULL, cb_data);
+  cb (".reg2", MIPS_FBSD_NUM_FPREGS * regsize, &mips_fbsd_fpregset,
+      NULL, cb_data);
+}
+
+/* Signal trampoline support.  */
+
+#define FBSD_SYS_sigreturn	417
+
+#define MIPS_INST_LI_V0_SIGRETURN 0x24020000 + FBSD_SYS_sigreturn
+#define MIPS_INST_SYSCALL	0x0000000c
+#define MIPS_INST_BREAK		0x0000000d
+
+#define O32_SIGFRAME_UCONTEXT_OFFSET	(16)
+#define O32_SIGSET_T_SIZE	(16)
+
+#define O32_UCONTEXT_ONSTACK	(O32_SIGSET_T_SIZE)
+#define O32_UCONTEXT_PC		(O32_UCONTEXT_ONSTACK + 4)
+#define O32_UCONTEXT_REGS	(O32_UCONTEXT_PC + 4)
+#define O32_UCONTEXT_SR		(O32_UCONTEXT_REGS + 4 * 32)
+#define O32_UCONTEXT_LO		(O32_UCONTEXT_SR + 4)
+#define O32_UCONTEXT_HI		(O32_UCONTEXT_LO + 4)
+#define O32_UCONTEXT_FPUSED	(O32_UCONTEXT_HI + 4)
+#define O32_UCONTEXT_FPREGS	(O32_UCONTEXT_FPUSED + 4)
+
+#define O32_UCONTEXT_REG_SIZE	4
+
+static void
+mips_fbsd_sigframe_init (const struct tramp_frame *self,
+			 struct frame_info *this_frame,
+			 struct trad_frame_cache *cache,
+			 CORE_ADDR func)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  CORE_ADDR sp, ucontext_addr, addr;
+  int regnum;
+  gdb_byte buf[4];
+
+  /* We find the appropriate instance of `ucontext_t' at a
+     fixed offset in the signal frame.  */
+  sp = get_frame_register_signed (this_frame,
+				  MIPS_SP_REGNUM + gdbarch_num_regs (gdbarch));
+  ucontext_addr = sp + O32_SIGFRAME_UCONTEXT_OFFSET;
+
+  /* PC.  */
+  regnum = mips_regnum (gdbarch)->pc;
+  trad_frame_set_reg_addr (cache,
+			   regnum + gdbarch_num_regs (gdbarch),
+			   ucontext_addr + O32_UCONTEXT_PC);
+
+  /* GPRs.  */
+  for (regnum = MIPS_ZERO_REGNUM, addr = ucontext_addr + O32_UCONTEXT_REGS;
+       regnum <= MIPS_RA_REGNUM; regnum++, addr += O32_UCONTEXT_REG_SIZE)
+    trad_frame_set_reg_addr (cache,
+			     regnum + gdbarch_num_regs (gdbarch),
+			     addr);
+
+  regnum = MIPS_PS_REGNUM;
+  trad_frame_set_reg_addr (cache,
+			   regnum + gdbarch_num_regs (gdbarch),
+			   ucontext_addr + O32_UCONTEXT_SR);
+
+  /* HI and LO.  */
+  regnum = mips_regnum (gdbarch)->lo;
+  trad_frame_set_reg_addr (cache,
+			   regnum + gdbarch_num_regs (gdbarch),
+			   ucontext_addr + O32_UCONTEXT_LO);
+  regnum = mips_regnum (gdbarch)->hi;
+  trad_frame_set_reg_addr (cache,
+			   regnum + gdbarch_num_regs (gdbarch),
+			   ucontext_addr + O32_UCONTEXT_HI);
+
+  if (target_read_memory (ucontext_addr + O32_UCONTEXT_FPUSED, buf, 4) == 0 &&
+      extract_unsigned_integer (buf, 4, byte_order) != 0)
+    {
+      for (regnum = 0, addr = ucontext_addr + O32_UCONTEXT_FPREGS;
+	   regnum < 32; regnum++, addr += O32_UCONTEXT_REG_SIZE)
+	trad_frame_set_reg_addr (cache,
+				 regnum + gdbarch_fp0_regnum (gdbarch),
+				 addr);
+      trad_frame_set_reg_addr (cache, mips_regnum (gdbarch)->fp_control_status,
+			       addr);
+    }
+
+  trad_frame_set_id (cache, frame_id_build (sp, func));
+}
+
+#define MIPS_INST_ADDIU_A0_SP_O32 (0x27a40000 \
+				   + O32_SIGFRAME_UCONTEXT_OFFSET)
+
+static const struct tramp_frame mips_fbsd_sigframe =
+{
+  SIGTRAMP_FRAME,
+  MIPS_INSN32_SIZE,
+  {
+    { MIPS_INST_ADDIU_A0_SP_O32, -1 },	/* addiu   a0, sp, SIGF_UC */
+    { MIPS_INST_LI_V0_SIGRETURN, -1 },	/* li      v0, SYS_sigreturn */
+    { MIPS_INST_SYSCALL, -1 },		/* syscall */
+    { MIPS_INST_BREAK, -1 },		/* break */
+    { TRAMP_SENTINEL_INSN, -1 }
+  },
+  mips_fbsd_sigframe_init
+};
+
+#define N64_SIGFRAME_UCONTEXT_OFFSET	(32)
+#define N64_SIGSET_T_SIZE	(16)
+
+#define N64_UCONTEXT_ONSTACK	(N64_SIGSET_T_SIZE)
+#define N64_UCONTEXT_PC		(N64_UCONTEXT_ONSTACK + 8)
+#define N64_UCONTEXT_REGS	(N64_UCONTEXT_PC + 8)
+#define N64_UCONTEXT_SR		(N64_UCONTEXT_REGS + 8 * 32)
+#define N64_UCONTEXT_LO		(N64_UCONTEXT_SR + 8)
+#define N64_UCONTEXT_HI		(N64_UCONTEXT_LO + 8)
+#define N64_UCONTEXT_FPUSED	(N64_UCONTEXT_HI + 8)
+#define N64_UCONTEXT_FPREGS	(N64_UCONTEXT_FPUSED + 8)
+
+#define N64_UCONTEXT_REG_SIZE	8
+
+static void
+mips64_fbsd_sigframe_init (const struct tramp_frame *self,
+			   struct frame_info *this_frame,
+			   struct trad_frame_cache *cache,
+			   CORE_ADDR func)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  CORE_ADDR sp, ucontext_addr, addr;
+  int regnum;
+  gdb_byte buf[4];
+
+  /* We find the appropriate instance of `ucontext_t' at a
+     fixed offset in the signal frame.  */
+  sp = get_frame_register_signed (this_frame,
+				  MIPS_SP_REGNUM + gdbarch_num_regs (gdbarch));
+  ucontext_addr = sp + N64_SIGFRAME_UCONTEXT_OFFSET;
+
+  /* PC.  */
+  regnum = mips_regnum (gdbarch)->pc;
+  trad_frame_set_reg_addr (cache,
+			   regnum + gdbarch_num_regs (gdbarch),
+			   ucontext_addr + N64_UCONTEXT_PC);
+
+  /* GPRs.  */
+  for (regnum = MIPS_ZERO_REGNUM, addr = ucontext_addr + N64_UCONTEXT_REGS;
+       regnum <= MIPS_RA_REGNUM; regnum++, addr += N64_UCONTEXT_REG_SIZE)
+    trad_frame_set_reg_addr (cache,
+			     regnum + gdbarch_num_regs (gdbarch),
+			     addr);
+
+  regnum = MIPS_PS_REGNUM;
+  trad_frame_set_reg_addr (cache,
+			   regnum + gdbarch_num_regs (gdbarch),
+			   ucontext_addr + N64_UCONTEXT_SR);
+
+  /* HI and LO.  */
+  regnum = mips_regnum (gdbarch)->lo;
+  trad_frame_set_reg_addr (cache,
+			   regnum + gdbarch_num_regs (gdbarch),
+			   ucontext_addr + N64_UCONTEXT_LO);
+  regnum = mips_regnum (gdbarch)->hi;
+  trad_frame_set_reg_addr (cache,
+			   regnum + gdbarch_num_regs (gdbarch),
+			   ucontext_addr + N64_UCONTEXT_HI);
+
+  if (target_read_memory (ucontext_addr + N64_UCONTEXT_FPUSED, buf, 4) == 0 &&
+      extract_unsigned_integer (buf, 4, byte_order) != 0)
+    {
+      for (regnum = 0, addr = ucontext_addr + N64_UCONTEXT_FPREGS;
+	   regnum < 32; regnum++, addr += N64_UCONTEXT_REG_SIZE)
+	trad_frame_set_reg_addr (cache,
+				 regnum + gdbarch_fp0_regnum (gdbarch),
+				 addr);
+      trad_frame_set_reg_addr (cache, mips_regnum (gdbarch)->fp_control_status,
+			       addr);
+    }
+
+  trad_frame_set_id (cache, frame_id_build (sp, func));
+}
+
+#define MIPS_INST_DADDIU_A0_SP_N64 (0x67a40000 \
+				    + N64_SIGFRAME_UCONTEXT_OFFSET)
+
+static const struct tramp_frame mips64_fbsd_sigframe =
+{
+  SIGTRAMP_FRAME,
+  MIPS_INSN32_SIZE,
+  {
+    { MIPS_INST_DADDIU_A0_SP_N64, -1 },	/* daddiu  a0, sp, SIGF_UC */
+    { MIPS_INST_LI_V0_SIGRETURN, -1 },	/* li      v0, SYS_sigreturn */
+    { MIPS_INST_SYSCALL, -1 },		/* syscall */
+    { MIPS_INST_BREAK, -1 },		/* break */
+    { TRAMP_SENTINEL_INSN, -1 }
+  },
+  mips64_fbsd_sigframe_init
+};
+
+/* Shared library support.  */
+
+/* FreeBSD/mips uses a slightly different `struct link_map' than the
+   other FreeBSD platforms as it includes an additional `l_off'
+   member.  */
+
+static struct link_map_offsets *
+mips_fbsd_ilp32_fetch_link_map_offsets (void)
+{
+  static struct link_map_offsets lmo;
+  static struct link_map_offsets *lmp = NULL;
+
+  if (lmp == NULL)
+    {
+      lmp = &lmo;
+
+      lmo.r_version_offset = 0;
+      lmo.r_version_size = 4;
+      lmo.r_map_offset = 4;
+      lmo.r_brk_offset = 8;
+      lmo.r_ldsomap_offset = -1;
+
+      lmo.link_map_size = 24;
+      lmo.l_addr_offset = 0;
+      lmo.l_name_offset = 8;
+      lmo.l_ld_offset = 12;
+      lmo.l_next_offset = 16;
+      lmo.l_prev_offset = 20;
+    }
+
+  return lmp;
+}
+
+static struct link_map_offsets *
+mips_fbsd_lp64_fetch_link_map_offsets (void)
+{
+  static struct link_map_offsets lmo;
+  static struct link_map_offsets *lmp = NULL;
+
+  if (lmp == NULL)
+    {
+      lmp = &lmo;
+
+      lmo.r_version_offset = 0;
+      lmo.r_version_size = 4;
+      lmo.r_map_offset = 8;
+      lmo.r_brk_offset = 16;
+      lmo.r_ldsomap_offset = -1;
+
+      lmo.link_map_size = 48;
+      lmo.l_addr_offset = 0;
+      lmo.l_name_offset = 16;
+      lmo.l_ld_offset = 24;
+      lmo.l_next_offset = 32;
+      lmo.l_prev_offset = 40;
+    }
+
+  return lmp;
+}
+
+static void
+mips_fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  enum mips_abi abi = mips_abi (gdbarch);
+
+  /* Generic FreeBSD support.  */
+  fbsd_init_abi (info, gdbarch);
+
+  set_gdbarch_software_single_step (gdbarch, mips_software_single_step);
+
+  switch (abi)
+    {
+      case MIPS_ABI_O32:
+	tramp_frame_prepend_unwinder (gdbarch, &mips_fbsd_sigframe);
+	break;
+      case MIPS_ABI_N32:
+	break;
+      case MIPS_ABI_N64:
+	tramp_frame_prepend_unwinder (gdbarch, &mips64_fbsd_sigframe);
+	break;
+    }
+
+  set_gdbarch_iterate_over_regset_sections
+    (gdbarch, mips_fbsd_iterate_over_regset_sections);
+
+  /* FreeBSD/mips has SVR4-style shared libraries.  */
+  set_solib_svr4_fetch_link_map_offsets
+    (gdbarch, (gdbarch_ptr_bit (gdbarch) == 32 ?
+	       mips_fbsd_ilp32_fetch_link_map_offsets :
+	       mips_fbsd_lp64_fetch_link_map_offsets));
+}
+\f
+
+/* Provide a prototype to silence -Wmissing-prototypes.  */
+void _initialize_mips_fbsd_tdep (void);
+
+void
+_initialize_mips_fbsd_tdep (void)
+{
+  gdbarch_register_osabi (bfd_arch_mips, 0, GDB_OSABI_FREEBSD_ELF,
+			  mips_fbsd_init_abi);
+}
diff --git a/gdb/mips-fbsd-tdep.h b/gdb/mips-fbsd-tdep.h
new file mode 100644
index 0000000..ed5b729
--- /dev/null
+++ b/gdb/mips-fbsd-tdep.h
@@ -0,0 +1,28 @@
+/* Common target dependent code for GDB on MIPS systems running FreeBSD.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef MIPS_FBSD_TDEP_H
+#define MIPS_FBSD_TDEP_H
+
+void mips_fbsd_supply_fpregs (struct regcache *, int, const void *, size_t);
+void mips_fbsd_supply_gregs (struct regcache *, int, const void *, size_t);
+void mips_fbsd_collect_fpregs (const struct regcache *, int, void *, size_t);
+void mips_fbsd_collect_gregs (const struct regcache *, int, void *, size_t);
+
+#endif /* MIPS_FBSD_TDEP_H */
-- 
2.9.2

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 3/3] Add native target for FreeBSD/mips.
  2016-12-06 21:01 [PATCH v2 0/3] Add FreeBSD/mips targets to GDB John Baldwin
@ 2016-12-06 21:01 ` John Baldwin
  2016-12-08 18:53   ` Luis Machado
  2016-12-06 21:01 ` [PATCH v2 1/3] Use the ELF class to determine the word size for FreeBSD core notes John Baldwin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: John Baldwin @ 2016-12-06 21:01 UTC (permalink / raw)
  To: gdb-patches, binutils

This supports the o32 and n64 ABIs.

gdb/ChangeLog:

	* Makefile.in (ALLDEPFILES): Add mips-fbsd-nat.c.
	* config/mips/fbsd.mh: New file.
	* configure.host: Add mips*-*-freebsd*.
	* mips-fbsd-nat.c: New file.
---
 gdb/ChangeLog           |   7 +++
 gdb/Makefile.in         |   1 +
 gdb/config/mips/fbsd.mh |   3 ++
 gdb/configure.host      |   1 +
 gdb/mips-fbsd-nat.c     | 141 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 153 insertions(+)
 create mode 100644 gdb/config/mips/fbsd.mh
 create mode 100644 gdb/mips-fbsd-nat.c

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 890702e..fc43f81 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2016-12-06  John Baldwin  <jhb@FreeBSD.org>
 
+	* Makefile.in (ALLDEPFILES): Add mips-fbsd-nat.c.
+	* config/mips/fbsd.mh: New file.
+	* configure.host: Add mips*-*-freebsd*.
+	* mips-fbsd-nat.c: New file.
+
+2016-12-06  John Baldwin  <jhb@FreeBSD.org>
+
 	* Makefile.in (ALL_TARGET_OBS): Add mips-fbsd-tdep.o.
 	(ALLDEPFILES): Add mips-fbsd-tdep.c.
 	* configure.tgt: Add mips*-*-freebsd*.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index e34fa4a..dee9d73 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -2542,6 +2542,7 @@ ALLDEPFILES = \
 	microblaze-linux-tdep.c \
 	microblaze-tdep.c \
 	mingw-hdep.c \
+	mips-fbsd-nat.c \
 	mips-fbsd-tdep.c \
 	mips-linux-nat.c \
 	mips-linux-tdep.c \
diff --git a/gdb/config/mips/fbsd.mh b/gdb/config/mips/fbsd.mh
new file mode 100644
index 0000000..f433347
--- /dev/null
+++ b/gdb/config/mips/fbsd.mh
@@ -0,0 +1,3 @@
+# Host: FreeBSD/mips
+NATDEPFILES= fork-child.o inf-ptrace.o fbsd-nat.o mips-fbsd-nat.o
+HAVE_NATIVE_GCORE_HOST = 1
diff --git a/gdb/configure.host b/gdb/configure.host
index ef265eb..c45f61d 100644
--- a/gdb/configure.host
+++ b/gdb/configure.host
@@ -129,6 +129,7 @@ m88*-*-openbsd*)	gdb_host=obsd ;;
 mips*-*-linux*)		gdb_host=linux ;;
 mips*-*-netbsd* | mips*-*-knetbsd*-gnu)
 			gdb_host=nbsd ;;
+mips*-*-freebsd*)	gdb_host=fbsd ;;
 mips64*-*-openbsd*)	gdb_host=obsd64 ;;
 
 powerpc-*-aix* | rs6000-*-* | powerpc64-*-aix*)
diff --git a/gdb/mips-fbsd-nat.c b/gdb/mips-fbsd-nat.c
new file mode 100644
index 0000000..ec1dcd8
--- /dev/null
+++ b/gdb/mips-fbsd-nat.c
@@ -0,0 +1,141 @@
+/* Native-dependent code for FreeBSD/mips.
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This software was developed by SRI International and the University
+   of Cambridge Computer Laboratory under DARPA/AFRL contract
+   FA8750-10-C-0237 ("CTSRD"), as part of the DARPA CRASH research
+   programme.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   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 "inferior.h"
+#include "regcache.h"
+#include "target.h"
+
+#include <sys/types.h>
+#include <sys/ptrace.h>
+#include <machine/reg.h>
+
+#include "fbsd-nat.h"
+#include "mips-tdep.h"
+#include "mips-fbsd-tdep.h"
+#include "inf-ptrace.h"
+
+/* Determine if PT_GETREGS fetches this register.  */
+
+static bool
+getregs_supplies (struct gdbarch *gdbarch, int regnum)
+{
+  return ((regnum) >= MIPS_ZERO_REGNUM
+	  && (regnum) <= gdbarch_pc_regnum (gdbarch));
+}
+
+/* Fetch register REGNUM from the inferior.  If REGNUM is -1, do this
+   for all registers.  */
+
+static void
+mips_fbsd_fetch_inferior_registers (struct target_ops *ops,
+				    struct regcache *regcache, int regnum)
+{
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  if (regnum == -1 || getregs_supplies (gdbarch, regnum))
+    {
+      struct reg regs;
+
+      if (ptrace (PT_GETREGS, get_ptrace_pid (inferior_ptid),
+		  (PTRACE_TYPE_ARG3) &regs, 0) == -1)
+	perror_with_name (_("Couldn't get registers"));
+
+      mips_fbsd_supply_gregs (regcache, regnum, &regs, sizeof (register_t));
+      if (regnum != -1)
+	return;
+    }
+
+  if (regnum == -1
+      || regnum >= gdbarch_fp0_regnum (get_regcache_arch (regcache)))
+    {
+      struct fpreg fpregs;
+
+      if (ptrace (PT_GETFPREGS, get_ptrace_pid (inferior_ptid),
+		  (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
+	perror_with_name (_("Couldn't get floating point status"));
+
+      mips_fbsd_supply_fpregs (regcache, regnum, &fpregs,
+			       sizeof (f_register_t));
+    }
+}
+
+/* Store register REGNUM back into the inferior.  If REGNUM is -1, do
+   this for all registers.  */
+
+static void
+mips_fbsd_store_inferior_registers (struct target_ops *ops,
+				    struct regcache *regcache, int regnum)
+{
+  struct gdbarch *gdbarch = get_regcache_arch (regcache);
+  if (regnum == -1 || getregs_supplies (gdbarch, regnum))
+    {
+      struct reg regs;
+
+      if (ptrace (PT_GETREGS, get_ptrace_pid (inferior_ptid),
+		  (PTRACE_TYPE_ARG3) &regs, 0) == -1)
+	perror_with_name (_("Couldn't get registers"));
+
+      mips_fbsd_collect_gregs (regcache, regnum, (char *) &regs,
+			       sizeof (register_t));
+
+      if (ptrace (PT_SETREGS, get_ptrace_pid (inferior_ptid),
+		  (PTRACE_TYPE_ARG3) &regs, 0) == -1)
+	perror_with_name (_("Couldn't write registers"));
+
+      if (regnum != -1)
+	return;
+    }
+
+  if (regnum == -1
+      || regnum >= gdbarch_fp0_regnum (get_regcache_arch (regcache)))
+    {
+      struct fpreg fpregs;
+
+      if (ptrace (PT_GETFPREGS, get_ptrace_pid (inferior_ptid),
+		  (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
+	perror_with_name (_("Couldn't get floating point status"));
+
+      mips_fbsd_collect_fpregs (regcache, regnum, (char *) &fpregs,
+				sizeof (f_register_t));
+
+      if (ptrace (PT_SETFPREGS, get_ptrace_pid (inferior_ptid),
+		  (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
+	perror_with_name (_("Couldn't write floating point status"));
+    }
+}
+\f
+
+/* Provide a prototype to silence -Wmissing-prototypes.  */
+void _initialize_mips_fbsd_nat (void);
+
+void
+_initialize_mips_fbsd_nat (void)
+{
+  struct target_ops *t;
+
+  t = inf_ptrace_target ();
+  t->to_fetch_registers = mips_fbsd_fetch_inferior_registers;
+  t->to_store_registers = mips_fbsd_store_inferior_registers;
+  fbsd_nat_add_target (t);
+}
-- 
2.9.2

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] Add FreeBSD/mips architecture.
  2016-12-06 21:01 ` [PATCH v2 2/3] Add FreeBSD/mips architecture John Baldwin
@ 2016-12-08 18:47   ` Luis Machado
  2016-12-08 20:08     ` John Baldwin
  2016-12-16 12:22   ` GDB attribution policy (Re: [PATCH v2 2/3] Add FreeBSD/mips architecture.) Pedro Alves
  1 sibling, 1 reply; 16+ messages in thread
From: Luis Machado @ 2016-12-08 18:47 UTC (permalink / raw)
  To: John Baldwin, gdb-patches, binutils

On 12/06/2016 03:00 PM, John Baldwin wrote:
> +static void
> +mips_fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> +{
> +  enum mips_abi abi = mips_abi (gdbarch);
> +
> +  /* Generic FreeBSD support.  */
> +  fbsd_init_abi (info, gdbarch);
> +
> +  set_gdbarch_software_single_step (gdbarch, mips_software_single_step);
> +
> +  switch (abi)
> +    {
> +      case MIPS_ABI_O32:
> +	tramp_frame_prepend_unwinder (gdbarch, &mips_fbsd_sigframe);
> +	break;
> +      case MIPS_ABI_N32:
> +	break;
> +      case MIPS_ABI_N64:
> +	tramp_frame_prepend_unwinder (gdbarch, &mips64_fbsd_sigframe);
> +	break;
> +    }
> +
> +  set_gdbarch_iterate_over_regset_sections
> +    (gdbarch, mips_fbsd_iterate_over_regset_sections);
> +
> +  /* FreeBSD/mips has SVR4-style shared libraries.  */
> +  set_solib_svr4_fetch_link_map_offsets
> +    (gdbarch, (gdbarch_ptr_bit (gdbarch) == 32 ?
> +	       mips_fbsd_ilp32_fetch_link_map_offsets :
> +	       mips_fbsd_lp64_fetch_link_map_offsets));
> +}
> +\f
> +

Do we need to set a reasonable default in case abi is something unknown 
or undefined? Maybe as a fail-safe?

Other than that i have no further comments on this patch.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] Add native target for FreeBSD/mips.
  2016-12-06 21:01 ` [PATCH v2 3/3] Add native target for FreeBSD/mips John Baldwin
@ 2016-12-08 18:53   ` Luis Machado
  2016-12-08 20:09     ` John Baldwin
  0 siblings, 1 reply; 16+ messages in thread
From: Luis Machado @ 2016-12-08 18:53 UTC (permalink / raw)
  To: John Baldwin, gdb-patches, binutils

On 12/06/2016 03:00 PM, John Baldwin wrote:
> This supports the o32 and n64 ABIs.
>
> gdb/ChangeLog:
>
> 	* Makefile.in (ALLDEPFILES): Add mips-fbsd-nat.c.
> 	* config/mips/fbsd.mh: New file.
> 	* configure.host: Add mips*-*-freebsd*.
> 	* mips-fbsd-nat.c: New file.
> ---
>  gdb/ChangeLog           |   7 +++
>  gdb/Makefile.in         |   1 +
>  gdb/config/mips/fbsd.mh |   3 ++
>  gdb/configure.host      |   1 +
>  gdb/mips-fbsd-nat.c     | 141 ++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 153 insertions(+)
>  create mode 100644 gdb/config/mips/fbsd.mh
>  create mode 100644 gdb/mips-fbsd-nat.c
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 890702e..fc43f81 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,12 @@
>  2016-12-06  John Baldwin  <jhb@FreeBSD.org>
>
> +	* Makefile.in (ALLDEPFILES): Add mips-fbsd-nat.c.
> +	* config/mips/fbsd.mh: New file.
> +	* configure.host: Add mips*-*-freebsd*.
> +	* mips-fbsd-nat.c: New file.
> +
> +2016-12-06  John Baldwin  <jhb@FreeBSD.org>
> +
>  	* Makefile.in (ALL_TARGET_OBS): Add mips-fbsd-tdep.o.
>  	(ALLDEPFILES): Add mips-fbsd-tdep.c.
>  	* configure.tgt: Add mips*-*-freebsd*.
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index e34fa4a..dee9d73 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -2542,6 +2542,7 @@ ALLDEPFILES = \
>  	microblaze-linux-tdep.c \
>  	microblaze-tdep.c \
>  	mingw-hdep.c \
> +	mips-fbsd-nat.c \
>  	mips-fbsd-tdep.c \
>  	mips-linux-nat.c \
>  	mips-linux-tdep.c \
> diff --git a/gdb/config/mips/fbsd.mh b/gdb/config/mips/fbsd.mh
> new file mode 100644
> index 0000000..f433347
> --- /dev/null
> +++ b/gdb/config/mips/fbsd.mh
> @@ -0,0 +1,3 @@
> +# Host: FreeBSD/mips
> +NATDEPFILES= fork-child.o inf-ptrace.o fbsd-nat.o mips-fbsd-nat.o
> +HAVE_NATIVE_GCORE_HOST = 1

I suppose you tried the gcore command for native GDB on FreeBSD/MIPS and 
it worked fine?

> diff --git a/gdb/configure.host b/gdb/configure.host
> index ef265eb..c45f61d 100644
> --- a/gdb/configure.host
> +++ b/gdb/configure.host
> @@ -129,6 +129,7 @@ m88*-*-openbsd*)	gdb_host=obsd ;;
>  mips*-*-linux*)		gdb_host=linux ;;
>  mips*-*-netbsd* | mips*-*-knetbsd*-gnu)
>  			gdb_host=nbsd ;;
> +mips*-*-freebsd*)	gdb_host=fbsd ;;
>  mips64*-*-openbsd*)	gdb_host=obsd64 ;;
>
>  powerpc-*-aix* | rs6000-*-* | powerpc64-*-aix*)
> diff --git a/gdb/mips-fbsd-nat.c b/gdb/mips-fbsd-nat.c
> new file mode 100644
> index 0000000..ec1dcd8
> --- /dev/null
> +++ b/gdb/mips-fbsd-nat.c
> @@ -0,0 +1,141 @@
> +/* Native-dependent code for FreeBSD/mips.
> +
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +
> +   This software was developed by SRI International and the University
> +   of Cambridge Computer Laboratory under DARPA/AFRL contract
> +   FA8750-10-C-0237 ("CTSRD"), as part of the DARPA CRASH research
> +   programme.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   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 "inferior.h"
> +#include "regcache.h"
> +#include "target.h"
> +
> +#include <sys/types.h>
> +#include <sys/ptrace.h>
> +#include <machine/reg.h>
> +
> +#include "fbsd-nat.h"
> +#include "mips-tdep.h"
> +#include "mips-fbsd-tdep.h"
> +#include "inf-ptrace.h"
> +
> +/* Determine if PT_GETREGS fetches this register.  */
> +
> +static bool
> +getregs_supplies (struct gdbarch *gdbarch, int regnum)
> +{
> +  return ((regnum) >= MIPS_ZERO_REGNUM
> +	  && (regnum) <= gdbarch_pc_regnum (gdbarch));
> +}
> +

Not sure we need the extra ()'s around regnum.

Otherwise i have no further comments on this one.

I'm guessing someone from binutils' side will want to validate 1/3.

Thanks,
Luis

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] Add FreeBSD/mips architecture.
  2016-12-08 18:47   ` Luis Machado
@ 2016-12-08 20:08     ` John Baldwin
  2016-12-08 20:13       ` Luis Machado
  0 siblings, 1 reply; 16+ messages in thread
From: John Baldwin @ 2016-12-08 20:08 UTC (permalink / raw)
  To: gdb-patches, Luis Machado; +Cc: binutils

On Thursday, December 08, 2016 12:47:42 PM Luis Machado wrote:
> On 12/06/2016 03:00 PM, John Baldwin wrote:
> > +static void
> > +mips_fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> > +{
> > +  enum mips_abi abi = mips_abi (gdbarch);
> > +
> > +  /* Generic FreeBSD support.  */
> > +  fbsd_init_abi (info, gdbarch);
> > +
> > +  set_gdbarch_software_single_step (gdbarch, mips_software_single_step);
> > +
> > +  switch (abi)
> > +    {
> > +      case MIPS_ABI_O32:
> > +	tramp_frame_prepend_unwinder (gdbarch, &mips_fbsd_sigframe);
> > +	break;
> > +      case MIPS_ABI_N32:
> > +	break;
> > +      case MIPS_ABI_N64:
> > +	tramp_frame_prepend_unwinder (gdbarch, &mips64_fbsd_sigframe);
> > +	break;
> > +    }
> > +
> > +  set_gdbarch_iterate_over_regset_sections
> > +    (gdbarch, mips_fbsd_iterate_over_regset_sections);
> > +
> > +  /* FreeBSD/mips has SVR4-style shared libraries.  */
> > +  set_solib_svr4_fetch_link_map_offsets
> > +    (gdbarch, (gdbarch_ptr_bit (gdbarch) == 32 ?
> > +	       mips_fbsd_ilp32_fetch_link_map_offsets :
> > +	       mips_fbsd_lp64_fetch_link_map_offsets));
> > +}
> > +\f
> > +
> 
> Do we need to set a reasonable default in case abi is something unknown 
> or undefined? Maybe as a fail-safe?

Default in which sense, for the sigframe unwinder or something else?
I don't think there's any support in any toolchains I'm aware of to
generate FreeBSD binaries with other ABIs (no O64 in particular).

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] Add native target for FreeBSD/mips.
  2016-12-08 18:53   ` Luis Machado
@ 2016-12-08 20:09     ` John Baldwin
  2016-12-08 20:15       ` Luis Machado
  0 siblings, 1 reply; 16+ messages in thread
From: John Baldwin @ 2016-12-08 20:09 UTC (permalink / raw)
  To: Luis Machado; +Cc: gdb-patches, binutils

On Thursday, December 08, 2016 12:53:06 PM Luis Machado wrote:
> On 12/06/2016 03:00 PM, John Baldwin wrote:
> > This supports the o32 and n64 ABIs.
> >
> > gdb/ChangeLog:
> >
> > 	* Makefile.in (ALLDEPFILES): Add mips-fbsd-nat.c.
> > 	* config/mips/fbsd.mh: New file.
> > 	* configure.host: Add mips*-*-freebsd*.
> > 	* mips-fbsd-nat.c: New file.
> > ---
> >  gdb/ChangeLog           |   7 +++
> >  gdb/Makefile.in         |   1 +
> >  gdb/config/mips/fbsd.mh |   3 ++
> >  gdb/configure.host      |   1 +
> >  gdb/mips-fbsd-nat.c     | 141 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 153 insertions(+)
> >  create mode 100644 gdb/config/mips/fbsd.mh
> >  create mode 100644 gdb/mips-fbsd-nat.c
> >
> > diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> > index 890702e..fc43f81 100644
> > --- a/gdb/ChangeLog
> > +++ b/gdb/ChangeLog
> > @@ -1,5 +1,12 @@
> >  2016-12-06  John Baldwin  <jhb@FreeBSD.org>
> >
> > +	* Makefile.in (ALLDEPFILES): Add mips-fbsd-nat.c.
> > +	* config/mips/fbsd.mh: New file.
> > +	* configure.host: Add mips*-*-freebsd*.
> > +	* mips-fbsd-nat.c: New file.
> > +
> > +2016-12-06  John Baldwin  <jhb@FreeBSD.org>
> > +
> >  	* Makefile.in (ALL_TARGET_OBS): Add mips-fbsd-tdep.o.
> >  	(ALLDEPFILES): Add mips-fbsd-tdep.c.
> >  	* configure.tgt: Add mips*-*-freebsd*.
> > diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> > index e34fa4a..dee9d73 100644
> > --- a/gdb/Makefile.in
> > +++ b/gdb/Makefile.in
> > @@ -2542,6 +2542,7 @@ ALLDEPFILES = \
> >  	microblaze-linux-tdep.c \
> >  	microblaze-tdep.c \
> >  	mingw-hdep.c \
> > +	mips-fbsd-nat.c \
> >  	mips-fbsd-tdep.c \
> >  	mips-linux-nat.c \
> >  	mips-linux-tdep.c \
> > diff --git a/gdb/config/mips/fbsd.mh b/gdb/config/mips/fbsd.mh
> > new file mode 100644
> > index 0000000..f433347
> > --- /dev/null
> > +++ b/gdb/config/mips/fbsd.mh
> > @@ -0,0 +1,3 @@
> > +# Host: FreeBSD/mips
> > +NATDEPFILES= fork-child.o inf-ptrace.o fbsd-nat.o mips-fbsd-nat.o
> > +HAVE_NATIVE_GCORE_HOST = 1
> 
> I suppose you tried the gcore command for native GDB on FreeBSD/MIPS and 
> it worked fine?

It mostly does.  I can read the core fine using native GDB on MIPS, but
an x86 GDB doesn't parse the generated core correctly (though it does
parse a native core from the kernel correctly).  The issue here though
isn't really a MIPS-specific one.  Right now the FreeBSD target-dependent
code uses code from binutils to write out the prpsinfo and prstatus notes
and those write Linux-style "CORE" notes.  I have a todo to fix gcore in
gdb to write out "FreeBSD" process and thread status notes instead which
will fix this (along with a separate, but somewhat similar issue with
gdb's gcore on FreeBSD/powerpc).
 
> > +/* Determine if PT_GETREGS fetches this register.  */
> > +
> > +static bool
> > +getregs_supplies (struct gdbarch *gdbarch, int regnum)
> > +{
> > +  return ((regnum) >= MIPS_ZERO_REGNUM
> > +	  && (regnum) <= gdbarch_pc_regnum (gdbarch));
> > +}
> > +
> 
> Not sure we need the extra ()'s around regnum.

Oops, will fix.

> Otherwise i have no further comments on this one.
> 
> I'm guessing someone from binutils' side will want to validate 1/3.

Yes.

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] Add FreeBSD/mips architecture.
  2016-12-08 20:08     ` John Baldwin
@ 2016-12-08 20:13       ` Luis Machado
  2016-12-09 19:02         ` John Baldwin
  0 siblings, 1 reply; 16+ messages in thread
From: Luis Machado @ 2016-12-08 20:13 UTC (permalink / raw)
  To: John Baldwin, gdb-patches; +Cc: binutils

On 12/08/2016 02:08 PM, John Baldwin wrote:
> On Thursday, December 08, 2016 12:47:42 PM Luis Machado wrote:
>> On 12/06/2016 03:00 PM, John Baldwin wrote:
>>> +static void
>>> +mips_fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>>> +{
>>> +  enum mips_abi abi = mips_abi (gdbarch);
>>> +
>>> +  /* Generic FreeBSD support.  */
>>> +  fbsd_init_abi (info, gdbarch);
>>> +
>>> +  set_gdbarch_software_single_step (gdbarch, mips_software_single_step);
>>> +
>>> +  switch (abi)
>>> +    {
>>> +      case MIPS_ABI_O32:
>>> +	tramp_frame_prepend_unwinder (gdbarch, &mips_fbsd_sigframe);
>>> +	break;
>>> +      case MIPS_ABI_N32:
>>> +	break;
>>> +      case MIPS_ABI_N64:
>>> +	tramp_frame_prepend_unwinder (gdbarch, &mips64_fbsd_sigframe);
>>> +	break;
>>> +    }
>>> +
>>> +  set_gdbarch_iterate_over_regset_sections
>>> +    (gdbarch, mips_fbsd_iterate_over_regset_sections);
>>> +
>>> +  /* FreeBSD/mips has SVR4-style shared libraries.  */
>>> +  set_solib_svr4_fetch_link_map_offsets
>>> +    (gdbarch, (gdbarch_ptr_bit (gdbarch) == 32 ?
>>> +	       mips_fbsd_ilp32_fetch_link_map_offsets :
>>> +	       mips_fbsd_lp64_fetch_link_map_offsets));
>>> +}
>>> +\f
>>> +
>>
>> Do we need to set a reasonable default in case abi is something unknown
>> or undefined? Maybe as a fail-safe?
>
> Default in which sense, for the sigframe unwinder or something else?
> I don't think there's any support in any toolchains I'm aware of to
> generate FreeBSD binaries with other ABIs (no O64 in particular).
>

For the sigframe unwinders. I was mostly concerned about the switch 
construct without a default case for something invalid.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/3] Add native target for FreeBSD/mips.
  2016-12-08 20:09     ` John Baldwin
@ 2016-12-08 20:15       ` Luis Machado
  0 siblings, 0 replies; 16+ messages in thread
From: Luis Machado @ 2016-12-08 20:15 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches, binutils

On 12/08/2016 02:03 PM, John Baldwin wrote:
> On Thursday, December 08, 2016 12:53:06 PM Luis Machado wrote:
>> On 12/06/2016 03:00 PM, John Baldwin wrote:
>>> This supports the o32 and n64 ABIs.
>>>
>>> gdb/ChangeLog:
>>>
>>> 	* Makefile.in (ALLDEPFILES): Add mips-fbsd-nat.c.
>>> 	* config/mips/fbsd.mh: New file.
>>> 	* configure.host: Add mips*-*-freebsd*.
>>> 	* mips-fbsd-nat.c: New file.
>>> ---
>>>  gdb/ChangeLog           |   7 +++
>>>  gdb/Makefile.in         |   1 +
>>>  gdb/config/mips/fbsd.mh |   3 ++
>>>  gdb/configure.host      |   1 +
>>>  gdb/mips-fbsd-nat.c     | 141 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>  5 files changed, 153 insertions(+)
>>>  create mode 100644 gdb/config/mips/fbsd.mh
>>>  create mode 100644 gdb/mips-fbsd-nat.c
>>>
>>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>>> index 890702e..fc43f81 100644
>>> --- a/gdb/ChangeLog
>>> +++ b/gdb/ChangeLog
>>> @@ -1,5 +1,12 @@
>>>  2016-12-06  John Baldwin  <jhb@FreeBSD.org>
>>>
>>> +	* Makefile.in (ALLDEPFILES): Add mips-fbsd-nat.c.
>>> +	* config/mips/fbsd.mh: New file.
>>> +	* configure.host: Add mips*-*-freebsd*.
>>> +	* mips-fbsd-nat.c: New file.
>>> +
>>> +2016-12-06  John Baldwin  <jhb@FreeBSD.org>
>>> +
>>>  	* Makefile.in (ALL_TARGET_OBS): Add mips-fbsd-tdep.o.
>>>  	(ALLDEPFILES): Add mips-fbsd-tdep.c.
>>>  	* configure.tgt: Add mips*-*-freebsd*.
>>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
>>> index e34fa4a..dee9d73 100644
>>> --- a/gdb/Makefile.in
>>> +++ b/gdb/Makefile.in
>>> @@ -2542,6 +2542,7 @@ ALLDEPFILES = \
>>>  	microblaze-linux-tdep.c \
>>>  	microblaze-tdep.c \
>>>  	mingw-hdep.c \
>>> +	mips-fbsd-nat.c \
>>>  	mips-fbsd-tdep.c \
>>>  	mips-linux-nat.c \
>>>  	mips-linux-tdep.c \
>>> diff --git a/gdb/config/mips/fbsd.mh b/gdb/config/mips/fbsd.mh
>>> new file mode 100644
>>> index 0000000..f433347
>>> --- /dev/null
>>> +++ b/gdb/config/mips/fbsd.mh
>>> @@ -0,0 +1,3 @@
>>> +# Host: FreeBSD/mips
>>> +NATDEPFILES= fork-child.o inf-ptrace.o fbsd-nat.o mips-fbsd-nat.o
>>> +HAVE_NATIVE_GCORE_HOST = 1
>>
>> I suppose you tried the gcore command for native GDB on FreeBSD/MIPS and
>> it worked fine?
>
> It mostly does.  I can read the core fine using native GDB on MIPS, but
> an x86 GDB doesn't parse the generated core correctly (though it does
> parse a native core from the kernel correctly).  The issue here though
> isn't really a MIPS-specific one.  Right now the FreeBSD target-dependent
> code uses code from binutils to write out the prpsinfo and prstatus notes
> and those write Linux-style "CORE" notes.  I have a todo to fix gcore in
> gdb to write out "FreeBSD" process and thread status notes instead which
> will fix this (along with a separate, but somewhat similar issue with
> gdb's gcore on FreeBSD/powerpc).
>

Ok. I'm just making sure the feature is mostly usable before we enable 
it (gcore) here. It sounds like it is.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/3] Add FreeBSD/mips architecture.
  2016-12-08 20:13       ` Luis Machado
@ 2016-12-09 19:02         ` John Baldwin
  0 siblings, 0 replies; 16+ messages in thread
From: John Baldwin @ 2016-12-09 19:02 UTC (permalink / raw)
  To: Luis Machado; +Cc: gdb-patches, binutils

On Thursday, December 08, 2016 02:13:17 PM Luis Machado wrote:
> On 12/08/2016 02:08 PM, John Baldwin wrote:
> > On Thursday, December 08, 2016 12:47:42 PM Luis Machado wrote:
> >> On 12/06/2016 03:00 PM, John Baldwin wrote:
> >>> +static void
> >>> +mips_fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> >>> +{
> >>> +  enum mips_abi abi = mips_abi (gdbarch);
> >>> +
> >>> +  /* Generic FreeBSD support.  */
> >>> +  fbsd_init_abi (info, gdbarch);
> >>> +
> >>> +  set_gdbarch_software_single_step (gdbarch, mips_software_single_step);
> >>> +
> >>> +  switch (abi)
> >>> +    {
> >>> +      case MIPS_ABI_O32:
> >>> +	tramp_frame_prepend_unwinder (gdbarch, &mips_fbsd_sigframe);
> >>> +	break;
> >>> +      case MIPS_ABI_N32:
> >>> +	break;
> >>> +      case MIPS_ABI_N64:
> >>> +	tramp_frame_prepend_unwinder (gdbarch, &mips64_fbsd_sigframe);
> >>> +	break;
> >>> +    }
> >>> +
> >>> +  set_gdbarch_iterate_over_regset_sections
> >>> +    (gdbarch, mips_fbsd_iterate_over_regset_sections);
> >>> +
> >>> +  /* FreeBSD/mips has SVR4-style shared libraries.  */
> >>> +  set_solib_svr4_fetch_link_map_offsets
> >>> +    (gdbarch, (gdbarch_ptr_bit (gdbarch) == 32 ?
> >>> +	       mips_fbsd_ilp32_fetch_link_map_offsets :
> >>> +	       mips_fbsd_lp64_fetch_link_map_offsets));
> >>> +}
> >>> +\f
> >>> +
> >>
> >> Do we need to set a reasonable default in case abi is something unknown
> >> or undefined? Maybe as a fail-safe?
> >
> > Default in which sense, for the sigframe unwinder or something else?
> > I don't think there's any support in any toolchains I'm aware of to
> > generate FreeBSD binaries with other ABIs (no O64 in particular).
> >
> 
> For the sigframe unwinders. I was mostly concerned about the switch 
> construct without a default case for something invalid.

Well, in that case we simply won't have an unwinder for signal frames, but
the target will work for other things.  That said, I don't know of any
existing toolchain that supports generating FreeBSD/mips binaries that
supports any ABIs other than o32, n32, and n64.  Specifically, FreeBSD
does not have any o64 support, and binutils doesn't have a linker emulation
for any other ABIs on FreeBSD/mips.

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] Use the ELF class to determine the word size for FreeBSD core notes.
  2016-12-06 21:01 ` [PATCH v2 1/3] Use the ELF class to determine the word size for FreeBSD core notes John Baldwin
@ 2016-12-13 11:04   ` Nick Clifton
  2016-12-23 20:35     ` John Baldwin
  0 siblings, 1 reply; 16+ messages in thread
From: Nick Clifton @ 2016-12-13 11:04 UTC (permalink / raw)
  To: John Baldwin, gdb-patches, binutils

Hi John,

> FreeBSD MIPS64 core dumps contain an empty e_flags value so are assigned
> to the default bfd_arch_mips which is 32-bits.

I am sorry, but I am worried about this patch breaking things for other,
non-MIPS targets.  Wouldn't it be easier to update bfd/elfxx-mips.c:_bfd_elf_mips_mach()
to test the EI_CLASS field if no flags are set.  Maybe by using the ABI_64_P macro ?

Cheers
  Nick

^ permalink raw reply	[flat|nested] 16+ messages in thread

* GDB attribution policy (Re: [PATCH v2 2/3] Add FreeBSD/mips architecture.)
  2016-12-06 21:01 ` [PATCH v2 2/3] Add FreeBSD/mips architecture John Baldwin
  2016-12-08 18:47   ` Luis Machado
@ 2016-12-16 12:22   ` Pedro Alves
  2017-01-10 16:19     ` Pedro Alves
  1 sibling, 1 reply; 16+ messages in thread
From: Pedro Alves @ 2016-12-16 12:22 UTC (permalink / raw)
  To: John Baldwin, gdb-patches, binutils

Hi John,

On 12/06/2016 09:00 PM, John Baldwin wrote:
> --- /dev/null
> +++ b/gdb/mips-fbsd-tdep.c
> @@ -0,0 +1,565 @@
> +/* Target-dependent code for FreeBSD/mips.
> +
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +
> +   This software was developed by SRI International and the University
> +   of Cambridge Computer Laboratory under DARPA/AFRL contract
> +   FA8750-10-C-0237 ("CTSRD"), as part of the DARPA CRASH research
> +   programme.

IMO, "contributed by" "written by", etc. attribution statements are best not
added to source files, and are better placed in the "Contributors" node in
the manual [1], which seems to exist exactly for this purpose.

I see a few advantages:

- It ends up surviving even if these new files are deleted in the future.

- It's discoverable by end users too, unlike some comment deep in some
  source file.

- Doesn't get inaccurate over time, as other contributors touch / rework / add
  to / mostly rewrite the code over the years, who understandably won't
  remember or won't feel comfortable with touching the original "written by"
  notes.

For similar reasons, a few years back, glibc explicitly stopped accepting
attribution statements in sources, as can be seen in their version of the
contribution checklist [2].  I believe the discussion that led to that glibc
policy started here [3].

I've discussed this with other GDB maintainers off list and it seems
there's general agreement to follow such a policy in GDB as well.

[1] - https://sourceware.org/gdb/current/onlinedocs/gdb/Contributors.html#Contributors
[2] - https://sourceware.org/glibc/wiki/Contribution%20checklist#Attribution
[3] - https://sourceware.org/ml/libc-alpha/2012-04/msg00339.html

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/3] Add FreeBSD/mips targets to GDB
  2016-12-06 21:01 [PATCH v2 0/3] Add FreeBSD/mips targets to GDB John Baldwin
                   ` (2 preceding siblings ...)
  2016-12-06 21:01 ` [PATCH v2 2/3] Add FreeBSD/mips architecture John Baldwin
@ 2016-12-16 12:50 ` Pedro Alves
  3 siblings, 0 replies; 16+ messages in thread
From: Pedro Alves @ 2016-12-16 12:50 UTC (permalink / raw)
  To: John Baldwin, gdb-patches, binutils

On 12/06/2016 09:00 PM, John Baldwin wrote:
> The first patch works around an issue where FreeBSD/mips core
> dumps from the kernel include an empty ei_flags value in the ELF
> header.  This causes bfd to select the default MIPS architecture
> (bfd_mach_3000) even though it is a 64-bit core file.  As a result,
> parsing the core dump notes doesn't work correctly since bits_per_word
> is 32.  It's not clear to me if bfd shouldn't default to a 64-bit
> MIPS machine if the ELF header indicates a 64-bit class instead of
> this change?
> 
> Changes since the first version are to address most (but perhaps not
> all) of the feedback from Luis Machado on patches 2 and 3.

The GDB changes are OK (once you've addressed the latest minor nits).

Thanks for the patches, and thanks Luis for reviewing this.

New targets/hosts deserve a gdb/NEWS entry.  Can you write one?

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/3] Use the ELF class to determine the word size for FreeBSD core notes.
  2016-12-13 11:04   ` Nick Clifton
@ 2016-12-23 20:35     ` John Baldwin
  0 siblings, 0 replies; 16+ messages in thread
From: John Baldwin @ 2016-12-23 20:35 UTC (permalink / raw)
  To: Nick Clifton; +Cc: gdb-patches, binutils

On Tuesday, December 13, 2016 11:04:40 AM Nick Clifton wrote:
> Hi John,
> 
> > FreeBSD MIPS64 core dumps contain an empty e_flags value so are assigned
> > to the default bfd_arch_mips which is 32-bits.
> 
> I am sorry, but I am worried about this patch breaking things for other,
> non-MIPS targets.  Wouldn't it be easier to update bfd/elfxx-mips.c:_bfd_elf_mips_mach()
> to test the EI_CLASS field if no flags are set.  Maybe by using the ABI_64_P macro ?

I agree and would rather choose a 64-bit MIPS default bfd_arch_mips in
this case if possible.  I wasn't sure how to go about that and if
doing so would break other things (whereas the function I changed
should only break things on FreeBSD if it is wrong since it controls
parsing of FreeBSD-specific notes).

Here is a patch that does that.  I have some other things to respin for
a v3 on the GDB side, but am including this inline to see what you think:

(In particular, with an empty flags value of 0, the arch field of the
flags is E_MIPS_ARCH_1 since E_MIPS_ARCH_1 is 0)

diff --git a/bfd/elf32-mips.c b/bfd/elf32-mips.c
index d398810..1ba7042 100644
--- a/bfd/elf32-mips.c
+++ b/bfd/elf32-mips.c
@@ -2295,7 +2295,7 @@ mips_elf32_object_p (bfd *abfd)
   if (SGI_COMPAT (abfd))
     elf_bad_symtab (abfd) = TRUE;
 
-  mach = _bfd_elf_mips_mach (elf_elfheader (abfd)->e_flags);
+  mach = _bfd_elf_mips_mach (abfd);
   bfd_default_set_arch_mach (abfd, bfd_arch_mips, mach);
   return TRUE;
 }
diff --git a/bfd/elf64-mips.c b/bfd/elf64-mips.c
index 8cd0a73..10ab5e9 100644
--- a/bfd/elf64-mips.c
+++ b/bfd/elf64-mips.c
@@ -4251,7 +4251,7 @@ mips_elf64_object_p (bfd *abfd)
   if (elf64_mips_irix_compat (abfd) != ict_none)
     elf_bad_symtab (abfd) = TRUE;
 
-  mach = _bfd_elf_mips_mach (elf_elfheader (abfd)->e_flags);
+  mach = _bfd_elf_mips_mach (abfd);
   bfd_default_set_arch_mach (abfd, bfd_arch_mips, mach);
   return TRUE;
 }
diff --git a/bfd/elfn32-mips.c b/bfd/elfn32-mips.c
index c7ca646..2864d09 100644
--- a/bfd/elfn32-mips.c
+++ b/bfd/elfn32-mips.c
@@ -3513,7 +3513,7 @@ mips_elf_n32_object_p (bfd *abfd)
   if (SGI_COMPAT (abfd))
     elf_bad_symtab (abfd) = TRUE;
 
-  mach = _bfd_elf_mips_mach (elf_elfheader (abfd)->e_flags);
+  mach = _bfd_elf_mips_mach (abfd);
   bfd_default_set_arch_mach (abfd, bfd_arch_mips, mach);
   return TRUE;
 }
diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
index 96317aa..9b25303 100644
--- a/bfd/elfxx-mips.c
+++ b/bfd/elfxx-mips.c
@@ -6721,8 +6721,10 @@ mips_elf_create_dynamic_relocation (bfd *output_bfd,
 /* Return the MACH for a MIPS e_flags value.  */
 
 unsigned long
-_bfd_elf_mips_mach (flagword flags)
+_bfd_elf_mips_mach (bfd *abfd)
 {
+  flagword flags = elf_elfheader (abfd)->e_flags;
+
   switch (flags & EF_MIPS_MACH)
     {
     case E_MIPS_MACH_3900:
@@ -6784,7 +6786,10 @@ _bfd_elf_mips_mach (flagword flags)
        {
        default:
        case E_MIPS_ARCH_1:
-         return bfd_mach_mips3000;
+         if (ABI_64_P (abfd))
+           return bfd_mach_mips4000;
+         else
+           return bfd_mach_mips3000;
 
        case E_MIPS_ARCH_2:
          return bfd_mach_mips6000;
@@ -6817,8 +6822,6 @@ _bfd_elf_mips_mach (flagword flags)
          return bfd_mach_mipsisa64r6;
        }
     }
-
-  return 0;
 }
 
 /* Return printable name for ABI.  */
diff --git a/bfd/elfxx-mips.h b/bfd/elfxx-mips.h
index 8ea7b0f..be95a1b 100644
--- a/bfd/elfxx-mips.h
+++ b/bfd/elfxx-mips.h
@@ -138,7 +138,7 @@ extern bfd_reloc_status_type _bfd_mips_elf_lo16_reloc
 extern bfd_reloc_status_type _bfd_mips_elf_generic_reloc
   (bfd *, arelent *, asymbol *, void *, asection *, bfd *, char **);
 extern unsigned long _bfd_elf_mips_mach
-  (flagword);
+  (bfd *);
 extern bfd_boolean _bfd_mips_relax_section
   (bfd *, asection *, struct bfd_link_info *, bfd_boolean *);
 extern bfd_vma _bfd_mips_elf_sign_extend


-- 
John Baldwin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: GDB attribution policy (Re: [PATCH v2 2/3] Add FreeBSD/mips architecture.)
  2016-12-16 12:22   ` GDB attribution policy (Re: [PATCH v2 2/3] Add FreeBSD/mips architecture.) Pedro Alves
@ 2017-01-10 16:19     ` Pedro Alves
  0 siblings, 0 replies; 16+ messages in thread
From: Pedro Alves @ 2017-01-10 16:19 UTC (permalink / raw)
  To: John Baldwin, gdb-patches, binutils

FYI, I've added this to the wiki now:

 https://sourceware.org/gdb/wiki/ContributionChecklist#Attribution

Thanks,
Pedro Alves

On 12/16/2016 12:22 PM, Pedro Alves wrote:

> IMO, "contributed by" "written by", etc. attribution statements are best not
> added to source files, and are better placed in the "Contributors" node in
> the manual [1], which seems to exist exactly for this purpose.
> 
> I see a few advantages:
> 
> - It ends up surviving even if these new files are deleted in the future.
> 
> - It's discoverable by end users too, unlike some comment deep in some
>   source file.
> 
> - Doesn't get inaccurate over time, as other contributors touch / rework / add
>   to / mostly rewrite the code over the years, who understandably won't
>   remember or won't feel comfortable with touching the original "written by"
>   notes.
> 
> For similar reasons, a few years back, glibc explicitly stopped accepting
> attribution statements in sources, as can be seen in their version of the
> contribution checklist [2].  I believe the discussion that led to that glibc
> policy started here [3].
> 
> I've discussed this with other GDB maintainers off list and it seems
> there's general agreement to follow such a policy in GDB as well.
> 
> [1] - https://sourceware.org/gdb/current/onlinedocs/gdb/Contributors.html#Contributors
> [2] - https://sourceware.org/glibc/wiki/Contribution%20checklist#Attribution
> [3] - https://sourceware.org/ml/libc-alpha/2012-04/msg00339.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-01-10 16:19 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-06 21:01 [PATCH v2 0/3] Add FreeBSD/mips targets to GDB John Baldwin
2016-12-06 21:01 ` [PATCH v2 3/3] Add native target for FreeBSD/mips John Baldwin
2016-12-08 18:53   ` Luis Machado
2016-12-08 20:09     ` John Baldwin
2016-12-08 20:15       ` Luis Machado
2016-12-06 21:01 ` [PATCH v2 1/3] Use the ELF class to determine the word size for FreeBSD core notes John Baldwin
2016-12-13 11:04   ` Nick Clifton
2016-12-23 20:35     ` John Baldwin
2016-12-06 21:01 ` [PATCH v2 2/3] Add FreeBSD/mips architecture John Baldwin
2016-12-08 18:47   ` Luis Machado
2016-12-08 20:08     ` John Baldwin
2016-12-08 20:13       ` Luis Machado
2016-12-09 19:02         ` John Baldwin
2016-12-16 12:22   ` GDB attribution policy (Re: [PATCH v2 2/3] Add FreeBSD/mips architecture.) Pedro Alves
2017-01-10 16:19     ` Pedro Alves
2016-12-16 12:50 ` [PATCH v2 0/3] Add FreeBSD/mips targets to GDB Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).