public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 01/11] Support the fs_base and gs_base registers on i386.
  2019-02-09  0:42 [PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
  2019-02-09  0:42 ` [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type John Baldwin
@ 2019-02-09  0:42 ` John Baldwin
  2019-02-09  0:42 ` [PATCH v2 11/11] Support TLS variables on FreeBSD/powerpc John Baldwin
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 28+ messages in thread
From: John Baldwin @ 2019-02-09  0:42 UTC (permalink / raw)
  To: gdb-patches

As on amd64, these registers hold the base address of the fs and gs
segments, respectively.  For i386 these two registers are 32 bits.

gdb/ChangeLog:

	* amd64-fbsd-nat.c (amd64_fbsd_nat_target::read_description):
	Update calls to i386_target_description to add 'segments'
	parameter.
	* amd64-tdep.c (amd64_init_abi): Set tdep->fsbase_regnum.  Don't
	add segment base registers.
	* arch/i386.c (i386_create_target_description): Add 'segments'
	parameter to enable segment base registers.
	* arch/i386.h (i386_create_target_description): Likewise.
	* features/i386/32bit-segments.xml: New file.
	* features/i386/32bit-segments.c: Generate.
	* i386-fbsd-nat.c (i386_fbsd_nat_target::read_description): Update
	call to i386_target_description to add 'segments' parameter.
	* i386-fbsd-tdep.c (i386fbsd_core_read_description): Likewise.
	* i386-go32-tdep.c (i386_go32_init_abi): Likewise.
	* i386-linux-tdep.c (i386_linux_read_description): Likewise.
	* i386-tdep.c (i386_validate_tdesc_p): Add segment base registers
	if feature is present.
	(i386_gdbarch_init): Pass I386_NUM_REGS to set_gdbarch_num_regs.
	Add 'segments' parameter to call to i386_target_description.
	(i386_target_description): Add 'segments' parameter to enable
	segment base registers.
	(_initialize_i386_tdep) [GDB_SELF_TEST]: Add 'segments' parameter
	to call to i386_target_description.
	* i386-tdep.h (struct gdbarch_tdep): Add 'fsbase_regnum'.
	(enum i386_regnum): Add I386_FSBASE_REGNUM and I386_GSBASE_REGNUM.
	Define I386_NUM_REGS.
	(i386_target_description): Add 'segments' parameter to enable
	segment base registers.

gdb/gdbserver/ChangeLog:

	* linux-x86-tdesc.c (i386_linux_read_description): Update call to
	i386_create_target_description for 'segments' parameter.
	* lynx-i386-low.c (lynx_i386_arch_setup): Likewise.
	* nto-x86-low.c (nto_x86_arch_setup): Likewise.
	* win32-i386-low.c (i386_arch_setup): Likewise.
---
 gdb/ChangeLog                        | 31 ++++++++++++++++++++++++++
 gdb/amd64-fbsd-nat.c                 |  4 ++--
 gdb/amd64-tdep.c                     | 10 +--------
 gdb/arch/i386.c                      |  6 ++++-
 gdb/arch/i386.h                      |  3 ++-
 gdb/features/i386/32bit-segments.c   | 15 +++++++++++++
 gdb/features/i386/32bit-segments.xml | 12 ++++++++++
 gdb/gdbserver/ChangeLog              |  8 +++++++
 gdb/gdbserver/linux-x86-tdesc.c      |  2 +-
 gdb/gdbserver/lynx-i386-low.c        |  2 +-
 gdb/gdbserver/nto-x86-low.c          |  2 +-
 gdb/gdbserver/win32-i386-low.c       |  2 +-
 gdb/i386-fbsd-nat.c                  |  2 +-
 gdb/i386-fbsd-tdep.c                 |  2 +-
 gdb/i386-go32-tdep.c                 |  2 +-
 gdb/i386-linux-tdep.c                |  2 +-
 gdb/i386-tdep.c                      | 33 +++++++++++++++++++++-------
 gdb/i386-tdep.h                      | 12 ++++++++--
 18 files changed, 119 insertions(+), 31 deletions(-)
 create mode 100644 gdb/features/i386/32bit-segments.c
 create mode 100644 gdb/features/i386/32bit-segments.xml

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index de727739f2..6a05ef594e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,34 @@
+2019-02-08  John Baldwin  <jhb@FreeBSD.org>
+
+	* amd64-fbsd-nat.c (amd64_fbsd_nat_target::read_description):
+	Update calls to i386_target_description to add 'segments'
+	parameter.
+	* amd64-tdep.c (amd64_init_abi): Set tdep->fsbase_regnum.  Don't
+	add segment base registers.
+	* arch/i386.c (i386_create_target_description): Add 'segments'
+	parameter to enable segment base registers.
+	* arch/i386.h (i386_create_target_description): Likewise.
+	* features/i386/32bit-segments.xml: New file.
+	* features/i386/32bit-segments.c: Generate.
+	* i386-fbsd-nat.c (i386_fbsd_nat_target::read_description): Update
+	call to i386_target_description to add 'segments' parameter.
+	* i386-fbsd-tdep.c (i386fbsd_core_read_description): Likewise.
+	* i386-go32-tdep.c (i386_go32_init_abi): Likewise.
+	* i386-linux-tdep.c (i386_linux_read_description): Likewise.
+	* i386-tdep.c (i386_validate_tdesc_p): Add segment base registers
+	if feature is present.
+	(i386_gdbarch_init): Pass I386_NUM_REGS to set_gdbarch_num_regs.
+	Add 'segments' parameter to call to i386_target_description.
+	(i386_target_description): Add 'segments' parameter to enable
+	segment base registers.
+	(_initialize_i386_tdep) [GDB_SELF_TEST]: Add 'segments' parameter
+	to call to i386_target_description.
+	* i386-tdep.h (struct gdbarch_tdep): Add 'fsbase_regnum'.
+	(enum i386_regnum): Add I386_FSBASE_REGNUM and I386_GSBASE_REGNUM.
+	Define I386_NUM_REGS.
+	(i386_target_description): Add 'segments' parameter to enable
+	segment base registers.
+
 2019-02-08  Jim Wilson  <jimw@sifive.com>
 
 	* riscv-linux-tdep.c (riscv_linux_fregmap): New.
diff --git a/gdb/amd64-fbsd-nat.c b/gdb/amd64-fbsd-nat.c
index 74ef240766..9fff763dd3 100644
--- a/gdb/amd64-fbsd-nat.c
+++ b/gdb/amd64-fbsd-nat.c
@@ -190,13 +190,13 @@ amd64_fbsd_nat_target::read_description ()
       if (is64)
 	return amd64_target_description (xcr0, true);
       else
-	return i386_target_description (xcr0);
+	return i386_target_description (xcr0, false);
     }
 #endif
   if (is64)
     return amd64_target_description (X86_XSTATE_SSE_MASK, true);
   else
-    return i386_target_description (X86_XSTATE_SSE_MASK);
+    return i386_target_description (X86_XSTATE_SSE_MASK, false);
 }
 
 #if defined(HAVE_PT_GETDBREGS) && defined(USE_SIGTRAP_SIGINFO)
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 3f61997d66..d5892954d7 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -3107,15 +3107,7 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
 
   if (tdesc_find_feature (tdesc, "org.gnu.gdb.i386.segments") != NULL)
     {
-      const struct tdesc_feature *feature =
-	  tdesc_find_feature (tdesc, "org.gnu.gdb.i386.segments");
-      struct tdesc_arch_data *tdesc_data_segments =
-	  (struct tdesc_arch_data *) info.tdep_info;
-
-      tdesc_numbered_register (feature, tdesc_data_segments,
-		       AMD64_FSBASE_REGNUM, "fs_base");
-      tdesc_numbered_register (feature, tdesc_data_segments,
-		       AMD64_GSBASE_REGNUM, "gs_base");
+      tdep->fsbase_regnum = AMD64_FSBASE_REGNUM;
     }
 
   if (tdesc_find_feature (tdesc, "org.gnu.gdb.i386.pkeys") != NULL)
diff --git a/gdb/arch/i386.c b/gdb/arch/i386.c
index 7d2901333b..ab24cf71cb 100644
--- a/gdb/arch/i386.c
+++ b/gdb/arch/i386.c
@@ -28,11 +28,12 @@
 #include "../features/i386/32bit-avx512.c"
 #include "../features/i386/32bit-mpx.c"
 #include "../features/i386/32bit-pkeys.c"
+#include "../features/i386/32bit-segments.c"
 
 /* Create i386 target descriptions according to XCR0.  */
 
 target_desc *
-i386_create_target_description (uint64_t xcr0, bool is_linux)
+i386_create_target_description (uint64_t xcr0, bool is_linux, bool segments)
 {
   target_desc *tdesc = allocate_target_description ();
 
@@ -53,6 +54,9 @@ i386_create_target_description (uint64_t xcr0, bool is_linux)
   if (is_linux)
     regnum = create_feature_i386_32bit_linux (tdesc, regnum);
 
+  if (segments)
+    regnum = create_feature_i386_32bit_segments (tdesc, regnum);
+
   if (xcr0 & X86_XSTATE_AVX)
     regnum = create_feature_i386_32bit_avx (tdesc, regnum);
 
diff --git a/gdb/arch/i386.h b/gdb/arch/i386.h
index fa85438080..9a831cea30 100644
--- a/gdb/arch/i386.h
+++ b/gdb/arch/i386.h
@@ -21,6 +21,7 @@
 #include "common/tdesc.h"
 #include <stdint.h>
 
-target_desc *i386_create_target_description (uint64_t xcr0, bool is_linux);
+target_desc *i386_create_target_description (uint64_t xcr0, bool is_linux,
+					     bool segments);
 
 #endif /* ARCH_I386_H */
diff --git a/gdb/features/i386/32bit-segments.c b/gdb/features/i386/32bit-segments.c
new file mode 100644
index 0000000000..c22c3dfbc3
--- /dev/null
+++ b/gdb/features/i386/32bit-segments.c
@@ -0,0 +1,15 @@
+/* THIS FILE IS GENERATED.  -*- buffer-read-only: t -*- vi:set ro:
+  Original: 32bit-segments.xml */
+
+#include "common/tdesc.h"
+
+static int
+create_feature_i386_32bit_segments (struct target_desc *result, long regnum)
+{
+  struct tdesc_feature *feature;
+
+  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.segments");
+  tdesc_create_reg (feature, "fs_base", regnum++, 1, NULL, 32, "int");
+  tdesc_create_reg (feature, "gs_base", regnum++, 1, NULL, 32, "int");
+  return regnum;
+}
diff --git a/gdb/features/i386/32bit-segments.xml b/gdb/features/i386/32bit-segments.xml
new file mode 100644
index 0000000000..098948e5ec
--- /dev/null
+++ b/gdb/features/i386/32bit-segments.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2016-2018 Free Software Foundation, Inc.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.i386.segments">
+  <reg name="fs_base" bitsize="32" type="int"/>
+  <reg name="gs_base" bitsize="32" type="int"/>
+</feature>
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index e9fe5ab03f..190db936d0 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,11 @@
+2019-02-08  John Baldwin  <jhb@FreeBSD.org>
+
+	* linux-x86-tdesc.c (i386_linux_read_description): Update call to
+	i386_create_target_description for 'segments' parameter.
+	* lynx-i386-low.c (lynx_i386_arch_setup): Likewise.
+	* nto-x86-low.c (nto_x86_arch_setup): Likewise.
+	* win32-i386-low.c (i386_arch_setup): Likewise.
+
 2019-02-07  Alan Hayward  <alan.hayward@arm.com>
 
 	* linux-low.c (linux_attach): Add process before lwp.
diff --git a/gdb/gdbserver/linux-x86-tdesc.c b/gdb/gdbserver/linux-x86-tdesc.c
index 04bccc84ed..8f24a3d72d 100644
--- a/gdb/gdbserver/linux-x86-tdesc.c
+++ b/gdb/gdbserver/linux-x86-tdesc.c
@@ -87,7 +87,7 @@ i386_linux_read_description (uint64_t xcr0)
 
   if (*tdesc == NULL)
     {
-      *tdesc = i386_create_target_description (xcr0, true);
+      *tdesc = i386_create_target_description (xcr0, true, false);
 
       init_target_desc (*tdesc, i386_expedite_regs);
     }
diff --git a/gdb/gdbserver/lynx-i386-low.c b/gdb/gdbserver/lynx-i386-low.c
index bc1027dc52..e47f6b92f6 100644
--- a/gdb/gdbserver/lynx-i386-low.c
+++ b/gdb/gdbserver/lynx-i386-low.c
@@ -331,7 +331,7 @@ static void
 lynx_i386_arch_setup (void)
 {
   struct target_desc *tdesc
-    = i386_create_target_description (X86_XSTATE_SSE_MASK, false);
+    = i386_create_target_description (X86_XSTATE_SSE_MASK, false, false);
 
   init_target_desc (tdesc, i386_expedite_regs);
 
diff --git a/gdb/gdbserver/nto-x86-low.c b/gdb/gdbserver/nto-x86-low.c
index 1b00f7f6cc..cfbe7ba6d8 100644
--- a/gdb/gdbserver/nto-x86-low.c
+++ b/gdb/gdbserver/nto-x86-low.c
@@ -89,7 +89,7 @@ nto_x86_arch_setup (void)
 {
   the_low_target.num_regs = 16;
   struct target_desc *tdesc
-    = i386_create_target_description (X86_XSTATE_SSE_MASK, false);
+    = i386_create_target_description (X86_XSTATE_SSE_MASK, false, false);
 
   init_target_desc (tdesc, i386_expedite_regs);
 
diff --git a/gdb/gdbserver/win32-i386-low.c b/gdb/gdbserver/win32-i386-low.c
index 3be75d2bf2..7b187d3bea 100644
--- a/gdb/gdbserver/win32-i386-low.c
+++ b/gdb/gdbserver/win32-i386-low.c
@@ -439,7 +439,7 @@ i386_arch_setup (void)
 					   false, false);
   const char **expedite_regs = amd64_expedite_regs;
 #else
-  tdesc = i386_create_target_description (X86_XSTATE_SSE_MASK, false);
+  tdesc = i386_create_target_description (X86_XSTATE_SSE_MASK, false, false);
   const char **expedite_regs = i386_expedite_regs;
 #endif
 
diff --git a/gdb/i386-fbsd-nat.c b/gdb/i386-fbsd-nat.c
index 2309b76506..7106e90801 100644
--- a/gdb/i386-fbsd-nat.c
+++ b/gdb/i386-fbsd-nat.c
@@ -160,7 +160,7 @@ i386_fbsd_nat_target::read_description ()
   if (x86bsd_xsave_len == 0)
     xcr0 = X86_XSTATE_SSE_MASK;
 
-  return i386_target_description (xcr0);
+  return i386_target_description (xcr0, false);
 }
 #endif
 
diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c
index 236edd692a..2f28bad728 100644
--- a/gdb/i386-fbsd-tdep.c
+++ b/gdb/i386-fbsd-tdep.c
@@ -267,7 +267,7 @@ i386fbsd_core_read_description (struct gdbarch *gdbarch,
 				struct target_ops *target,
 				bfd *abfd)
 {
-  return i386_target_description (i386fbsd_core_read_xcr0 (abfd));
+  return i386_target_description (i386fbsd_core_read_xcr0 (abfd), false);
 }
 
 /* Similar to i386_supply_fpregset, but use XSAVE extended state.  */
diff --git a/gdb/i386-go32-tdep.c b/gdb/i386-go32-tdep.c
index 06833c346c..30db72d880 100644
--- a/gdb/i386-go32-tdep.c
+++ b/gdb/i386-go32-tdep.c
@@ -35,7 +35,7 @@ i386_go32_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 
   /* DJGPP does not support the SSE registers.  */
   if (!tdesc_has_registers (info.target_desc))
-    tdep->tdesc = i386_target_description (X86_XSTATE_X87_MASK);
+    tdep->tdesc = i386_target_description (X86_XSTATE_X87_MASK, false);
 
   /* Native compiler is GCC, which uses the SVR4 register numbering
      even in COFF and STABS.  See the comment in i386_gdbarch_init,
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index da81715061..fa6b86f1c8 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -694,7 +694,7 @@ i386_linux_read_description (uint64_t xcr0)
     [(xcr0 & X86_XSTATE_PKRU) ? 1 : 0];
 
   if (*tdesc == NULL)
-    *tdesc = i386_create_target_description (xcr0, true);
+    *tdesc = i386_create_target_description (xcr0, true, false);
 
   return *tdesc;
 }
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 663d510a91..01dfdaed3e 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8175,7 +8175,7 @@ i386_validate_tdesc_p (struct gdbarch_tdep *tdep,
   const struct tdesc_feature *feature_core;
 
   const struct tdesc_feature *feature_sse, *feature_avx, *feature_mpx,
-			     *feature_avx512, *feature_pkeys;
+			     *feature_avx512, *feature_pkeys, *feature_segments;
   int i, num_regs, valid_p;
 
   if (! tdesc_has_registers (tdesc))
@@ -8198,6 +8198,9 @@ i386_validate_tdesc_p (struct gdbarch_tdep *tdep,
   /* Try AVX512 registers.  */
   feature_avx512 = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.avx512");
 
+  /* Try segment base registers.  */
+  feature_segments = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.segments");
+
   /* Try PKEYS  */
   feature_pkeys = tdesc_find_feature (tdesc, "org.gnu.gdb.i386.pkeys");
 
@@ -8307,6 +8310,16 @@ i386_validate_tdesc_p (struct gdbarch_tdep *tdep,
 	    tdep->mpx_register_names[i]);
     }
 
+  if (feature_segments)
+    {
+      if (tdep->fsbase_regnum < 0)
+	tdep->fsbase_regnum = I386_FSBASE_REGNUM;
+      valid_p &= tdesc_numbered_register (feature_segments, tdesc_data,
+					  tdep->fsbase_regnum, "fs_base");
+      valid_p &= tdesc_numbered_register (feature_segments, tdesc_data,
+					  tdep->fsbase_regnum + 1, "gs_base");
+    }
+
   if (feature_pkeys)
     {
       tdep->xcr0 |= X86_XSTATE_PKRU;
@@ -8543,14 +8556,14 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Even though the default ABI only includes general-purpose registers,
      floating-point registers and the SSE registers, we have to leave a
      gap for the upper AVX, MPX and AVX512 registers.  */
-  set_gdbarch_num_regs (gdbarch, I386_PKEYS_NUM_REGS);
+  set_gdbarch_num_regs (gdbarch, I386_NUM_REGS);
 
   set_gdbarch_gnu_triplet_regexp (gdbarch, i386_gnu_triplet_regexp);
 
   /* Get the x86 target description from INFO.  */
   tdesc = info.target_desc;
   if (! tdesc_has_registers (tdesc))
-    tdesc = i386_target_description (X86_XSTATE_SSE_MASK);
+    tdesc = i386_target_description (X86_XSTATE_SSE_MASK, false);
   tdep->tdesc = tdesc;
 
   tdep->num_core_regs = I386_NUM_GREGS + I387_NUM_REGS;
@@ -8592,6 +8605,9 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   tdep->pkru_regnum = -1;
   tdep->num_pkeys_regs = 0;
 
+  /* No segment base registers.  */
+  tdep->fsbase_regnum = -1;
+
   tdesc_data = tdesc_data_alloc ();
 
   set_gdbarch_relocate_instruction (gdbarch, i386_relocate_instruction);
@@ -8717,20 +8733,21 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 /* Return the target description for a specified XSAVE feature mask.  */
 
 const struct target_desc *
-i386_target_description (uint64_t xcr0)
+i386_target_description (uint64_t xcr0, bool segments)
 {
   static target_desc *i386_tdescs \
-    [2/*SSE*/][2/*AVX*/][2/*MPX*/][2/*AVX512*/][2/*PKRU*/] = {};
+    [2/*SSE*/][2/*AVX*/][2/*MPX*/][2/*AVX512*/][2/*PKRU*/][2/*segments*/] = {};
   target_desc **tdesc;
 
   tdesc = &i386_tdescs[(xcr0 & X86_XSTATE_SSE) ? 1 : 0]
     [(xcr0 & X86_XSTATE_AVX) ? 1 : 0]
     [(xcr0 & X86_XSTATE_MPX) ? 1 : 0]
     [(xcr0 & X86_XSTATE_AVX512) ? 1 : 0]
-    [(xcr0 & X86_XSTATE_PKRU) ? 1 : 0];
+    [(xcr0 & X86_XSTATE_PKRU) ? 1 : 0]
+    [segments ? 1 : 0];
 
   if (*tdesc == NULL)
-    *tdesc = i386_create_target_description (xcr0, false);
+    *tdesc = i386_create_target_description (xcr0, false, segments);
 
   return *tdesc;
 }
@@ -9072,7 +9089,7 @@ Show Intel Memory Protection Extensions specific variables."),
 
   for (auto &a : xml_masks)
     {
-      auto tdesc = i386_target_description (a.mask);
+      auto tdesc = i386_target_description (a.mask, false);
 
       selftests::record_xml_tdesc (a.xml, tdesc);
     }
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 2532306e5c..c0d494824c 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -200,6 +200,10 @@ struct gdbarch_tdep
   /* PKEYS register names.  */
   const char **pkeys_register_names;
 
+  /* Register number for %fsbase.  Set this to -1 to indicate the
+     absence of segment base registers.  */
+  int fsbase_regnum;
+
   /* Target description.  */
   const struct target_desc *tdesc;
 
@@ -296,7 +300,9 @@ enum i386_regnum
   I386_K7_REGNUM = I386_K0_REGNUM + 7,
   I386_ZMM0H_REGNUM,		/* %zmm0h */
   I386_ZMM7H_REGNUM = I386_ZMM0H_REGNUM + 7,
-  I386_PKRU_REGNUM
+  I386_PKRU_REGNUM,
+  I386_FSBASE_REGNUM,
+  I386_GSBASE_REGNUM
 };
 
 /* Register numbers of RECORD_REGMAP.  */
@@ -337,6 +343,7 @@ enum record_i386_regnum
 #define I386_MPX_NUM_REGS	(I386_BNDSTATUS_REGNUM + 1)
 #define I386_AVX512_NUM_REGS	(I386_ZMM7H_REGNUM + 1)
 #define I386_PKEYS_NUM_REGS	(I386_PKRU_REGNUM + 1)
+#define I386_NUM_REGS		(I386_GSBASE_REGNUM + 1)
 
 /* Size of the largest register.  */
 #define I386_MAX_REGISTER_SIZE	64
@@ -440,7 +447,8 @@ extern int i386_svr4_reg_to_regnum (struct gdbarch *gdbarch, int reg);
 
 extern int i386_process_record (struct gdbarch *gdbarch,
                                 struct regcache *regcache, CORE_ADDR addr);
-extern const struct target_desc *i386_target_description (uint64_t xcr0);
+extern const struct target_desc *i386_target_description (uint64_t xcr0,
+							  bool segments);
 
 /* Return true iff the current target is MPX enabled.  */
 extern int i386_mpx_enabled (void);
-- 
2.19.2

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

* [PATCH v2 07/11] Add a helper function to resolve TLS variable addresses for FreeBSD.
  2019-02-09  0:42 [PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
                   ` (5 preceding siblings ...)
  2019-02-09  0:42 ` [PATCH v2 09/11] Support TLS variables on FreeBSD/i386 John Baldwin
@ 2019-02-09  0:42 ` John Baldwin
  2019-03-07 16:18   ` Simon Marchi
  2019-02-09  0:42 ` [PATCH v2 02/11] Support fs_base and gs_base on FreeBSD/i386 John Baldwin
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-02-09  0:42 UTC (permalink / raw)
  To: gdb-patches

The fbsd_get_thread_local_address function accepts the base address of
a thread's DTV array and the base address of an object file's link map
and uses this to compute a TLS variable's address.  FreeBSD
architectures use an architecture-specific method to determine the
address of the DTV array pointer and call this helper function to
perform the rest of the address calculation.

	* fbsd-tdep.c (fbsd_pspace_data_handle): New variable.
	(struct fbsd_pspace_data): New type.
	(get_fbsd_pspace_data, fbsd_pspace_data_cleanup)
	(fbsd_read_integer_by_name, fbsd_fetch_rtld_offsets)
	(fbsd_get_tls_index, fbsd_get_thread_local_address): New function.
	(_initialize_fbsd_tdep): Initialize 'fbsd_pspace_data_handle'.
	* fbsd-tdep.c (fbsd_get_thread_local_address): New prototype.
---
 gdb/ChangeLog   |  10 ++++
 gdb/fbsd-tdep.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/fbsd-tdep.h |  10 ++++
 3 files changed, 173 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c7fee7eb11..387d5c1aee 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2019-02-08  John Baldwin  <jhb@FreeBSD.org>
+
+	* fbsd-tdep.c (fbsd_pspace_data_handle): New variable.
+	(struct fbsd_pspace_data): New type.
+	(get_fbsd_pspace_data, fbsd_pspace_data_cleanup)
+	(fbsd_read_integer_by_name, fbsd_fetch_rtld_offsets)
+	(fbsd_get_tls_index, fbsd_get_thread_local_address): New function.
+	(_initialize_fbsd_tdep): Initialize 'fbsd_pspace_data_handle'.
+	* fbsd-tdep.c (fbsd_get_thread_local_address): New prototype.
+
 2019-02-08  John Baldwin  <jhb@FreeBSD.org>
 
 	* gdbtypes.c (lookup_struct_elt): New function.
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index d971d3a653..59d7c30f73 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -24,6 +24,7 @@
 #include "regcache.h"
 #include "regset.h"
 #include "gdbthread.h"
+#include "objfiles.h"
 #include "xml-syscall.h"
 #include <sys/socket.h>
 #include <arpa/inet.h>
@@ -444,6 +445,41 @@ get_fbsd_gdbarch_data (struct gdbarch *gdbarch)
 	  gdbarch_data (gdbarch, fbsd_gdbarch_data_handle));
 }
 
+/* Per-program-space data for FreeBSD architectures.  */
+static const struct program_space_data *fbsd_pspace_data_handle;
+
+struct fbsd_pspace_data
+  {
+    /* Offsets in the runtime linker's 'Obj_Entry' structure.  */
+    LONGEST off_linkmap;
+    LONGEST off_tlsindex;
+    bool rtld_offsets_valid;
+  };
+
+static struct fbsd_pspace_data *
+get_fbsd_pspace_data (struct program_space *pspace)
+{
+  struct fbsd_pspace_data *data;
+
+  data = ((struct fbsd_pspace_data *)
+	  program_space_data (pspace, fbsd_pspace_data_handle));
+  if (data == NULL)
+    {
+      data = XCNEW (struct fbsd_pspace_data);
+      set_program_space_data (pspace, fbsd_pspace_data_handle, data);
+    }
+
+  return data;
+}
+
+/* The cleanup callback for FreeBSD architecture per-program-space data.  */
+
+static void
+fbsd_pspace_data_cleanup (struct program_space *pspace, void *data)
+{
+  xfree (data);
+}
+
 /* This is how we want PTIDs from core files to be printed.  */
 
 static const char *
@@ -1932,6 +1968,121 @@ fbsd_get_syscall_number (struct gdbarch *gdbarch, thread_info *thread)
   internal_error (__FILE__, __LINE__, _("fbsd_get_sycall_number called"));
 }
 
+/* Read an integer symbol value from the current target.  */
+
+static LONGEST
+fbsd_read_integer_by_name (struct gdbarch *gdbarch, const char *name)
+{
+  bound_minimal_symbol ms = lookup_minimal_symbol (name, NULL, NULL);
+  if (ms.minsym == NULL)
+    error (_("Unable to resolve symbol '%s'"), name);
+
+  gdb_byte buf[4];
+  if (target_read_memory (BMSYMBOL_VALUE_ADDRESS (ms), buf, sizeof buf) != 0)
+    error (_("Unable to read value of '%s'"), name);
+
+  return extract_signed_integer (buf, sizeof buf, gdbarch_byte_order (gdbarch));
+}
+
+/* Lookup offsets of fields in the runtime linker's 'Obj_Entry'
+   structure needed to determine the TLS index of an object file.  */
+
+static void
+fbsd_fetch_rtld_offsets (struct gdbarch *gdbarch, struct fbsd_pspace_data *data)
+{
+  TRY
+    {
+      /* Fetch offsets from debug symbols in rtld.  */
+      struct symbol *obj_entry_sym
+	= lookup_symbol_in_language ("Struct_Obj_Entry", NULL, STRUCT_DOMAIN,
+				     language_c, NULL).symbol;
+      if (obj_entry_sym == NULL)
+	error (_("Unable to find Struct_Obj_Entry symbol"));
+      data->off_linkmap = lookup_struct_elt (SYMBOL_TYPE(obj_entry_sym),
+					     "linkmap", 0).offset / 8;
+      data->off_tlsindex = lookup_struct_elt (SYMBOL_TYPE(obj_entry_sym),
+					      "tlsindex", 0).offset / 8;
+      data->rtld_offsets_valid = true;
+      return;
+    }
+  CATCH (e, RETURN_MASK_ERROR)
+    {
+      data->off_linkmap = -1;
+    }
+  END_CATCH
+
+  TRY
+    {
+      /* Fetch offsets from global variables in libthr.  Note that
+	 this does not work for single-threaded processes that are not
+	 linked against libthr.  */
+      data->off_linkmap = fbsd_read_integer_by_name(gdbarch,
+						    "_thread_off_linkmap");
+      data->off_tlsindex = fbsd_read_integer_by_name(gdbarch,
+						     "_thread_off_tlsindex");
+      data->rtld_offsets_valid = true;
+      return;
+    }
+  CATCH (e, RETURN_MASK_ERROR)
+    {
+      data->off_linkmap = -1;
+    }
+  END_CATCH
+}
+
+/* Helper function to read the TLS index of an object file associated
+   with a link map entry at LM_ADDR.  */
+
+static LONGEST
+fbsd_get_tls_index (struct gdbarch *gdbarch, CORE_ADDR lm_addr)
+{
+  struct fbsd_pspace_data *data = get_fbsd_pspace_data (current_program_space);
+
+  if (!data->rtld_offsets_valid)
+    fbsd_fetch_rtld_offsets (gdbarch, data);
+
+  if (data->off_linkmap == -1)
+    throw_error (TLS_GENERIC_ERROR,
+		 _("Cannot fetch runtime linker structure offsets"));
+
+  /* Simulate container_of to convert from LM_ADDR to the Obj_Entry
+     pointer and then compute the offset of the tlsindex member.  */
+  CORE_ADDR tlsindex_addr = lm_addr - data->off_linkmap + data->off_tlsindex;
+
+  gdb_byte buf[4];
+  if (target_read_memory (tlsindex_addr, buf, sizeof buf) != 0)
+    throw_error (TLS_GENERIC_ERROR,
+		 _("Cannot find thread-local variables on this target"));
+
+  return extract_signed_integer (buf, sizeof buf, gdbarch_byte_order (gdbarch));
+}
+
+/* See fbsd-tdep.h.  */
+
+CORE_ADDR
+fbsd_get_thread_local_address (struct gdbarch *gdbarch, CORE_ADDR dtv_addr,
+			       CORE_ADDR lm_addr, CORE_ADDR offset)
+{
+  LONGEST tls_index = fbsd_get_tls_index (gdbarch, lm_addr);
+
+  gdb_byte buf[gdbarch_ptr_bit (gdbarch) / TARGET_CHAR_BIT];
+  if (target_read_memory (dtv_addr, buf, sizeof buf) != 0)
+    throw_error (TLS_GENERIC_ERROR,
+		 _("Cannot find thread-local variables on this target"));
+
+  const struct builtin_type *builtin = builtin_type (gdbarch);
+  CORE_ADDR addr = gdbarch_pointer_to_address (gdbarch,
+					       builtin->builtin_data_ptr, buf);
+
+  addr += (tls_index + 1) * TYPE_LENGTH (builtin->builtin_data_ptr);
+  if (target_read_memory (addr, buf, sizeof buf) != 0)
+    throw_error (TLS_GENERIC_ERROR,
+		 _("Cannot find thread-local variables on this target"));
+
+  addr = gdbarch_pointer_to_address (gdbarch, builtin->builtin_data_ptr, buf);
+  return addr + offset;
+}
+
 /* To be called from GDB_OSABI_FREEBSD handlers. */
 
 void
@@ -1957,4 +2108,6 @@ _initialize_fbsd_tdep (void)
 {
   fbsd_gdbarch_data_handle =
     gdbarch_data_register_post_init (init_fbsd_gdbarch_data);
+  fbsd_pspace_data_handle
+    = register_program_space_data_with_cleanup (NULL, fbsd_pspace_data_cleanup);
 }
diff --git a/gdb/fbsd-tdep.h b/gdb/fbsd-tdep.h
index efd7c2c78f..0b4d1e92c7 100644
--- a/gdb/fbsd-tdep.h
+++ b/gdb/fbsd-tdep.h
@@ -60,4 +60,14 @@ extern void fbsd_info_proc_mappings_entry (int addr_bit, ULONGEST kve_start,
 					   int kve_flags, int kve_protection,
 					   const void *kve_path);
 
+/* Helper function to fetch the address of a thread-local variable.
+   DTV_ADDR is the base address of the thread's dtv array.  LM_ADDR is
+   the address of the link_map structure for the associated object
+   file.  */
+
+extern CORE_ADDR fbsd_get_thread_local_address (struct gdbarch *gdbarch,
+						CORE_ADDR dtv_addr,
+						CORE_ADDR lm_addr,
+						CORE_ADDR offset);
+
 #endif /* fbsd-tdep.h */
-- 
2.19.2

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

* [PATCH v2 08/11] Support TLS variables on FreeBSD/amd64.
  2019-02-09  0:42 [PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
                   ` (2 preceding siblings ...)
  2019-02-09  0:42 ` [PATCH v2 11/11] Support TLS variables on FreeBSD/powerpc John Baldwin
@ 2019-02-09  0:42 ` John Baldwin
  2019-02-09  0:42 ` [PATCH v2 03/11] Handle an edge case for minisym TLS variable lookups John Baldwin
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 28+ messages in thread
From: John Baldwin @ 2019-02-09  0:42 UTC (permalink / raw)
  To: gdb-patches

Use the fs_base register to fetch the address of a thread's tcb and
calculate the address of the DTV array.  This value is then passed to
fbsd_get_thread_local_address to compute the final variable address.

Note that fs_base is currently only available via the native target as
core dumps on FreeBSD do not store the value of fs_base.

gdb/ChangeLog:

	* amd64-fbsd-tdep.c (amd64fbsd_get_thread_local_address): New.
	(amd64fbsd_init_abi): Install gdbarch
	"fetch_tls_load_module_address" and "get_thread_local_address"
	methods.
---
 gdb/ChangeLog         |  7 +++++++
 gdb/amd64-fbsd-tdep.c | 25 +++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 387d5c1aee..2ed15dc048 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-02-08  John Baldwin  <jhb@FreeBSD.org>
+
+	* amd64-fbsd-tdep.c (amd64fbsd_get_thread_local_address): New.
+	(amd64fbsd_init_abi): Install gdbarch
+	"fetch_tls_load_module_address" and "get_thread_local_address"
+	methods.
+
 2019-02-08  John Baldwin  <jhb@FreeBSD.org>
 
 	* fbsd-tdep.c (fbsd_pspace_data_handle): New variable.
diff --git a/gdb/amd64-fbsd-tdep.c b/gdb/amd64-fbsd-tdep.c
index 403e65022d..7e2e9edf21 100644
--- a/gdb/amd64-fbsd-tdep.c
+++ b/gdb/amd64-fbsd-tdep.c
@@ -204,6 +204,26 @@ amd64fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
       &amd64fbsd_xstateregset, "XSAVE extended state", cb_data);
 }
 
+/* Implement the get_thread_local_address gdbarch method.  */
+
+static CORE_ADDR
+amd64fbsd_get_thread_local_address (struct gdbarch *gdbarch, ptid_t ptid,
+				    CORE_ADDR lm_addr, CORE_ADDR offset)
+{
+  struct regcache *regcache;
+
+  regcache = get_thread_arch_regcache (ptid, gdbarch);
+
+  target_fetch_registers (regcache, AMD64_FSBASE_REGNUM);
+
+  ULONGEST fsbase;
+  if (regcache->cooked_read (AMD64_FSBASE_REGNUM, &fsbase) != REG_VALID)
+    error (_("Unable to fetch %%fsbase"));
+
+  CORE_ADDR dtv_addr = fsbase + gdbarch_ptr_bit (gdbarch) / 8;
+  return fbsd_get_thread_local_address (gdbarch, dtv_addr, lm_addr, offset);
+}
+
 static void
 amd64fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -241,6 +261,11 @@ amd64fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   /* FreeBSD uses SVR4-style shared libraries.  */
   set_solib_svr4_fetch_link_map_offsets
     (gdbarch, svr4_lp64_fetch_link_map_offsets);
+
+  set_gdbarch_fetch_tls_load_module_address (gdbarch,
+					     svr4_fetch_objfile_link_map);
+  set_gdbarch_get_thread_local_address (gdbarch,
+					amd64fbsd_get_thread_local_address);
 }
 
 void
-- 
2.19.2

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

* [PATCH v2 03/11] Handle an edge case for minisym TLS variable lookups.
  2019-02-09  0:42 [PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
                   ` (3 preceding siblings ...)
  2019-02-09  0:42 ` [PATCH v2 08/11] Support TLS variables on FreeBSD/amd64 John Baldwin
@ 2019-02-09  0:42 ` John Baldwin
  2019-02-09  0:42 ` [PATCH v2 09/11] Support TLS variables on FreeBSD/i386 John Baldwin
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 28+ messages in thread
From: John Baldwin @ 2019-02-09  0:42 UTC (permalink / raw)
  To: gdb-patches

If a TLS variable is provided by a minisym from a separate debug file,
the separate debug file is passed to
gdbarch_fetch_tls_load_module_address.  However, the object files
stored in the shared object list are the original object files, not
the separate debug object files.  In this case,
svr4_fetch_objfile_link_map was failing to find the link map entry
since the debug object file is not in its internal list, only the
original object file.

gdb/ChangeLog:

	* solib-svr4.c (svr4_fetch_objfile_link_map): Look for
	objfile->separate_debug_objfile_backlink if not NULL.
---
 gdb/ChangeLog    | 5 +++++
 gdb/solib-svr4.c | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9751118c0e..7f4e912ad3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2019-02-08  John Baldwin  <jhb@FreeBSD.org>
+
+	* solib-svr4.c (svr4_fetch_objfile_link_map): Look for
+	objfile->separate_debug_objfile_backlink if not NULL.
+
 2019-02-08  John Baldwin  <jhb@FreeBSD.org>
 
 	* amd64-bsd-nat.c (amd64bsd_fetch_inferior_registers): Use
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 787b98ba26..d7a792580d 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1548,6 +1548,11 @@ svr4_fetch_objfile_link_map (struct objfile *objfile)
   if (objfile == symfile_objfile)
     return info->main_lm_addr;
 
+  /* If OBJFILE is a separate debug object file, look for the
+     original object file.  */
+  if (objfile->separate_debug_objfile_backlink != NULL)
+    objfile = objfile->separate_debug_objfile_backlink;
+
   /* The other link map addresses may be found by examining the list
      of shared libraries.  */
   for (so = master_so_list (); so; so = so->next)
-- 
2.19.2

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

* [PATCH v2 00/11] Support for thread-local variables on FreeBSD
@ 2019-02-09  0:42 John Baldwin
  2019-02-09  0:42 ` [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type John Baldwin
                   ` (11 more replies)
  0 siblings, 12 replies; 28+ messages in thread
From: John Baldwin @ 2019-02-09  0:42 UTC (permalink / raw)
  To: gdb-patches

Relative to the first version the changes are:

- Patch 2 fixes some #ifdef's pointed by Simon in amd64-bsd-nat.c.
- Patch 3's description has been reworked to narrow down the cases in
  which it applies.
- Patch 5 is a new cleanup patch
- Patch 6 is a new patch that introduces a new method to lookup the
  offset of a field in a structure.  It is slightly more general than
  just returning offsets so that it can hopefully be used to replace
  the lk_find_field function in the Linux kernel patchset.
- Patch 7 (formerly 5) now uses the new method from Patch 6 to lookup
  the offsets of two fields in a runtime linker structure instead of
  parse_and_eval_long with manual offsetof.

John Baldwin (11):
  Support the fs_base and gs_base registers on i386.
  Support fs_base and gs_base on FreeBSD/i386.
  Handle an edge case for minisym TLS variable lookups.
  Add a new gdbarch method to resolve the address of TLS variables.
  Remove code disabled since at least 1999 from lookup_struct_elt_type.
  Add a more general version of lookup_struct_elt_type.
  Add a helper function to resolve TLS variable addresses for FreeBSD.
  Support TLS variables on FreeBSD/amd64.
  Support TLS variables on FreeBSD/i386.
  Support TLS variables on FreeBSD/riscv.
  Support TLS variables on FreeBSD/powerpc.

 gdb/ChangeLog                        | 108 +++++++++++++++++++
 gdb/amd64-bsd-nat.c                  |  26 +++--
 gdb/amd64-fbsd-nat.c                 |   4 +-
 gdb/amd64-fbsd-tdep.c                |  25 +++++
 gdb/amd64-tdep.c                     |  10 +-
 gdb/arch/i386.c                      |   6 +-
 gdb/arch/i386.h                      |   3 +-
 gdb/fbsd-tdep.c                      | 153 +++++++++++++++++++++++++++
 gdb/fbsd-tdep.h                      |  10 ++
 gdb/features/i386/32bit-segments.c   |  15 +++
 gdb/features/i386/32bit-segments.xml |  12 +++
 gdb/gdbarch.c                        |  32 ++++++
 gdb/gdbarch.h                        |   6 ++
 gdb/gdbarch.sh                       |   1 +
 gdb/gdbserver/ChangeLog              |   8 ++
 gdb/gdbserver/linux-x86-tdesc.c      |   2 +-
 gdb/gdbserver/lynx-i386-low.c        |   2 +-
 gdb/gdbserver/nto-x86-low.c          |   2 +-
 gdb/gdbserver/win32-i386-low.c       |   2 +-
 gdb/gdbtypes.c                       |  73 +++++++------
 gdb/gdbtypes.h                       |  19 ++++
 gdb/i386-bsd-nat.c                   |  54 ++++++++++
 gdb/i386-fbsd-nat.c                  |   2 +-
 gdb/i386-fbsd-tdep.c                 |  31 +++++-
 gdb/i386-go32-tdep.c                 |   2 +-
 gdb/i386-linux-tdep.c                |   2 +-
 gdb/i386-tdep.c                      |  33 ++++--
 gdb/i386-tdep.h                      |  12 ++-
 gdb/ppc-fbsd-tdep.c                  |  35 ++++++
 gdb/riscv-fbsd-tdep.c                |  27 +++++
 gdb/solib-svr4.c                     |   5 +
 gdb/target.c                         |  13 ++-
 32 files changed, 656 insertions(+), 79 deletions(-)
 create mode 100644 gdb/features/i386/32bit-segments.c
 create mode 100644 gdb/features/i386/32bit-segments.xml

-- 
2.19.2

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

* [PATCH v2 11/11] Support TLS variables on FreeBSD/powerpc.
  2019-02-09  0:42 [PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
  2019-02-09  0:42 ` [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type John Baldwin
  2019-02-09  0:42 ` [PATCH v2 01/11] Support the fs_base and gs_base registers on i386 John Baldwin
@ 2019-02-09  0:42 ` John Baldwin
  2019-03-07 16:26   ` Simon Marchi
  2019-02-09  0:42 ` [PATCH v2 08/11] Support TLS variables on FreeBSD/amd64 John Baldwin
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-02-09  0:42 UTC (permalink / raw)
  To: gdb-patches

Derive the pointer to the DTV array from the %r2 register on 32-bit
powerpc and %r13 on 64-bit powerpc.

gdb/ChangeLog:

	* ppc-fbsd-tdep.c (ppcfbsd_get_thread_local_address): New.
	(ppcfbsd_init_abi): Install gdbarch
	"fetch_tls_load_module_address" and "get_thread_local_address"
	methods.
---
 gdb/ChangeLog       |  7 +++++++
 gdb/ppc-fbsd-tdep.c | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6fa12ff815..90070b4039 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-02-08  John Baldwin  <jhb@FreeBSD.org>
+
+	* ppc-fbsd-tdep.c (ppcfbsd_get_thread_local_address): New.
+	(ppcfbsd_init_abi): Install gdbarch
+	"fetch_tls_load_module_address" and "get_thread_local_address"
+	methods.
+
 2019-02-08  John Baldwin  <jhb@FreeBSD.org>
 
 	* riscv-fbsd-tdep.c (riscv_fbsd_get_thread_local_address): New.
diff --git a/gdb/ppc-fbsd-tdep.c b/gdb/ppc-fbsd-tdep.c
index c21a52c898..290bd1fd88 100644
--- a/gdb/ppc-fbsd-tdep.c
+++ b/gdb/ppc-fbsd-tdep.c
@@ -279,6 +279,39 @@ ppcfbsd_return_value (struct gdbarch *gdbarch, struct value *function,
 					   regcache, readbuf, writebuf);
 }
 
+/* Implement the "get_thread_local_address" gdbarch method.  */
+
+static CORE_ADDR
+ppcfbsd_get_thread_local_address (struct gdbarch *gdbarch, ptid_t ptid,
+				  CORE_ADDR lm_addr, CORE_ADDR offset)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  struct regcache *regcache;
+  int tp_offset, tp_regnum;
+
+  regcache = get_thread_arch_regcache (ptid, gdbarch);
+
+  if (tdep->wordsize == 4)
+    {
+      tp_offset = 0x7008;
+      tp_regnum = PPC_R0_REGNUM + 2;
+    }
+  else
+    {
+      tp_offset = 0x7010;
+      tp_regnum = PPC_R0_REGNUM + 13;
+    }
+  target_fetch_registers (regcache, tp_regnum);
+
+  ULONGEST tp;
+  if (regcache->cooked_read (tp_regnum, &tp) != REG_VALID)
+    error (_("Unable to fetch tcb pointer"));
+
+  /* tp points to the end of the TCB block.  The first member of the
+     TCB is the pointer to the DTV array.  */
+  CORE_ADDR dtv_addr = tp - tp_offset;
+  return fbsd_get_thread_local_address (gdbarch, dtv_addr, lm_addr, offset);
+}
 
 static void
 ppcfbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
@@ -322,6 +355,8 @@ ppcfbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
 					     svr4_fetch_objfile_link_map);
+  set_gdbarch_get_thread_local_address (gdbarch,
+					ppcfbsd_get_thread_local_address);
 }
 
 void
-- 
2.19.2

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

* [PATCH v2 09/11] Support TLS variables on FreeBSD/i386.
  2019-02-09  0:42 [PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
                   ` (4 preceding siblings ...)
  2019-02-09  0:42 ` [PATCH v2 03/11] Handle an edge case for minisym TLS variable lookups John Baldwin
@ 2019-02-09  0:42 ` John Baldwin
  2019-02-09  0:42 ` [PATCH v2 07/11] Add a helper function to resolve TLS variable addresses for FreeBSD John Baldwin
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 28+ messages in thread
From: John Baldwin @ 2019-02-09  0:42 UTC (permalink / raw)
  To: gdb-patches

Derive the pointer to the DTV array from the gs_base register.  As
with FreeBSD/amd64, gs_base is currently only available via the native
target.

gdb/ChangeLog:

	* i386-fbsd-tdep.c (i386fbsd_get_thread_local_address): New.
	(i386fbsd_init_abi): Install gdbarch
	"fetch_tls_load_module_address" and "get_thread_local_address"
	methods.
---
 gdb/ChangeLog        |  7 +++++++
 gdb/i386-fbsd-tdep.c | 29 +++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2ed15dc048..7af211fd2d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-02-08  John Baldwin  <jhb@FreeBSD.org>
+
+	* i386-fbsd-tdep.c (i386fbsd_get_thread_local_address): New.
+	(i386fbsd_init_abi): Install gdbarch
+	"fetch_tls_load_module_address" and "get_thread_local_address"
+	methods.
+
 2019-02-08  John Baldwin  <jhb@FreeBSD.org>
 
 	* amd64-fbsd-tdep.c (amd64fbsd_get_thread_local_address): New.
diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c
index ac57e7383d..f274847174 100644
--- a/gdb/i386-fbsd-tdep.c
+++ b/gdb/i386-fbsd-tdep.c
@@ -320,6 +320,30 @@ i386fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
 	"XSAVE extended state", cb_data);
 }
 
+/* Implement the get_thread_local_address gdbarch method.  */
+
+static CORE_ADDR
+i386fbsd_get_thread_local_address (struct gdbarch *gdbarch, ptid_t ptid,
+				   CORE_ADDR lm_addr, CORE_ADDR offset)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+  struct regcache *regcache;
+
+  if (tdep->fsbase_regnum == -1)
+    error (_("Unable to fetch %%gsbase"));
+
+  regcache = get_thread_arch_regcache (ptid, gdbarch);
+
+  target_fetch_registers (regcache, tdep->fsbase_regnum + 1);
+
+  ULONGEST gsbase;
+  if (regcache->cooked_read (tdep->fsbase_regnum + 1, &gsbase) != REG_VALID)
+    error (_("Unable to fetch %%gsbase"));
+
+  CORE_ADDR dtv_addr = gsbase + gdbarch_ptr_bit (gdbarch) / 8;
+  return fbsd_get_thread_local_address (gdbarch, dtv_addr, lm_addr, offset);
+}
+
 static void
 i386fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -418,6 +442,11 @@ i386fbsd4_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 
   set_gdbarch_core_read_description (gdbarch,
 				     i386fbsd_core_read_description);
+
+  set_gdbarch_fetch_tls_load_module_address (gdbarch,
+					     svr4_fetch_objfile_link_map);
+  set_gdbarch_get_thread_local_address (gdbarch,
+					i386fbsd_get_thread_local_address);
 }
 
 void
-- 
2.19.2

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

* [PATCH v2 02/11] Support fs_base and gs_base on FreeBSD/i386.
  2019-02-09  0:42 [PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
                   ` (6 preceding siblings ...)
  2019-02-09  0:42 ` [PATCH v2 07/11] Add a helper function to resolve TLS variable addresses for FreeBSD John Baldwin
@ 2019-02-09  0:42 ` John Baldwin
  2019-02-09  0:50 ` [PATCH v2 10/11] Support TLS variables on FreeBSD/riscv John Baldwin
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 28+ messages in thread
From: John Baldwin @ 2019-02-09  0:42 UTC (permalink / raw)
  To: gdb-patches

The i386 BSD native target uses the same ptrace operations
(PT_[GS]ET[FG]SBASE) as the amd64 BSD native target to fetch and store
the registers.

The amd64 BSD native now uses 'tdep->fsbase_regnum' instead of
hardcoding AMD64_FSBASE_REGNUM and AMD64_GSBASE_REGNUM to support
32-bit targets.  In addition, the store operations explicitly zero the
new register value before fetching it from the register cache to
ensure 32-bit values are zero-extended.

gdb/ChangeLog:

	* amd64-bsd-nat.c (amd64bsd_fetch_inferior_registers): Use
	tdep->fsbase_regnum instead of constants for fs_base and gs_base.
	(amd64bsd_store_inferior_registers): Likewise.
	* amd64-fbsd-nat.c (amd64_fbsd_nat_target::read_description):
	Enable segment base registers.
	* i386-bsd-nat.c (i386bsd_fetch_inferior_registers): Use
	PT_GETFSBASE and PT_GETGSBASE.
	(i386bsd_store_inferior_registers): Use PT_SETFSBASE and PT_SETGSBASE.
	* i386-fbsd-nat.c (i386_fbsd_nat_target::read_description): Enable
	segment base registers.
	* i386-fbsd-tdep.c (i386fbsd_core_read_description): Likewise.
---
 gdb/ChangeLog        | 14 ++++++++++++
 gdb/amd64-bsd-nat.c  | 26 ++++++++++++++-------
 gdb/amd64-fbsd-nat.c |  4 ++--
 gdb/i386-bsd-nat.c   | 54 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/i386-fbsd-nat.c  |  2 +-
 gdb/i386-fbsd-tdep.c |  2 +-
 6 files changed, 90 insertions(+), 12 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6a05ef594e..9751118c0e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,17 @@
+2019-02-08  John Baldwin  <jhb@FreeBSD.org>
+
+	* amd64-bsd-nat.c (amd64bsd_fetch_inferior_registers): Use
+	tdep->fsbase_regnum instead of constants for fs_base and gs_base.
+	(amd64bsd_store_inferior_registers): Likewise.
+	* amd64-fbsd-nat.c (amd64_fbsd_nat_target::read_description):
+	Enable segment base registers.
+	* i386-bsd-nat.c (i386bsd_fetch_inferior_registers): Use
+	PT_GETFSBASE and PT_GETGSBASE.
+	(i386bsd_store_inferior_registers): Use PT_SETFSBASE and PT_SETGSBASE.
+	* i386-fbsd-nat.c (i386_fbsd_nat_target::read_description): Enable
+	segment base registers.
+	* i386-fbsd-tdep.c (i386fbsd_core_read_description): Likewise.
+
 2019-02-08  John Baldwin  <jhb@FreeBSD.org>
 
 	* amd64-fbsd-nat.c (amd64_fbsd_nat_target::read_description):
diff --git a/gdb/amd64-bsd-nat.c b/gdb/amd64-bsd-nat.c
index a2a91abb91..35763a5b95 100644
--- a/gdb/amd64-bsd-nat.c
+++ b/gdb/amd64-bsd-nat.c
@@ -43,6 +43,9 @@ amd64bsd_fetch_inferior_registers (struct regcache *regcache, int regnum)
 {
   struct gdbarch *gdbarch = regcache->arch ();
   pid_t pid = get_ptrace_pid (regcache->ptid ());
+#if defined(PT_GETFSBASE) || defined(PT_GETGSBASE)
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+#endif
 
   if (regnum == -1 || amd64_native_gregset_supplies_p (gdbarch, regnum))
     {
@@ -57,27 +60,27 @@ amd64bsd_fetch_inferior_registers (struct regcache *regcache, int regnum)
     }
 
 #ifdef PT_GETFSBASE
-  if (regnum == -1 || regnum == AMD64_FSBASE_REGNUM)
+  if (regnum == -1 || regnum == tdep->fsbase_regnum)
     {
       register_t base;
 
       if (ptrace (PT_GETFSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
 	perror_with_name (_("Couldn't get segment register fs_base"));
 
-      regcache->raw_supply (AMD64_FSBASE_REGNUM, &base);
+      regcache->raw_supply (tdep->fsbase_regnum, &base);
       if (regnum != -1)
 	return;
     }
 #endif
 #ifdef PT_GETGSBASE
-  if (regnum == -1 || regnum == AMD64_GSBASE_REGNUM)
+  if (regnum == -1 || regnum == tdep->fsbase_regnum + 1)
     {
       register_t base;
 
       if (ptrace (PT_GETGSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
 	perror_with_name (_("Couldn't get segment register gs_base"));
 
-      regcache->raw_supply (AMD64_GSBASE_REGNUM, &base);
+      regcache->raw_supply (tdep->fsbase_regnum + 1, &base);
       if (regnum != -1)
 	return;
     }
@@ -116,6 +119,9 @@ amd64bsd_store_inferior_registers (struct regcache *regcache, int regnum)
 {
   struct gdbarch *gdbarch = regcache->arch ();
   pid_t pid = get_ptrace_pid (regcache->ptid ());
+#if defined(PT_SETFSBASE) || defined(PT_SETGSBASE)
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+#endif
 
   if (regnum == -1 || amd64_native_gregset_supplies_p (gdbarch, regnum))
     {
@@ -134,11 +140,13 @@ amd64bsd_store_inferior_registers (struct regcache *regcache, int regnum)
     }
 
 #ifdef PT_SETFSBASE
-  if (regnum == -1 || regnum == AMD64_FSBASE_REGNUM)
+  if (regnum == -1 || regnum == tdep->fsbase_regnum)
     {
       register_t base;
 
-      regcache->raw_collect (AMD64_FSBASE_REGNUM, &base);
+      /* Clear the full base value to support 32-bit targets.  */
+      base = 0;
+      regcache->raw_collect (tdep->fsbase_regnum, &base);
 
       if (ptrace (PT_SETFSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
 	perror_with_name (_("Couldn't write segment register fs_base"));
@@ -147,11 +155,13 @@ amd64bsd_store_inferior_registers (struct regcache *regcache, int regnum)
     }
 #endif
 #ifdef PT_SETGSBASE
-  if (regnum == -1 || regnum == AMD64_GSBASE_REGNUM)
+  if (regnum == -1 || regnum == tdep->fsbase_regnum + 1)
     {
       register_t base;
 
-      regcache->raw_collect (AMD64_GSBASE_REGNUM, &base);
+      /* Clear the full base value to support 32-bit targets.  */
+      base = 0;
+      regcache->raw_collect (tdep->fsbase_regnum + 1, &base);
 
       if (ptrace (PT_SETGSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
 	perror_with_name (_("Couldn't write segment register gs_base"));
diff --git a/gdb/amd64-fbsd-nat.c b/gdb/amd64-fbsd-nat.c
index 9fff763dd3..cc676d3214 100644
--- a/gdb/amd64-fbsd-nat.c
+++ b/gdb/amd64-fbsd-nat.c
@@ -190,13 +190,13 @@ amd64_fbsd_nat_target::read_description ()
       if (is64)
 	return amd64_target_description (xcr0, true);
       else
-	return i386_target_description (xcr0, false);
+	return i386_target_description (xcr0, true);
     }
 #endif
   if (is64)
     return amd64_target_description (X86_XSTATE_SSE_MASK, true);
   else
-    return i386_target_description (X86_XSTATE_SSE_MASK, false);
+    return i386_target_description (X86_XSTATE_SSE_MASK, true);
 }
 
 #if defined(HAVE_PT_GETDBREGS) && defined(USE_SIGTRAP_SIGINFO)
diff --git a/gdb/i386-bsd-nat.c b/gdb/i386-bsd-nat.c
index 009a8dc1b2..a10b496096 100644
--- a/gdb/i386-bsd-nat.c
+++ b/gdb/i386-bsd-nat.c
@@ -144,6 +144,33 @@ i386bsd_fetch_inferior_registers (struct regcache *regcache, int regnum)
 	return;
     }
 
+#ifdef PT_GETFSBASE
+  if (regnum == -1 || regnum == I386_FSBASE_REGNUM)
+    {
+      register_t base;
+
+      if (ptrace (PT_GETFSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
+	perror_with_name (_("Couldn't get segment register fs_base"));
+
+      regcache->raw_supply (I386_FSBASE_REGNUM, &base);
+      if (regnum != -1)
+	return;
+    }
+#endif
+#ifdef PT_GETGSBASE
+  if (regnum == -1 || regnum == I386_GSBASE_REGNUM)
+    {
+      register_t base;
+
+      if (ptrace (PT_GETGSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
+	perror_with_name (_("Couldn't get segment register gs_base"));
+
+      regcache->raw_supply (I386_GSBASE_REGNUM, &base);
+      if (regnum != -1)
+	return;
+    }
+#endif
+
   if (regnum == -1 || regnum >= I386_ST0_REGNUM)
     {
       struct fpreg fpregs;
@@ -211,6 +238,33 @@ i386bsd_store_inferior_registers (struct regcache *regcache, int regnum)
 	return;
     }
 
+#ifdef PT_SETFSBASE
+  if (regnum == -1 || regnum == I386_FSBASE_REGNUM)
+    {
+      register_t base;
+
+      regcache->raw_collect (I386_FSBASE_REGNUM, &base);
+
+      if (ptrace (PT_SETFSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
+	perror_with_name (_("Couldn't write segment register fs_base"));
+      if (regnum != -1)
+	return;
+    }
+#endif
+#ifdef PT_SETGSBASE
+  if (regnum == -1 || regnum == I386_GSBASE_REGNUM)
+    {
+      register_t base;
+
+      regcache->raw_collect (I386_GSBASE_REGNUM, &base);
+
+      if (ptrace (PT_SETGSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
+	perror_with_name (_("Couldn't write segment register gs_base"));
+      if (regnum != -1)
+	return;
+    }
+#endif
+
   if (regnum == -1 || regnum >= I386_ST0_REGNUM)
     {
       struct fpreg fpregs;
diff --git a/gdb/i386-fbsd-nat.c b/gdb/i386-fbsd-nat.c
index 7106e90801..be5d4c67be 100644
--- a/gdb/i386-fbsd-nat.c
+++ b/gdb/i386-fbsd-nat.c
@@ -160,7 +160,7 @@ i386_fbsd_nat_target::read_description ()
   if (x86bsd_xsave_len == 0)
     xcr0 = X86_XSTATE_SSE_MASK;
 
-  return i386_target_description (xcr0, false);
+  return i386_target_description (xcr0, true);
 }
 #endif
 
diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c
index 2f28bad728..ac57e7383d 100644
--- a/gdb/i386-fbsd-tdep.c
+++ b/gdb/i386-fbsd-tdep.c
@@ -267,7 +267,7 @@ i386fbsd_core_read_description (struct gdbarch *gdbarch,
 				struct target_ops *target,
 				bfd *abfd)
 {
-  return i386_target_description (i386fbsd_core_read_xcr0 (abfd), false);
+  return i386_target_description (i386fbsd_core_read_xcr0 (abfd), true);
 }
 
 /* Similar to i386_supply_fpregset, but use XSAVE extended state.  */
-- 
2.19.2

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

* [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type.
  2019-02-09  0:42 [PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
@ 2019-02-09  0:42 ` John Baldwin
  2019-02-09  1:08   ` John Baldwin
  2019-03-07 15:53   ` Simon Marchi
  2019-02-09  0:42 ` [PATCH v2 01/11] Support the fs_base and gs_base registers on i386 John Baldwin
                   ` (10 subsequent siblings)
  11 siblings, 2 replies; 28+ messages in thread
From: John Baldwin @ 2019-02-09  0:42 UTC (permalink / raw)
  To: gdb-patches

lookup_struct_elt is a new function which returns a tuple of
information about a component of a structure or union.  The returned
tuple contains a pointer to the struct field object for the component
as well as a bit offset of that field within the structure.  If the
field names a field in an anonymous substructure, the offset is the
"global" offset relative to the original structure type.  If noerr is
set, then the returned tuple will set the field pointer to NULL to
indicate a missing component rather than throwing an error.

lookup_struct_elt_type is now reimplemented in terms of this new
function.  It simply returns the type of the returned field.

gdb/ChangeLog:

	* gdbtypes.c (lookup_struct_elt): New function.
	(lookup_struct_elt_type): Reimplement via lookup_struct_elt.
	* gdbtypes.h (struct struct_elt): New type.
	(lookup_struct_elt): New prototype.
---
 gdb/ChangeLog  |  7 ++++++
 gdb/gdbtypes.c | 60 ++++++++++++++++++++++++++++++++------------------
 gdb/gdbtypes.h | 19 ++++++++++++++++
 3 files changed, 65 insertions(+), 21 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index cebd63bcb5..c7fee7eb11 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-02-08  John Baldwin  <jhb@FreeBSD.org>
+
+	* gdbtypes.c (lookup_struct_elt): New function.
+	(lookup_struct_elt_type): Reimplement via lookup_struct_elt.
+	* gdbtypes.h (struct struct_elt): New type.
+	(lookup_struct_elt): New prototype.
+
 2019-02-08  John Baldwin  <jhb@FreeBSD.org>
 
 	* gdbtypes.c (lookup_struct_elt_type): Update comment and
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index e4acb0e985..0f3a450f9f 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1644,7 +1644,8 @@ lookup_template_type (char *name, struct type *type,
   return (SYMBOL_TYPE (sym));
 }
 
-/* Given a type TYPE, lookup the type of the component named NAME.
+/* Given a type TYPE, lookup the field and offset of the component named
+   NAME.
 
    TYPE can be either a struct or union, or a pointer or reference to
    a struct or union.  If it is a pointer or reference, its target
@@ -1652,11 +1653,11 @@ lookup_template_type (char *name, struct type *type,
    as specified for the definitions of the expression element types
    STRUCTOP_STRUCT and STRUCTOP_PTR.
 
-   If NOERR is nonzero, return NULL if there is no component named
-   NAME.  */
+   If NOERR is nonzero, the returned structure will have field set to
+   NULL if there is no component named NAME.  */
 
-struct type *
-lookup_struct_elt_type (struct type *type, const char *name, int noerr)
+struct_elt
+lookup_struct_elt (struct type *type, const char *name, int noerr)
 {
   int i;
 
@@ -1683,39 +1684,56 @@ lookup_struct_elt_type (struct type *type, const char *name, int noerr)
 
       if (t_field_name && (strcmp_iw (t_field_name, name) == 0))
 	{
-	  return TYPE_FIELD_TYPE (type, i);
+	  return struct_elt (&TYPE_FIELD(type, i), TYPE_FIELD_BITPOS (type, i));
 	}
      else if (!t_field_name || *t_field_name == '\0')
 	{
-	  struct type *subtype 
-	    = lookup_struct_elt_type (TYPE_FIELD_TYPE (type, i), name, 1);
-
-	  if (subtype != NULL)
-	    return subtype;
+	  struct_elt elt
+	    = lookup_struct_elt (TYPE_FIELD_TYPE (type, i), name, 1);
+	  if (elt.field != NULL)
+	    {
+	      elt.offset += TYPE_FIELD_BITPOS (type, i);
+	      return elt;
+	    }
 	}
     }
 
   /* OK, it's not in this class.  Recursively check the baseclasses.  */
   for (i = TYPE_N_BASECLASSES (type) - 1; i >= 0; i--)
     {
-      struct type *t;
-
-      t = lookup_struct_elt_type (TYPE_BASECLASS (type, i), name, 1);
-      if (t != NULL)
-	{
-	  return t;
-	}
+      struct_elt elt = lookup_struct_elt (TYPE_BASECLASS (type, i), name, 1);
+      if (elt.field != NULL)
+	return elt;
     }
 
   if (noerr)
-    {
-      return NULL;
-    }
+      return struct_elt ();
 
   std::string type_name = type_to_string (type);
   error (_("Type %s has no component named %s."), type_name.c_str (), name);
 }
 
+/* Given a type TYPE, lookup the type of the component named NAME.
+
+   TYPE can be either a struct or union, or a pointer or reference to
+   a struct or union.  If it is a pointer or reference, its target
+   type is automatically used.  Thus '.' and '->' are interchangable,
+   as specified for the definitions of the expression element types
+   STRUCTOP_STRUCT and STRUCTOP_PTR.
+
+   If NOERR is nonzero, return NULL if there is no component named
+   NAME.  */
+
+struct type *
+lookup_struct_elt_type (struct type *type, const char *name, int noerr)
+{
+  struct_elt elt = lookup_struct_elt (type, name, noerr);
+  if (elt.field != NULL)
+    return FIELD_TYPE (*elt.field);
+  else
+    return NULL;
+}
+
 /* Store in *MAX the largest number representable by unsigned integer type
    TYPE.  */
 
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index a6d4f64e9b..894c7b2fc6 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1873,6 +1873,25 @@ extern struct type *allocate_stub_method (struct type *);
 
 extern const char *type_name_or_error (struct type *type);
 
+struct struct_elt
+{
+  /* The field of the element, or NULL if no element was found.  */
+  struct field *field;
+
+  /* The bit offset of the element in the parent structure.  */
+  LONGEST offset;
+
+  struct_elt ()
+    : field (nullptr), offset (0)
+  {}
+
+  struct_elt (struct field *field, LONGEST offset)
+    : field (field), offset (offset)
+  {}
+};
+
+extern struct_elt lookup_struct_elt (struct type *, const char *, int);
+
 extern struct type *lookup_struct_elt_type (struct type *, const char *, int);
 
 extern struct type *make_pointer_type (struct type *, struct type **);
-- 
2.19.2

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

* [PATCH v2 10/11] Support TLS variables on FreeBSD/riscv.
  2019-02-09  0:42 [PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
                   ` (7 preceding siblings ...)
  2019-02-09  0:42 ` [PATCH v2 02/11] Support fs_base and gs_base on FreeBSD/i386 John Baldwin
@ 2019-02-09  0:50 ` John Baldwin
  2019-02-09  0:50 ` [PATCH v2 04/11] Add a new gdbarch method to resolve the address of TLS variables John Baldwin
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 28+ messages in thread
From: John Baldwin @ 2019-02-09  0:50 UTC (permalink / raw)
  To: gdb-patches

Derive the pointer to the DTV array from the tp register.

gdb/ChangeLog:

	* riscv-fbsd-tdep.c (riscv_fbsd_get_thread_local_address): New.
	(riscv_fbsd_init_abi): Install gdbarch
	"fetch_tls_load_module_address" and "get_thread_local_address"
	methods.
---
 gdb/ChangeLog         |  7 +++++++
 gdb/riscv-fbsd-tdep.c | 27 +++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7af211fd2d..6fa12ff815 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-02-08  John Baldwin  <jhb@FreeBSD.org>
+
+	* riscv-fbsd-tdep.c (riscv_fbsd_get_thread_local_address): New.
+	(riscv_fbsd_init_abi): Install gdbarch
+	"fetch_tls_load_module_address" and "get_thread_local_address"
+	methods.
+
 2019-02-08  John Baldwin  <jhb@FreeBSD.org>
 
 	* i386-fbsd-tdep.c (i386fbsd_get_thread_local_address): New.
diff --git a/gdb/riscv-fbsd-tdep.c b/gdb/riscv-fbsd-tdep.c
index 97ad28f59a..3125a2285e 100644
--- a/gdb/riscv-fbsd-tdep.c
+++ b/gdb/riscv-fbsd-tdep.c
@@ -174,6 +174,28 @@ static const struct tramp_frame riscv_fbsd_sigframe =
   riscv_fbsd_sigframe_init
 };
 
+/* Implement the "get_thread_local_address" gdbarch method.  */
+
+static CORE_ADDR
+riscv_fbsd_get_thread_local_address (struct gdbarch *gdbarch, ptid_t ptid,
+				     CORE_ADDR lm_addr, CORE_ADDR offset)
+{
+  struct regcache *regcache;
+
+  regcache = get_thread_arch_regcache (ptid, gdbarch);
+
+  target_fetch_registers (regcache, RISCV_TP_REGNUM);
+
+  ULONGEST tp;
+  if (regcache->cooked_read (RISCV_TP_REGNUM, &tp) != REG_VALID)
+    error (_("Unable to fetch %%tp"));
+
+  /* %tp points to the end of the TCB which contains two pointers.
+      The first pointer in the TCB points to the DTV array.  */
+  CORE_ADDR dtv_addr = tp - (gdbarch_ptr_bit (gdbarch) / 8) * 2;
+  return fbsd_get_thread_local_address (gdbarch, dtv_addr, lm_addr, offset);
+}
+
 /* Implement the 'init_osabi' method of struct gdb_osabi_handler.  */
 
 static void
@@ -193,6 +215,11 @@ riscv_fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 
   set_gdbarch_iterate_over_regset_sections
     (gdbarch, riscv_fbsd_iterate_over_regset_sections);
+
+  set_gdbarch_fetch_tls_load_module_address (gdbarch,
+					     svr4_fetch_objfile_link_map);
+  set_gdbarch_get_thread_local_address (gdbarch,
+					riscv_fbsd_get_thread_local_address);
 }
 
 void
-- 
2.19.2

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

* [PATCH v2 05/11] Remove code disabled since at least 1999 from lookup_struct_elt_type.
  2019-02-09  0:42 [PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
                   ` (9 preceding siblings ...)
  2019-02-09  0:50 ` [PATCH v2 04/11] Add a new gdbarch method to resolve the address of TLS variables John Baldwin
@ 2019-02-09  0:50 ` John Baldwin
  2019-03-07 16:25   ` Simon Marchi
  2019-02-22 17:22 ` [PING][PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
  11 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-02-09  0:50 UTC (permalink / raw)
  To: gdb-patches

Update the comment above the function to reflect the code removal and
document the existing behavior.

gdb/ChangeLog:

	* gdbtypes.c (lookup_struct_elt_type): Update comment and
	remove disabled code block.
---
 gdb/ChangeLog  |  5 +++++
 gdb/gdbtypes.c | 21 +++------------------
 2 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 621b9af800..cebd63bcb5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2019-02-08  John Baldwin  <jhb@FreeBSD.org>
+
+	* gdbtypes.c (lookup_struct_elt_type): Update comment and
+	remove disabled code block.
+
 2019-02-08  John Baldwin  <jhb@FreeBSD.org>
 
 	* gdbarch.sh (get_thread_local_address): New method.
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index d1ca304a92..e4acb0e985 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1644,8 +1644,7 @@ lookup_template_type (char *name, struct type *type,
   return (SYMBOL_TYPE (sym));
 }
 
-/* Given a type TYPE, lookup the type of the component of type named
-   NAME.
+/* Given a type TYPE, lookup the type of the component named NAME.
 
    TYPE can be either a struct or union, or a pointer or reference to
    a struct or union.  If it is a pointer or reference, its target
@@ -1653,8 +1652,8 @@ lookup_template_type (char *name, struct type *type,
    as specified for the definitions of the expression element types
    STRUCTOP_STRUCT and STRUCTOP_PTR.
 
-   If NOERR is nonzero, return zero if NAME is not suitably defined.
-   If NAME is the name of a baseclass type, return that type.  */
+   If NOERR is nonzero, return NULL if there is no component named
+   NAME.  */
 
 struct type *
 lookup_struct_elt_type (struct type *type, const char *name, int noerr)
@@ -1678,20 +1677,6 @@ lookup_struct_elt_type (struct type *type, const char *name, int noerr)
 	     type_name.c_str ());
     }
 
-#if 0
-  /* FIXME: This change put in by Michael seems incorrect for the case
-     where the structure tag name is the same as the member name.
-     I.e. when doing "ptype bell->bar" for "struct foo { int bar; int
-     foo; } bell;" Disabled by fnf.  */
-  {
-    char *type_name;
-
-    type_name = TYPE_NAME (type);
-    if (type_name != NULL && strcmp (type_name, name) == 0)
-      return type;
-  }
-#endif
-
   for (i = TYPE_NFIELDS (type) - 1; i >= TYPE_N_BASECLASSES (type); i--)
     {
       const char *t_field_name = TYPE_FIELD_NAME (type, i);
-- 
2.19.2

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

* [PATCH v2 04/11] Add a new gdbarch method to resolve the address of TLS variables.
  2019-02-09  0:42 [PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
                   ` (8 preceding siblings ...)
  2019-02-09  0:50 ` [PATCH v2 10/11] Support TLS variables on FreeBSD/riscv John Baldwin
@ 2019-02-09  0:50 ` John Baldwin
  2019-03-07 16:08   ` Simon Marchi
  2019-02-09  0:50 ` [PATCH v2 05/11] Remove code disabled since at least 1999 from lookup_struct_elt_type John Baldwin
  2019-02-22 17:22 ` [PING][PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
  11 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-02-09  0:50 UTC (permalink / raw)
  To: gdb-patches

Permit TLS variable addresses to be resolved purely by an ABI rather
than requiring a target method.  This doesn't try the target method if
the ABI function is present (even if the ABI function fails) to
simplify error handling.

gdb/ChangeLog:

	* gdbarch.sh (get_thread_local_address): New method.
	* gdbarch.h, gdbarch.c: Regenerate.
	* target.c (target_translate_tls_address): Use
	gdbarch_get_thread_local_address if present instead of
	target::get_thread_local_address.
---
 gdb/ChangeLog  |  8 ++++++++
 gdb/gdbarch.c  | 32 ++++++++++++++++++++++++++++++++
 gdb/gdbarch.h  |  6 ++++++
 gdb/gdbarch.sh |  1 +
 gdb/target.c   | 13 ++++++++-----
 5 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7f4e912ad3..621b9af800 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2019-02-08  John Baldwin  <jhb@FreeBSD.org>
+
+	* gdbarch.sh (get_thread_local_address): New method.
+	* gdbarch.h, gdbarch.c: Regenerate.
+	* target.c (target_translate_tls_address): Use
+	gdbarch_get_thread_local_address if present instead of
+	target::get_thread_local_address.
+
 2019-02-08  John Baldwin  <jhb@FreeBSD.org>
 
 	* solib-svr4.c (svr4_fetch_objfile_link_map): Look for
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 434ee3bfcf..2b3fcef004 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -251,6 +251,7 @@ struct gdbarch
   CORE_ADDR deprecated_function_start_offset;
   gdbarch_remote_register_number_ftype *remote_register_number;
   gdbarch_fetch_tls_load_module_address_ftype *fetch_tls_load_module_address;
+  gdbarch_get_thread_local_address_ftype *get_thread_local_address;
   CORE_ADDR frame_args_skip;
   gdbarch_unwind_pc_ftype *unwind_pc;
   gdbarch_unwind_sp_ftype *unwind_sp;
@@ -613,6 +614,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of deprecated_function_start_offset, invalid_p == 0 */
   /* Skip verify of remote_register_number, invalid_p == 0 */
   /* Skip verify of fetch_tls_load_module_address, has predicate.  */
+  /* Skip verify of get_thread_local_address, has predicate.  */
   /* Skip verify of frame_args_skip, invalid_p == 0 */
   /* Skip verify of unwind_pc, invalid_p == 0 */
   /* Skip verify of unwind_sp, invalid_p == 0 */
@@ -1073,6 +1075,12 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   fprintf_unfiltered (file,
                       "gdbarch_dump: get_syscall_number = <%s>\n",
                       host_address_to_string (gdbarch->get_syscall_number));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: gdbarch_get_thread_local_address_p() = %d\n",
+                      gdbarch_get_thread_local_address_p (gdbarch));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: get_thread_local_address = <%s>\n",
+                      host_address_to_string (gdbarch->get_thread_local_address));
   fprintf_unfiltered (file,
                       "gdbarch_dump: gnu_triplet_regexp = <%s>\n",
                       host_address_to_string (gdbarch->gnu_triplet_regexp));
@@ -3018,6 +3026,30 @@ set_gdbarch_fetch_tls_load_module_address (struct gdbarch *gdbarch,
   gdbarch->fetch_tls_load_module_address = fetch_tls_load_module_address;
 }
 
+int
+gdbarch_get_thread_local_address_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->get_thread_local_address != NULL;
+}
+
+CORE_ADDR
+gdbarch_get_thread_local_address (struct gdbarch *gdbarch, ptid_t ptid, CORE_ADDR lm_addr, CORE_ADDR offset)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->get_thread_local_address != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_get_thread_local_address called\n");
+  return gdbarch->get_thread_local_address (gdbarch, ptid, lm_addr, offset);
+}
+
+void
+set_gdbarch_get_thread_local_address (struct gdbarch *gdbarch,
+                                      gdbarch_get_thread_local_address_ftype get_thread_local_address)
+{
+  gdbarch->get_thread_local_address = get_thread_local_address;
+}
+
 CORE_ADDR
 gdbarch_frame_args_skip (struct gdbarch *gdbarch)
 {
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 5b265a462a..e11144941f 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -649,6 +649,12 @@ typedef CORE_ADDR (gdbarch_fetch_tls_load_module_address_ftype) (struct objfile
 extern CORE_ADDR gdbarch_fetch_tls_load_module_address (struct gdbarch *gdbarch, struct objfile *objfile);
 extern void set_gdbarch_fetch_tls_load_module_address (struct gdbarch *gdbarch, gdbarch_fetch_tls_load_module_address_ftype *fetch_tls_load_module_address);
 
+extern int gdbarch_get_thread_local_address_p (struct gdbarch *gdbarch);
+
+typedef CORE_ADDR (gdbarch_get_thread_local_address_ftype) (struct gdbarch *gdbarch, ptid_t ptid, CORE_ADDR lm_addr, CORE_ADDR offset);
+extern CORE_ADDR gdbarch_get_thread_local_address (struct gdbarch *gdbarch, ptid_t ptid, CORE_ADDR lm_addr, CORE_ADDR offset);
+extern void set_gdbarch_get_thread_local_address (struct gdbarch *gdbarch, gdbarch_get_thread_local_address_ftype *get_thread_local_address);
+
 extern CORE_ADDR gdbarch_frame_args_skip (struct gdbarch *gdbarch);
 extern void set_gdbarch_frame_args_skip (struct gdbarch *gdbarch, CORE_ADDR frame_args_skip);
 
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index afc4da7cdd..09097bcbaf 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -602,6 +602,7 @@ m;int;remote_register_number;int regno;regno;;default_remote_register_number;;0
 
 # Fetch the target specific address used to represent a load module.
 F;CORE_ADDR;fetch_tls_load_module_address;struct objfile *objfile;objfile
+M;CORE_ADDR;get_thread_local_address;ptid_t ptid, CORE_ADDR lm_addr, CORE_ADDR offset;ptid, lm_addr, offset
 #
 v;CORE_ADDR;frame_args_skip;;;0;;;0
 m;CORE_ADDR;unwind_pc;struct frame_info *next_frame;next_frame;;default_unwind_pc;;0
diff --git a/gdb/target.c b/gdb/target.c
index c1ab07f760..9864103826 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -691,8 +691,9 @@ target_translate_tls_address (struct objfile *objfile, CORE_ADDR offset)
 {
   volatile CORE_ADDR addr = 0;
   struct target_ops *target = current_top_target ();
+  struct gdbarch *gdbarch = target_gdbarch ();
 
-  if (gdbarch_fetch_tls_load_module_address_p (target_gdbarch ()))
+  if (gdbarch_fetch_tls_load_module_address_p (gdbarch))
     {
       ptid_t ptid = inferior_ptid;
 
@@ -701,10 +702,14 @@ target_translate_tls_address (struct objfile *objfile, CORE_ADDR offset)
 	  CORE_ADDR lm_addr;
 	  
 	  /* Fetch the load module address for this objfile.  */
-	  lm_addr = gdbarch_fetch_tls_load_module_address (target_gdbarch (),
+	  lm_addr = gdbarch_fetch_tls_load_module_address (gdbarch,
 	                                                   objfile);
 
-	  addr = target->get_thread_local_address (ptid, lm_addr, offset);
+	  if (gdbarch_get_thread_local_address_p (gdbarch))
+	    addr = gdbarch_get_thread_local_address (gdbarch, ptid, lm_addr,
+						     offset);
+	  else
+	    addr = target->get_thread_local_address (ptid, lm_addr, offset);
 	}
       /* If an error occurred, print TLS related messages here.  Otherwise,
          throw the error to some higher catcher.  */
@@ -759,8 +764,6 @@ target_translate_tls_address (struct objfile *objfile, CORE_ADDR offset)
 	}
       END_CATCH
     }
-  /* It wouldn't be wrong here to try a gdbarch method, too; finding
-     TLS is an ABI-specific thing.  But we don't do that yet.  */
   else
     error (_("Cannot find thread-local variables on this target"));
 
-- 
2.19.2

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

* Re: [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type.
  2019-02-09  0:42 ` [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type John Baldwin
@ 2019-02-09  1:08   ` John Baldwin
  2019-02-11 10:27     ` Philipp Rudo
  2019-03-07 15:53   ` Simon Marchi
  1 sibling, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-02-09  1:08 UTC (permalink / raw)
  To: Omair Javaid, Philipp Rudo; +Cc: gdb-patches

On 2/8/19 4:40 PM, John Baldwin wrote:
> lookup_struct_elt is a new function which returns a tuple of
> information about a component of a structure or union.  The returned
> tuple contains a pointer to the struct field object for the component
> as well as a bit offset of that field within the structure.  If the
> field names a field in an anonymous substructure, the offset is the
> "global" offset relative to the original structure type.  If noerr is
> set, then the returned tuple will set the field pointer to NULL to
> indicate a missing component rather than throwing an error.
> 
> lookup_struct_elt_type is now reimplemented in terms of this new
> function.  It simply returns the type of the returned field.

Hopefully this is close enough to lk_find_field that you can reuse it.
One difference is that it defines its own dedicated type and the second is
that it returns the raw bitpos so that it is hopefully easier to reuse in
other places.  I think you can probably call it and just pass the members
the returned structure (with an added divide for the offset to convert to
bytes) to construct an lk_symbol.

-- 
John Baldwin

                                                                            

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

* Re: [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type.
  2019-02-09  1:08   ` John Baldwin
@ 2019-02-11 10:27     ` Philipp Rudo
  2019-02-11 17:44       ` John Baldwin
  0 siblings, 1 reply; 28+ messages in thread
From: Philipp Rudo @ 2019-02-11 10:27 UTC (permalink / raw)
  To: John Baldwin; +Cc: Omair Javaid, gdb-patches

Hey Jon

On Fri, 8 Feb 2019 17:07:22 -0800
John Baldwin <jhb@FreeBSD.org> wrote:

> On 2/8/19 4:40 PM, John Baldwin wrote:
> > lookup_struct_elt is a new function which returns a tuple of
> > information about a component of a structure or union.  The returned
> > tuple contains a pointer to the struct field object for the component
> > as well as a bit offset of that field within the structure.  If the
> > field names a field in an anonymous substructure, the offset is the
> > "global" offset relative to the original structure type.  If noerr is
> > set, then the returned tuple will set the field pointer to NULL to
> > indicate a missing component rather than throwing an error.
> > 
> > lookup_struct_elt_type is now reimplemented in terms of this new
> > function.  It simply returns the type of the returned field.  
> 
> Hopefully this is close enough to lk_find_field that you can reuse it.
> One difference is that it defines its own dedicated type and the second is
> that it returns the raw bitpos so that it is hopefully easier to reuse in
> other places.  I think you can probably call it and just pass the members
> the returned structure (with an added divide for the offset to convert to
> bytes) to construct an lk_symbol.

sorry, I totally missed your v1.

The patch looks sane to me. It should be possible to use it in lk_find_field.
I'm not fully sure what the 'check on baseclasses' does for C structs, but I
guess it doesn't harm. Otherwise there would have already been an outcry :)

Thanks
Philipp

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

* Re: [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type.
  2019-02-11 10:27     ` Philipp Rudo
@ 2019-02-11 17:44       ` John Baldwin
  0 siblings, 0 replies; 28+ messages in thread
From: John Baldwin @ 2019-02-11 17:44 UTC (permalink / raw)
  To: Philipp Rudo; +Cc: Omair Javaid, gdb-patches

On 2/11/19 2:27 AM, Philipp Rudo wrote:
> Hey Jon
> 
> On Fri, 8 Feb 2019 17:07:22 -0800
> John Baldwin <jhb@FreeBSD.org> wrote:
> 
>> On 2/8/19 4:40 PM, John Baldwin wrote:
>>> lookup_struct_elt is a new function which returns a tuple of
>>> information about a component of a structure or union.  The returned
>>> tuple contains a pointer to the struct field object for the component
>>> as well as a bit offset of that field within the structure.  If the
>>> field names a field in an anonymous substructure, the offset is the
>>> "global" offset relative to the original structure type.  If noerr is
>>> set, then the returned tuple will set the field pointer to NULL to
>>> indicate a missing component rather than throwing an error.
>>>
>>> lookup_struct_elt_type is now reimplemented in terms of this new
>>> function.  It simply returns the type of the returned field.  
>>
>> Hopefully this is close enough to lk_find_field that you can reuse it.
>> One difference is that it defines its own dedicated type and the second is
>> that it returns the raw bitpos so that it is hopefully easier to reuse in
>> other places.  I think you can probably call it and just pass the members
>> the returned structure (with an added divide for the offset to convert to
>> bytes) to construct an lk_symbol.
> 
> sorry, I totally missed your v1.

Oh, this patch wasn't in the v1 is why.  I was using parse_and_eval_long
with hand-coded offsetof equivalents previously.

> The patch looks sane to me. It should be possible to use it in lk_find_field.
> I'm not fully sure what the 'check on baseclasses' does for C structs, but I
> guess it doesn't harm. Otherwise there would have already been an outcry :)

Ok.

-- 
John Baldwin

                                                                            

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

* Re: [PING][PATCH v2 00/11] Support for thread-local variables on FreeBSD
  2019-02-09  0:42 [PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
                   ` (10 preceding siblings ...)
  2019-02-09  0:50 ` [PATCH v2 05/11] Remove code disabled since at least 1999 from lookup_struct_elt_type John Baldwin
@ 2019-02-22 17:22 ` John Baldwin
  2019-03-12 20:21   ` Simon Marchi
  11 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-02-22 17:22 UTC (permalink / raw)
  To: gdb-patches

On 2/8/19 4:40 PM, John Baldwin wrote:
> Relative to the first version the changes are:
> 
> - Patch 2 fixes some #ifdef's pointed by Simon in amd64-bsd-nat.c.
> - Patch 3's description has been reworked to narrow down the cases in
>   which it applies.
> - Patch 5 is a new cleanup patch
> - Patch 6 is a new patch that introduces a new method to lookup the
>   offset of a field in a structure.  It is slightly more general than
>   just returning offsets so that it can hopefully be used to replace
>   the lk_find_field function in the Linux kernel patchset.
> - Patch 7 (formerly 5) now uses the new method from Patch 6 to lookup
>   the offsets of two fields in a runtime linker structure instead of
>   parse_and_eval_long with manual offsetof.
> 
> John Baldwin (11):
>   Support the fs_base and gs_base registers on i386.
>   Support fs_base and gs_base on FreeBSD/i386.
>   Handle an edge case for minisym TLS variable lookups.
>   Add a new gdbarch method to resolve the address of TLS variables.
>   Remove code disabled since at least 1999 from lookup_struct_elt_type.
>   Add a more general version of lookup_struct_elt_type.
>   Add a helper function to resolve TLS variable addresses for FreeBSD.
>   Support TLS variables on FreeBSD/amd64.
>   Support TLS variables on FreeBSD/i386.
>   Support TLS variables on FreeBSD/riscv.
>   Support TLS variables on FreeBSD/powerpc.

I'll probably wait until the 8.3 branch if this is OK'd.  Simon looked at
the first 3 patches previously (though 3 has a new description since the
first series).  Andrew ok'd the FreeBSD/riscv patch as well.  The
FreeBSD-specific ones are probably ok, but patches 4-6 probably could use
at least some looking over.

-- 
John Baldwin

                                                                            

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

* Re: [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type.
  2019-02-09  0:42 ` [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type John Baldwin
  2019-02-09  1:08   ` John Baldwin
@ 2019-03-07 15:53   ` Simon Marchi
  2019-03-08  0:04     ` John Baldwin
  1 sibling, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2019-03-07 15:53 UTC (permalink / raw)
  To: John Baldwin, gdb-patches

On 2019-02-08 7:40 p.m., John Baldwin wrote:
> lookup_struct_elt is a new function which returns a tuple of
> information about a component of a structure or union.  The returned
> tuple contains a pointer to the struct field object for the component
> as well as a bit offset of that field within the structure.  If the
> field names a field in an anonymous substructure, the offset is the
> "global" offset relative to the original structure type.

Can you add this bit of detail (about anonymous struct) to the function doc?

> If noerr is
> set, then the returned tuple will set the field pointer to NULL to
> indicate a missing component rather than throwing an error.
> 
> lookup_struct_elt_type is now reimplemented in terms of this new
> function.  It simply returns the type of the returned field.
> 
> gdb/ChangeLog:
> 
> 	* gdbtypes.c (lookup_struct_elt): New function.
> 	(lookup_struct_elt_type): Reimplement via lookup_struct_elt.
> 	* gdbtypes.h (struct struct_elt): New type.
> 	(lookup_struct_elt): New prototype.
> ---
>   gdb/ChangeLog  |  7 ++++++
>   gdb/gdbtypes.c | 60 ++++++++++++++++++++++++++++++++------------------
>   gdb/gdbtypes.h | 19 ++++++++++++++++
>   3 files changed, 65 insertions(+), 21 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index cebd63bcb5..c7fee7eb11 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,10 @@
> +2019-02-08  John Baldwin  <jhb@FreeBSD.org>
> +
> +	* gdbtypes.c (lookup_struct_elt): New function.
> +	(lookup_struct_elt_type): Reimplement via lookup_struct_elt.
> +	* gdbtypes.h (struct struct_elt): New type.
> +	(lookup_struct_elt): New prototype.
> +
>   2019-02-08  John Baldwin  <jhb@FreeBSD.org>
>   
>   	* gdbtypes.c (lookup_struct_elt_type): Update comment and
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index e4acb0e985..0f3a450f9f 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -1644,7 +1644,8 @@ lookup_template_type (char *name, struct type *type,
>     return (SYMBOL_TYPE (sym));
>   }
>   
> -/* Given a type TYPE, lookup the type of the component named NAME.
> +/* Given a type TYPE, lookup the field and offset of the component named
> +   NAME.
>   
>      TYPE can be either a struct or union, or a pointer or reference to
>      a struct or union.  If it is a pointer or reference, its target
> @@ -1652,11 +1653,11 @@ lookup_template_type (char *name, struct type *type,
>      as specified for the definitions of the expression element types
>      STRUCTOP_STRUCT and STRUCTOP_PTR.
>   
> -   If NOERR is nonzero, return NULL if there is no component named
> -   NAME.  */
> +   If NOERR is nonzero, the returned structure will have field set to
> +   NULL if there is no component named NAME.  */

For both functions, please move the comment to the .h, and add /* See 
gdbtypes.h.  */ here.

>   
> -struct type *
> -lookup_struct_elt_type (struct type *type, const char *name, int noerr)
> +struct_elt
> +lookup_struct_elt (struct type *type, const char *name, int noerr) >   {
>     int i;
>   
> @@ -1683,39 +1684,56 @@ lookup_struct_elt_type (struct type *type, const char *name, int noerr)
>   
>         if (t_field_name && (strcmp_iw (t_field_name, name) == 0))
>   	{
> -	  return TYPE_FIELD_TYPE (type, i);
> +	  return struct_elt (&TYPE_FIELD(type, i), TYPE_FIELD_BITPOS (type, i));
>   	}
>        else if (!t_field_name || *t_field_name == '\0')
>   	{
> -	  struct type *subtype
> -	    = lookup_struct_elt_type (TYPE_FIELD_TYPE (type, i), name, 1);
> -
> -	  if (subtype != NULL)
> -	    return subtype;
> +	  struct_elt elt
> +	    = lookup_struct_elt (TYPE_FIELD_TYPE (type, i), name, 1);
> +	  if (elt.field != NULL)
> +	    {
> +	      elt.offset += TYPE_FIELD_BITPOS (type, i);
> +	      return elt;
> +	    }
>   	}
>       }
>   
>     /* OK, it's not in this class.  Recursively check the baseclasses.  */
>     for (i = TYPE_N_BASECLASSES (type) - 1; i >= 0; i--)
>       {
> -      struct type *t;
> -
> -      t = lookup_struct_elt_type (TYPE_BASECLASS (type, i), name, 1);
> -      if (t != NULL)
> -	{
> -	  return t;
> -	}
> +      struct_elt elt = lookup_struct_elt (TYPE_BASECLASS (type, i), name, 1);
> +      if (elt.field != NULL)
> +	return elt;
>       }
>   
>     if (noerr)
> -    {
> -      return NULL;
> -    }
> +      return struct_elt ();
>   
>     std::string type_name = type_to_string (type);
>     error (_("Type %s has no component named %s."), type_name.c_str (), name);
>   }
>   
> +/* Given a type TYPE, lookup the type of the component named NAME.
> +
> +   TYPE can be either a struct or union, or a pointer or reference to
> +   a struct or union.  If it is a pointer or reference, its target
> +   type is automatically used.  Thus '.' and '->' are interchangable,
> +   as specified for the definitions of the expression element types
> +   STRUCTOP_STRUCT and STRUCTOP_PTR.
> +
> +   If NOERR is nonzero, return NULL if there is no component named
> +   NAME.  */
> +
> +struct type *
> +lookup_struct_elt_type (struct type *type, const char *name, int noerr)
> +{
> +  struct_elt elt = lookup_struct_elt (type, name, noerr);
> +  if (elt.field != NULL)
> +    return FIELD_TYPE (*elt.field);
> +  else
> +    return NULL;
> +}
> +
>   /* Store in *MAX the largest number representable by unsigned integer type
>      TYPE.  */
>   
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index a6d4f64e9b..894c7b2fc6 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -1873,6 +1873,25 @@ extern struct type *allocate_stub_method (struct type *);
>   
>   extern const char *type_name_or_error (struct type *type);
>   
> +struct struct_elt
> +{
> +  /* The field of the element, or NULL if no element was found.  */
> +  struct field *field;
> +
> +  /* The bit offset of the element in the parent structure.  */
> +  LONGEST offset;
> +
> +  struct_elt ()
> +    : field (nullptr), offset (0)
> +  {}
> +
> +  struct_elt (struct field *field, LONGEST offset)
> +    : field (field), offset (offset)
> +  {}
> +};

Not really a big deal, but I find it a bit overkill to define a type 
just for this, I would have probably just returned the the offset as an 
output parameter.  But maybe this way is better, in that if we want to 
add something to the return type, we don't have to update the callers.

So the patch LGTM with the nits noted above.

Simon

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

* Re: [PATCH v2 04/11] Add a new gdbarch method to resolve the address of TLS variables.
  2019-02-09  0:50 ` [PATCH v2 04/11] Add a new gdbarch method to resolve the address of TLS variables John Baldwin
@ 2019-03-07 16:08   ` Simon Marchi
  2019-03-07 23:50     ` John Baldwin
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2019-03-07 16:08 UTC (permalink / raw)
  To: John Baldwin, gdb-patches

On 2019-02-08 7:40 p.m., John Baldwin wrote:
> Permit TLS variable addresses to be resolved purely by an ABI rather
> than requiring a target method.  This doesn't try the target method if
> the ABI function is present (even if the ABI function fails) to
> simplify error handling.

I don't see anything wrong with the patch (and the comment you removed 
in target_translate_tls_address hints it is right), but again I am not 
very familiar with how TLS works, so I wouldn't spot if anything was 
conceptually wrong with this approach.  I would appreciate if another 
maintainer could take a look and give their opinion.

> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index afc4da7cdd..09097bcbaf 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -602,6 +602,7 @@ m;int;remote_register_number;int regno;regno;;default_remote_register_number;;0
>   
>   # Fetch the target specific address used to represent a load module.
>   F;CORE_ADDR;fetch_tls_load_module_address;struct objfile *objfile;objfile
> +M;CORE_ADDR;get_thread_local_address;ptid_t ptid, CORE_ADDR lm_addr, CORE_ADDR offset;ptid, lm_addr, offset

Could you document the method, especially the meaning of the parameters?

Simon

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

* Re: [PATCH v2 07/11] Add a helper function to resolve TLS variable addresses for FreeBSD.
  2019-02-09  0:42 ` [PATCH v2 07/11] Add a helper function to resolve TLS variable addresses for FreeBSD John Baldwin
@ 2019-03-07 16:18   ` Simon Marchi
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Marchi @ 2019-03-07 16:18 UTC (permalink / raw)
  To: John Baldwin, gdb-patches

On 2019-02-08 7:40 p.m., John Baldwin wrote:
> The fbsd_get_thread_local_address function accepts the base address of
> a thread's DTV array and the base address of an object file's link map
> and uses this to compute a TLS variable's address.  FreeBSD
> architectures use an architecture-specific method to determine the
> address of the DTV array pointer and call this helper function to
> perform the rest of the address calculation.

I didn't check against FreeBSD's data structures (I trust you for that 
:)), but in general this LGTM.  Just some formatting nits.

> +/* Per-program-space data for FreeBSD architectures.  */
> +static const struct program_space_data *fbsd_pspace_data_handle;
> +
> +struct fbsd_pspace_data
> +  {
> +    /* Offsets in the runtime linker's 'Obj_Entry' structure.  */
> +    LONGEST off_linkmap;
> +    LONGEST off_tlsindex;
> +    bool rtld_offsets_valid;
> +  };

Unindent the { } and what's between.

> +/* Lookup offsets of fields in the runtime linker's 'Obj_Entry'
> +   structure needed to determine the TLS index of an object file.  */
> +
> +static void
> +fbsd_fetch_rtld_offsets (struct gdbarch *gdbarch, struct fbsd_pspace_data *data)
> +{
> +  TRY
> +    {
> +      /* Fetch offsets from debug symbols in rtld.  */
> +      struct symbol *obj_entry_sym
> +	= lookup_symbol_in_language ("Struct_Obj_Entry", NULL, STRUCT_DOMAIN,
> +				     language_c, NULL).symbol;
> +      if (obj_entry_sym == NULL)
> +	error (_("Unable to find Struct_Obj_Entry symbol"));
> +      data->off_linkmap = lookup_struct_elt (SYMBOL_TYPE(obj_entry_sym),
> +					     "linkmap", 0).offset / 8;
> +      data->off_tlsindex = lookup_struct_elt (SYMBOL_TYPE(obj_entry_sym),
> +					      "tlsindex", 0).offset / 8;
> +      data->rtld_offsets_valid = true;
> +      return;
> +    }
> +  CATCH (e, RETURN_MASK_ERROR)
> +    {
> +      data->off_linkmap = -1;
> +    }
> +  END_CATCH
> +
> +  TRY
> +    {
> +      /* Fetch offsets from global variables in libthr.  Note that
> +	 this does not work for single-threaded processes that are not
> +	 linked against libthr.  */
> +      data->off_linkmap = fbsd_read_integer_by_name(gdbarch,
> +						    "_thread_off_linkmap");
> +      data->off_tlsindex = fbsd_read_integer_by_name(gdbarch,
> +						     "_thread_off_tlsindex");

Missing spaces before parentheses.

> +/* Helper function to fetch the address of a thread-local variable.
> +   DTV_ADDR is the base address of the thread's dtv array.  LM_ADDR is
> +   the address of the link_map structure for the associated object
> +   file.  */
> +
> +extern CORE_ADDR fbsd_get_thread_local_address (struct gdbarch *gdbarch,
> +						CORE_ADDR dtv_addr,
> +						CORE_ADDR lm_addr,
> +						CORE_ADDR offset);

Could you document the offset parameter?  I know it's obvious for 
somebody who already knows how TLS works, but if somebody is still a 
noob, they will appreciate the comment :).

Simon

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

* Re: [PATCH v2 05/11] Remove code disabled since at least 1999 from lookup_struct_elt_type.
  2019-02-09  0:50 ` [PATCH v2 05/11] Remove code disabled since at least 1999 from lookup_struct_elt_type John Baldwin
@ 2019-03-07 16:25   ` Simon Marchi
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Marchi @ 2019-03-07 16:25 UTC (permalink / raw)
  To: John Baldwin, gdb-patches

On 2019-02-08 7:40 p.m., John Baldwin wrote:
> Update the comment above the function to reflect the code removal and
> document the existing behavior.

LGTM.

Simon

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

* Re: [PATCH v2 11/11] Support TLS variables on FreeBSD/powerpc.
  2019-02-09  0:42 ` [PATCH v2 11/11] Support TLS variables on FreeBSD/powerpc John Baldwin
@ 2019-03-07 16:26   ` Simon Marchi
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Marchi @ 2019-03-07 16:26 UTC (permalink / raw)
  To: John Baldwin, gdb-patches

On 2019-02-08 7:40 p.m., John Baldwin wrote:
> Derive the pointer to the DTV array from the %r2 register on 32-bit
> powerpc and %r13 on 64-bit powerpc.

Hi John,

Patches 8-11 LGTM. So I think there's just patch 4/11 that remains, 
which I have asked for somebody else to take a look.

Simon

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

* Re: [PATCH v2 04/11] Add a new gdbarch method to resolve the address of TLS variables.
  2019-03-07 16:08   ` Simon Marchi
@ 2019-03-07 23:50     ` John Baldwin
  2019-03-08  2:55       ` Simon Marchi
  0 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-03-07 23:50 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 3/7/19 8:08 AM, Simon Marchi wrote:
> On 2019-02-08 7:40 p.m., John Baldwin wrote:
>> Permit TLS variable addresses to be resolved purely by an ABI rather
>> than requiring a target method.  This doesn't try the target method if
>> the ABI function is present (even if the ABI function fails) to
>> simplify error handling.
> 
> I don't see anything wrong with the patch (and the comment you removed 
> in target_translate_tls_address hints it is right), but again I am not 
> very familiar with how TLS works, so I wouldn't spot if anything was 
> conceptually wrong with this approach.  I would appreciate if another 
> maintainer could take a look and give their opinion.

Ok.  FWIW, the reason for target vs gdbarch has to do with the different ways one can
resolve a TLS variable.  Some background:

In ELF relocations, a TLS variable is identified by an offset in a special TLS section
of an ELF file, similar to global symbols being an offset relative to .data or .bss.
However, TLS variables are duplicated for each thread.  To support this, the runtime
linker allocates an array of pointers for each thread called the DTV array.  The runtime
linker also assigns an array index to each ELF object, so the executable is assigned array
index 1, and other shared libraries that use TLS are assigned indices as they are mapped
by the runtime linker.  The pointers in each thread's array point to the per-thread blocks
of TLS variables for a given ELF object.  Thus, if index 1 is for my program and index 2
was assigned to libc, then DTV[1] contains a pointer to all of the TLS variables in my
main program and DTV[2] contains a pointer to all of the TLS variables in libc.

Thus, if libc has two TLS integers 'foo' and 'bar', they might be assigned offsets of
0 and 4.  To read the value of 'foo' one uses the expression '*(int *)(DTV[1])'.  To
read 'bar' you would use '*(int *)(DTV[1] + 4)'.

There are some extra optimizations in the compiler-generated code (there's something
called static TLS that can be at a fixed offset from the per-thread TCB pointer IIRC,
but there are also valid DTV[] pointers that can get to the same variables just via
more indirection.  Compiled code is also allowed to fetch the 'base' of a TLS block
for a given shared object and then save that 'base' and use offsets from it to access
different variables.  Put another way, the compiler can assume that &bar - &foo is
always '4' and just add the relative offset to '&foo' to compute '&bar' without going
through the DTV array every time).

In target_translate_tls_address() we are given the 'struct objfile' of the ELF object
and the offset of the TLS variable we are trying to find.  The
gdbarch_fetch_tls_load_module_address function fetches the pointer to the runtime
linker's data structure describing that ELF object.

The target version (target::get_thread_local_address) expects to use some target
specific method to turn a (thread, linker_module_addr, offset) tuple into the
address of a TLS variable.  On Linux and other systems using libthread_db for this,
it calls a libthread_db function.  Internally that libthread_db function looks at
the runtime linker's structure to extract the TLS index of the ELF object.  It
then looks in the thread library's per-thread data structure to find a pointer
to the DTV array.  It then uses 'DTV[index] + offset' to compute the final address.
Note that this is all done in libthread_db rather than in gdb itself.

The gdbarch method I'm using for FreeBSD doesn't use libthread_db.  Instead, it
more closely mimics what the compiler-generated code does.  Many architectures use
some sort of register to point to a per-thread Thread Control Block (TCB), and they
store a pointer to the DTV array either in the TCB or at a fixed offset relative to
the TCB.  For example, 64-bit x86 uses the %fs segment prefix to access the TCB,
and the %fs_base register is thus a pointer to the TCB.  32-bit x86 uses %gs and
%gs_base instead.  RISC-V has a 'tp' register for this purpose, etc.  The approach
I'm using for FreeBSD is to provide an architecture-specific function that uses the
relevant register to locate the pointer to the DTV array.  It then calls a shared
function (patch 7) that extracts the TLS index from the runtime linker's data
structure and computes the final address via 'DTV[index] + offset'.

Mostly I did this because I don't like libthread_db, but using a gdbarch method
should also be a bit more cross-debugger friendly (you don't have to have a libthread_db
on a FreeBSD host that understands the Linux runtime linker or thread library or
vice versa, and similar concerns with 32-bit vs 64-bit and x86 vs ARM, etc.).

>> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
>> index afc4da7cdd..09097bcbaf 100755
>> --- a/gdb/gdbarch.sh
>> +++ b/gdb/gdbarch.sh
>> @@ -602,6 +602,7 @@ m;int;remote_register_number;int regno;regno;;default_remote_register_number;;0
>>   
>>   # Fetch the target specific address used to represent a load module.
>>   F;CORE_ADDR;fetch_tls_load_module_address;struct objfile *objfile;objfile
>> +M;CORE_ADDR;get_thread_local_address;ptid_t ptid, CORE_ADDR lm_addr, CORE_ADDR offset;ptid, lm_addr, offset
> 
> Could you document the method, especially the meaning of the parameters?

Sure.  I used a variant of the comment from the target method:

diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 48fcebd19a..d15b6aa794 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -602,6 +602,14 @@ m;int;remote_register_number;int regno;regno;;default_remote_register_number;;0
 
 # Fetch the target specific address used to represent a load module.
 F;CORE_ADDR;fetch_tls_load_module_address;struct objfile *objfile;objfile
+
+# Return the thread-local address at OFFSET in the thread-local
+# storage for the thread PTID and the shared library or executable
+# file given by LM_ADDR.  If that block of thread-local storage hasn't
+# been allocated yet, this function may return an error.  LM_ADDR may
+# be zero for statically linked multithreaded inferiors.
+
+M;CORE_ADDR;get_thread_local_address;ptid_t ptid, CORE_ADDR lm_addr, CORE_ADDR offset;ptid, lm_addr, offset
 #
 v;CORE_ADDR;frame_args_skip;;;0;;;0
 m;CORE_ADDR;unwind_pc;struct frame_info *next_frame;next_frame;;default_unwind_pc;;0

-- 
John Baldwin

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

* Re: [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type.
  2019-03-07 15:53   ` Simon Marchi
@ 2019-03-08  0:04     ` John Baldwin
  2019-03-08  0:32       ` Pedro Alves
  0 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-03-08  0:04 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 3/7/19 7:53 AM, Simon Marchi wrote:
> Not really a big deal, but I find it a bit overkill to define a type 
> just for this, I would have probably just returned the the offset as an 
> output parameter.  But maybe this way is better, in that if we want to 
> add something to the return type, we don't have to update the callers.

Honestly, I just modeled this after the similar code in the Linux
kernel thread patches.  Using a reference parameter for the offset
would be fine and I don't mind making that change.  I don't think that
we are going to add more things to that structure in the future.  The
field generally has everything else you want to know other than the
offset.

-- 
John Baldwin

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

* Re: [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type.
  2019-03-08  0:04     ` John Baldwin
@ 2019-03-08  0:32       ` Pedro Alves
  2019-03-08 18:39         ` John Baldwin
  0 siblings, 1 reply; 28+ messages in thread
From: Pedro Alves @ 2019-03-08  0:32 UTC (permalink / raw)
  To: John Baldwin, Simon Marchi, gdb-patches

On 03/08/2019 12:04 AM, John Baldwin wrote:
> On 3/7/19 7:53 AM, Simon Marchi wrote:
>> Not really a big deal, but I find it a bit overkill to define a type 
>> just for this, I would have probably just returned the the offset as an 
>> output parameter.  But maybe this way is better, in that if we want to 
>> add something to the return type, we don't have to update the callers.
> 
> Honestly, I just modeled this after the similar code in the Linux
> kernel thread patches.  Using a reference parameter for the offset
> would be fine and I don't mind making that change.  I don't think that
> we are going to add more things to that structure in the future.  The
> field generally has everything else you want to know other than the
> offset.
> 

Note that you can keep the struct with just the data fields
and no ctor boilerplate if you use brace initialization.

I.e., with just:

struct struct_elt
{
  /* The field of the element, or NULL if no element was found.  */
  struct field *field;

  /* The bit offset of the element in the parent structure.  */
  LONGEST offset;
};

You write:

 return {nullptr, 0};
 return {&TYPE_FIELD (type, i), TYPE_FIELD_BITPOS (type, i)};

Instead of:

 return struct_elt ();
 return struct_elt (&TYPE_FIELD (type, i), TYPE_FIELD_BITPOS (type, i));




BTW, I noticed the missing space before parens below:

 > -	  return TYPE_FIELD_TYPE (type, i);
 > +	  return struct_elt (&TYPE_FIELD(type, i), TYPE_FIELD_BITPOS (type, i));

Thanks,
Pedro Alves

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

* Re: [PATCH v2 04/11] Add a new gdbarch method to resolve the address of TLS variables.
  2019-03-07 23:50     ` John Baldwin
@ 2019-03-08  2:55       ` Simon Marchi
  2019-03-08 18:39         ` John Baldwin
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2019-03-08  2:55 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

On 2019-03-07 18:50, John Baldwin wrote:
> On 3/7/19 8:08 AM, Simon Marchi wrote:
>> On 2019-02-08 7:40 p.m., John Baldwin wrote:
>>> Permit TLS variable addresses to be resolved purely by an ABI rather
>>> than requiring a target method.  This doesn't try the target method 
>>> if
>>> the ABI function is present (even if the ABI function fails) to
>>> simplify error handling.
>> 
>> I don't see anything wrong with the patch (and the comment you removed
>> in target_translate_tls_address hints it is right), but again I am not
>> very familiar with how TLS works, so I wouldn't spot if anything was
>> conceptually wrong with this approach.  I would appreciate if another
>> maintainer could take a look and give their opinion.
> 
> Ok.  FWIW, the reason for target vs gdbarch has to do with the
> different ways one can
> resolve a TLS variable.  Some background:
> 
> In ELF relocations, a TLS variable is identified by an offset in a
> special TLS section
> of an ELF file, similar to global symbols being an offset relative to
> .data or .bss.
> However, TLS variables are duplicated for each thread.  To support
> this, the runtime
> linker allocates an array of pointers for each thread called the DTV
> array.  The runtime
> linker also assigns an array index to each ELF object, so the
> executable is assigned array
> index 1, and other shared libraries that use TLS are assigned indices
> as they are mapped
> by the runtime linker.  The pointers in each thread's array point to
> the per-thread blocks
> of TLS variables for a given ELF object.  Thus, if index 1 is for my
> program and index 2
> was assigned to libc, then DTV[1] contains a pointer to all of the TLS
> variables in my
> main program and DTV[2] contains a pointer to all of the TLS variables 
> in libc.
> 
> Thus, if libc has two TLS integers 'foo' and 'bar', they might be
> assigned offsets of
> 0 and 4.  To read the value of 'foo' one uses the expression '*(int
> *)(DTV[1])'.  To
> read 'bar' you would use '*(int *)(DTV[1] + 4)'.
> 
> There are some extra optimizations in the compiler-generated code
> (there's something
> called static TLS that can be at a fixed offset from the per-thread
> TCB pointer IIRC,
> but there are also valid DTV[] pointers that can get to the same
> variables just via
> more indirection.  Compiled code is also allowed to fetch the 'base'
> of a TLS block
> for a given shared object and then save that 'base' and use offsets
> from it to access
> different variables.  Put another way, the compiler can assume that
> &bar - &foo is
> always '4' and just add the relative offset to '&foo' to compute
> '&bar' without going
> through the DTV array every time).
> 
> In target_translate_tls_address() we are given the 'struct objfile' of
> the ELF object
> and the offset of the TLS variable we are trying to find.  The
> gdbarch_fetch_tls_load_module_address function fetches the pointer to
> the runtime
> linker's data structure describing that ELF object.
> 
> The target version (target::get_thread_local_address) expects to use 
> some target
> specific method to turn a (thread, linker_module_addr, offset) tuple 
> into the
> address of a TLS variable.  On Linux and other systems using
> libthread_db for this,
> it calls a libthread_db function.  Internally that libthread_db
> function looks at
> the runtime linker's structure to extract the TLS index of the ELF 
> object.  It
> then looks in the thread library's per-thread data structure to find a 
> pointer
> to the DTV array.  It then uses 'DTV[index] + offset' to compute the
> final address.
> Note that this is all done in libthread_db rather than in gdb itself.
> 
> The gdbarch method I'm using for FreeBSD doesn't use libthread_db.  
> Instead, it
> more closely mimics what the compiler-generated code does.  Many
> architectures use
> some sort of register to point to a per-thread Thread Control Block
> (TCB), and they
> store a pointer to the DTV array either in the TCB or at a fixed
> offset relative to
> the TCB.  For example, 64-bit x86 uses the %fs segment prefix to access 
> the TCB,
> and the %fs_base register is thus a pointer to the TCB.  32-bit x86 
> uses %gs and
> %gs_base instead.  RISC-V has a 'tp' register for this purpose, etc.
> The approach
> I'm using for FreeBSD is to provide an architecture-specific function
> that uses the
> relevant register to locate the pointer to the DTV array.  It then
> calls a shared
> function (patch 7) that extracts the TLS index from the runtime 
> linker's data
> structure and computes the final address via 'DTV[index] + offset'.
> 
> Mostly I did this because I don't like libthread_db, but using a 
> gdbarch method
> should also be a bit more cross-debugger friendly (you don't have to
> have a libthread_db
> on a FreeBSD host that understands the Linux runtime linker or thread 
> library or
> vice versa, and similar concerns with 32-bit vs 64-bit and x86 vs ARM, 
> etc.).

Ok, thanks to your explanation I think I understand better the need to 
have an arch-specific way of doing it.

>>> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
>>> index afc4da7cdd..09097bcbaf 100755
>>> --- a/gdb/gdbarch.sh
>>> +++ b/gdb/gdbarch.sh
>>> @@ -602,6 +602,7 @@ m;int;remote_register_number;int 
>>> regno;regno;;default_remote_register_number;;0
>>> 
>>>   # Fetch the target specific address used to represent a load 
>>> module.
>>>   F;CORE_ADDR;fetch_tls_load_module_address;struct objfile 
>>> *objfile;objfile
>>> +M;CORE_ADDR;get_thread_local_address;ptid_t ptid, CORE_ADDR lm_addr, 
>>> CORE_ADDR offset;ptid, lm_addr, offset
>> 
>> Could you document the method, especially the meaning of the 
>> parameters?
> 
> Sure.  I used a variant of the comment from the target method:
> 
> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index 48fcebd19a..d15b6aa794 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -602,6 +602,14 @@ m;int;remote_register_number;int
> regno;regno;;default_remote_register_number;;0
> 
>  # Fetch the target specific address used to represent a load module.
>  F;CORE_ADDR;fetch_tls_load_module_address;struct objfile 
> *objfile;objfile
> +
> +# Return the thread-local address at OFFSET in the thread-local
> +# storage for the thread PTID and the shared library or executable
> +# file given by LM_ADDR.  If that block of thread-local storage hasn't
> +# been allocated yet, this function may return an error.  LM_ADDR may
> +# be zero for statically linked multithreaded inferiors.

What does "may return an error" mean?  A special CORE_ADDR value, or it 
throws an error?

Simon

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

* Re: [PATCH v2 04/11] Add a new gdbarch method to resolve the address of TLS variables.
  2019-03-08  2:55       ` Simon Marchi
@ 2019-03-08 18:39         ` John Baldwin
  0 siblings, 0 replies; 28+ messages in thread
From: John Baldwin @ 2019-03-08 18:39 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On 3/7/19 6:55 PM, Simon Marchi wrote:
> On 2019-03-07 18:50, John Baldwin wrote:
>> Sure.  I used a variant of the comment from the target method:
>>
>> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
>> index 48fcebd19a..d15b6aa794 100755
>> --- a/gdb/gdbarch.sh
>> +++ b/gdb/gdbarch.sh
>> @@ -602,6 +602,14 @@ m;int;remote_register_number;int
>> regno;regno;;default_remote_register_number;;0
>>
>>  # Fetch the target specific address used to represent a load module.
>>  F;CORE_ADDR;fetch_tls_load_module_address;struct objfile 
>> *objfile;objfile
>> +
>> +# Return the thread-local address at OFFSET in the thread-local
>> +# storage for the thread PTID and the shared library or executable
>> +# file given by LM_ADDR.  If that block of thread-local storage hasn't
>> +# been allocated yet, this function may return an error.  LM_ADDR may
>> +# be zero for statically linked multithreaded inferiors.
> 
> What does "may return an error" mean?  A special CORE_ADDR value, or it 
> throws an error?

Hmm, it throws an error, so maybe "this function may throw an error."?

I'll also add a little patch to update the comment for target::get_thread_local_address
to match.

-- 
John Baldwin

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

* Re: [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type.
  2019-03-08  0:32       ` Pedro Alves
@ 2019-03-08 18:39         ` John Baldwin
  0 siblings, 0 replies; 28+ messages in thread
From: John Baldwin @ 2019-03-08 18:39 UTC (permalink / raw)
  To: Pedro Alves, Simon Marchi, gdb-patches

On 3/7/19 4:32 PM, Pedro Alves wrote:
> On 03/08/2019 12:04 AM, John Baldwin wrote:
>> On 3/7/19 7:53 AM, Simon Marchi wrote:
>>> Not really a big deal, but I find it a bit overkill to define a type 
>>> just for this, I would have probably just returned the the offset as an 
>>> output parameter.  But maybe this way is better, in that if we want to 
>>> add something to the return type, we don't have to update the callers.
>>
>> Honestly, I just modeled this after the similar code in the Linux
>> kernel thread patches.  Using a reference parameter for the offset
>> would be fine and I don't mind making that change.  I don't think that
>> we are going to add more things to that structure in the future.  The
>> field generally has everything else you want to know other than the
>> offset.
>>
> 
> Note that you can keep the struct with just the data fields
> and no ctor boilerplate if you use brace initialization.

Thanks, I've made this change and also fixed the missing space.

-- 
John Baldwin

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

* Re: [PING][PATCH v2 00/11] Support for thread-local variables on FreeBSD
  2019-02-22 17:22 ` [PING][PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
@ 2019-03-12 20:21   ` Simon Marchi
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Marchi @ 2019-03-12 20:21 UTC (permalink / raw)
  To: John Baldwin, gdb-patches

On 2019-02-22 12:21 p.m., John Baldwin wrote:
> On 2/8/19 4:40 PM, John Baldwin wrote:
>> Relative to the first version the changes are:
>>
>> - Patch 2 fixes some #ifdef's pointed by Simon in amd64-bsd-nat.c.
>> - Patch 3's description has been reworked to narrow down the cases in
>>    which it applies.
>> - Patch 5 is a new cleanup patch
>> - Patch 6 is a new patch that introduces a new method to lookup the
>>    offset of a field in a structure.  It is slightly more general than
>>    just returning offsets so that it can hopefully be used to replace
>>    the lk_find_field function in the Linux kernel patchset.
>> - Patch 7 (formerly 5) now uses the new method from Patch 6 to lookup
>>    the offsets of two fields in a runtime linker structure instead of
>>    parse_and_eval_long with manual offsetof.
>>
>> John Baldwin (11):
>>    Support the fs_base and gs_base registers on i386.
>>    Support fs_base and gs_base on FreeBSD/i386.
>>    Handle an edge case for minisym TLS variable lookups.
>>    Add a new gdbarch method to resolve the address of TLS variables.
>>    Remove code disabled since at least 1999 from lookup_struct_elt_type.
>>    Add a more general version of lookup_struct_elt_type.
>>    Add a helper function to resolve TLS variable addresses for FreeBSD.
>>    Support TLS variables on FreeBSD/amd64.
>>    Support TLS variables on FreeBSD/i386.
>>    Support TLS variables on FreeBSD/riscv.
>>    Support TLS variables on FreeBSD/powerpc.
> 
> I'll probably wait until the 8.3 branch if this is OK'd.  Simon looked at
> the first 3 patches previously (though 3 has a new description since the
> first series).  Andrew ok'd the FreeBSD/riscv patch as well.  The
> FreeBSD-specific ones are probably ok, but patches 4-6 probably could use
> at least some looking over.

Hi John,

Since nobody else answered to patch 4, I am fine with you pushing the 
series, we should have plenty of time before the next release to fix any 
problem.

Thanks,

Simon

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

end of thread, other threads:[~2019-03-12 20:21 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-09  0:42 [PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
2019-02-09  0:42 ` [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type John Baldwin
2019-02-09  1:08   ` John Baldwin
2019-02-11 10:27     ` Philipp Rudo
2019-02-11 17:44       ` John Baldwin
2019-03-07 15:53   ` Simon Marchi
2019-03-08  0:04     ` John Baldwin
2019-03-08  0:32       ` Pedro Alves
2019-03-08 18:39         ` John Baldwin
2019-02-09  0:42 ` [PATCH v2 01/11] Support the fs_base and gs_base registers on i386 John Baldwin
2019-02-09  0:42 ` [PATCH v2 11/11] Support TLS variables on FreeBSD/powerpc John Baldwin
2019-03-07 16:26   ` Simon Marchi
2019-02-09  0:42 ` [PATCH v2 08/11] Support TLS variables on FreeBSD/amd64 John Baldwin
2019-02-09  0:42 ` [PATCH v2 03/11] Handle an edge case for minisym TLS variable lookups John Baldwin
2019-02-09  0:42 ` [PATCH v2 09/11] Support TLS variables on FreeBSD/i386 John Baldwin
2019-02-09  0:42 ` [PATCH v2 07/11] Add a helper function to resolve TLS variable addresses for FreeBSD John Baldwin
2019-03-07 16:18   ` Simon Marchi
2019-02-09  0:42 ` [PATCH v2 02/11] Support fs_base and gs_base on FreeBSD/i386 John Baldwin
2019-02-09  0:50 ` [PATCH v2 10/11] Support TLS variables on FreeBSD/riscv John Baldwin
2019-02-09  0:50 ` [PATCH v2 04/11] Add a new gdbarch method to resolve the address of TLS variables John Baldwin
2019-03-07 16:08   ` Simon Marchi
2019-03-07 23:50     ` John Baldwin
2019-03-08  2:55       ` Simon Marchi
2019-03-08 18:39         ` John Baldwin
2019-02-09  0:50 ` [PATCH v2 05/11] Remove code disabled since at least 1999 from lookup_struct_elt_type John Baldwin
2019-03-07 16:25   ` Simon Marchi
2019-02-22 17:22 ` [PING][PATCH v2 00/11] Support for thread-local variables on FreeBSD John Baldwin
2019-03-12 20:21   ` Simon Marchi

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