public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 9/9] Support TLS variables on FreeBSD/powerpc.
  2019-01-22 18:43 [PATCH 0/9] Support for thread-local variables on FreeBSD John Baldwin
                   ` (3 preceding siblings ...)
  2019-01-22 18:43 ` [PATCH 7/9] Support TLS variables on FreeBSD/i386 John Baldwin
@ 2019-01-22 18:43 ` John Baldwin
  2019-01-22 18:43 ` [PATCH 3/9] Handle TLS variable lookups when using separate debug object files John Baldwin
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: John Baldwin @ 2019-01-22 18:43 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 5d228614db..938fa83ca8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-01-22  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-01-22  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 6/9] Support TLS variables on FreeBSD/amd64.
  2019-01-22 18:43 [PATCH 0/9] Support for thread-local variables on FreeBSD John Baldwin
  2019-01-22 18:43 ` [PATCH 1/9] Support the fs_base and gs_base registers on i386 John Baldwin
  2019-01-22 18:43 ` [PATCH 2/9] Support fs_base and gs_base on FreeBSD/i386 John Baldwin
@ 2019-01-22 18:43 ` John Baldwin
  2019-01-22 18:43 ` [PATCH 7/9] Support TLS variables on FreeBSD/i386 John Baldwin
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: John Baldwin @ 2019-01-22 18:43 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 09e57e7474..7e97fae997 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-01-22  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-01-22  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 628d42f7a0..32bda5741c 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 1/9] Support the fs_base and gs_base registers on i386.
  2019-01-22 18:43 [PATCH 0/9] Support for thread-local variables on FreeBSD John Baldwin
@ 2019-01-22 18:43 ` John Baldwin
  2019-01-27  4:22   ` Simon Marchi
  2019-01-22 18:43 ` [PATCH 2/9] Support fs_base and gs_base on FreeBSD/i386 John Baldwin
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-01-22 18:43 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 8e03dbf883..4afd5b664e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,34 @@
+2019-01-22  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-01-22  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* event-top.c (handle_line_of_input): use unique_xmalloc_ptr for
diff --git a/gdb/amd64-fbsd-nat.c b/gdb/amd64-fbsd-nat.c
index 6d8e12b033..39ba69d9bf 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 d4143ae155..1eb0bcdf17 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 73987da97b..311b5fd8c8 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 a6a801b818..af8c6bd73f 100644
--- a/gdb/arch/i386.h
+++ b/gdb/arch/i386.h
@@ -18,4 +18,5 @@
 #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);
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 5312ca9919..fc8b37976a 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,11 @@
+2019-01-22  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-01-21  Tom Tromey  <tom@tromey.com>
 
 	* tracepoint.c: Fix includes.
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 0731d2a73d..77ec8235e0 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 19c9acf42b..6930664e00 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 30f7af7553..9b7bc40225 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 855572143b..8ff104cf72 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 82dfbd5c35..43b15167ae 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 5a18f35f99..200de53cd8 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 0f344b0710..b2461836ad 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 8b2c72ac7b..65a5d6ab95 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 2/9] Support fs_base and gs_base on FreeBSD/i386.
  2019-01-22 18:43 [PATCH 0/9] Support for thread-local variables on FreeBSD John Baldwin
  2019-01-22 18:43 ` [PATCH 1/9] Support the fs_base and gs_base registers on i386 John Baldwin
@ 2019-01-22 18:43 ` John Baldwin
  2019-02-02 15:26   ` Simon Marchi
  2019-01-22 18:43 ` [PATCH 6/9] Support TLS variables on FreeBSD/amd64 John Baldwin
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-01-22 18:43 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 4afd5b664e..056a60fa23 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,17 @@
+2019-01-22  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-01-22  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..0f47ff6c61 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 ());
+#ifdef PT_GETFSBASE
+  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 ());
+#ifdef PT_GETFSBASE
+  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 39ba69d9bf..e31232bd20 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 8ff104cf72..f2703a3c75 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 43b15167ae..239ca91abe 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 3/9] Handle TLS variable lookups when using separate debug object files.
  2019-01-22 18:43 [PATCH 0/9] Support for thread-local variables on FreeBSD John Baldwin
                   ` (4 preceding siblings ...)
  2019-01-22 18:43 ` [PATCH 9/9] Support TLS variables on FreeBSD/powerpc John Baldwin
@ 2019-01-22 18:43 ` John Baldwin
  2019-02-02 15:52   ` Simon Marchi
  2019-01-22 18:52 ` [PATCH 8/9] Support TLS variables on FreeBSD/riscv John Baldwin
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-01-22 18:43 UTC (permalink / raw)
  To: gdb-patches

The object files stored in the shared object list are the original
object files, not the separate debug object files.  However, when
resolving a TLS variable, the variable is resolved against a separate
debug object file if it exists.  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 056a60fa23..c02e534fe5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2019-01-22  John Baldwin  <jhb@FreeBSD.org>
+
+	* solib-svr4.c (svr4_fetch_objfile_link_map): Look for
+	objfile->separate_debug_objfile_backlink if not NULL.
+
 2019-01-22  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 0/9] Support for thread-local variables on FreeBSD
@ 2019-01-22 18:43 John Baldwin
  2019-01-22 18:43 ` [PATCH 1/9] Support the fs_base and gs_base registers on i386 John Baldwin
                   ` (8 more replies)
  0 siblings, 9 replies; 28+ messages in thread
From: John Baldwin @ 2019-01-22 18:43 UTC (permalink / raw)
  To: gdb-patches

This series adds support for thread-local variables on a few FreeBSD
architectures.  Unlike other platforms, this patch series does not
rely on libthread_db but uses a different approach.  It instead relies
on an architecture-specific method to locate an individual thread's
DTV array of TLS block pointers.  The code to locate the specific TLS
block for an object file is shared in a helper function (patch 5) that
the per-architecture methods invoke after locating the DTV array.
Unlike the libthread_db approach, this permits examining TLS variables
of single-threaded variables and supports arbitrary cross debugging
(subject to the relevant registers being available from a core dump).
This did require introducing a new gdbarch method to resolve a TLS
variable address used instead of a target method (patch 4).

For FreeBSD/i386, TLS variables are stored relative to the %gs segment
register.  This requires access to the base address of %gs to locate
the DTV array.  To facilitate this, the first two patches add support
for the %fs_base and %gs_base registers on i386.  I did test this on
Linux x86-64 where it introduces no new regressions in the test suite
in my testing.

There are some limitations in the FreeBSD TLS support:

1) FreeBSD/amd64 and FreeBSD/i386 do not currently include the
   %fs_base and %gs_base registers in core dumps, so TLS only works
   for "live" processes currently.

2) TLS variables in the executable didn't work for me on FreeBSD/riscv,
   but this appears to be a compiler bug as the variable did not have
   a valid DW_at_location in the dwarf info.  TLS variables in libc
   worked fine both live and with core dumps.

3) For FreeBSD/powerpc, I was only able to test powerpc64 using a
   cross-debugger on amd64.

John Baldwin (9):
  Support the fs_base and gs_base registers on i386.
  Support fs_base and gs_base on FreeBSD/i386.
  Handle TLS variable lookups when using separate debug object files.
  Add a new gdbarch method to resolve the address of TLS variables.
  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                        |  96 ++++++++++++++++++
 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                      | 146 +++++++++++++++++++++++++++
 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/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 ++-
 30 files changed, 580 insertions(+), 44 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 7/9] Support TLS variables on FreeBSD/i386.
  2019-01-22 18:43 [PATCH 0/9] Support for thread-local variables on FreeBSD John Baldwin
                   ` (2 preceding siblings ...)
  2019-01-22 18:43 ` [PATCH 6/9] Support TLS variables on FreeBSD/amd64 John Baldwin
@ 2019-01-22 18:43 ` John Baldwin
  2019-01-22 18:43 ` [PATCH 9/9] Support TLS variables on FreeBSD/powerpc John Baldwin
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: John Baldwin @ 2019-01-22 18:43 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 7e97fae997..06c637c80a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-01-22  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-01-22  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 239ca91abe..01141b9e14 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 8/9] Support TLS variables on FreeBSD/riscv.
  2019-01-22 18:43 [PATCH 0/9] Support for thread-local variables on FreeBSD John Baldwin
                   ` (5 preceding siblings ...)
  2019-01-22 18:43 ` [PATCH 3/9] Handle TLS variable lookups when using separate debug object files John Baldwin
@ 2019-01-22 18:52 ` John Baldwin
  2019-01-27 23:35   ` Andrew Burgess
  2019-01-22 18:52 ` [PATCH 5/9] Add a helper function to resolve TLS variable addresses for FreeBSD John Baldwin
  2019-01-22 18:52 ` [PATCH 4/9] Add a new gdbarch method to resolve the address of TLS variables John Baldwin
  8 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-01-22 18:52 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 06c637c80a..5d228614db 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-01-22  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-01-22  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 4/9] Add a new gdbarch method to resolve the address of TLS variables.
  2019-01-22 18:43 [PATCH 0/9] Support for thread-local variables on FreeBSD John Baldwin
                   ` (7 preceding siblings ...)
  2019-01-22 18:52 ` [PATCH 5/9] Add a helper function to resolve TLS variable addresses for FreeBSD John Baldwin
@ 2019-01-22 18:52 ` John Baldwin
  8 siblings, 0 replies; 28+ messages in thread
From: John Baldwin @ 2019-01-22 18:52 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 c02e534fe5..7c46bb0009 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2019-01-22  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-01-22  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 ad7eba3fa3..54d90e4f34 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

* [PATCH 5/9] Add a helper function to resolve TLS variable addresses for FreeBSD.
  2019-01-22 18:43 [PATCH 0/9] Support for thread-local variables on FreeBSD John Baldwin
                   ` (6 preceding siblings ...)
  2019-01-22 18:52 ` [PATCH 8/9] Support TLS variables on FreeBSD/riscv John Baldwin
@ 2019-01-22 18:52 ` John Baldwin
  2019-01-24 17:09   ` John Baldwin
  2019-01-22 18:52 ` [PATCH 4/9] Add a new gdbarch method to resolve the address of TLS variables John Baldwin
  8 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-01-22 18:52 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 | 146 ++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/fbsd-tdep.h |  10 ++++
 3 files changed, 166 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7c46bb0009..09e57e7474 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2019-01-22  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-01-22  John Baldwin  <jhb@FreeBSD.org>
 
 	* gdbarch.sh (get_thread_local_address): New method.
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index d971d3a653..2e0f72d17b 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,114 @@ 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.  */
+      data->off_linkmap = parse_and_eval_long ("&((Obj_Entry *)0)->linkmap");
+      data->off_tlsindex = parse_and_eval_long ("&((Obj_Entry *)0)->tlsindex");
+      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 +2101,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

* Re: [PATCH 5/9] Add a helper function to resolve TLS variable addresses for FreeBSD.
  2019-01-22 18:52 ` [PATCH 5/9] Add a helper function to resolve TLS variable addresses for FreeBSD John Baldwin
@ 2019-01-24 17:09   ` John Baldwin
  2019-02-07  5:05     ` Simon Marchi
  0 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-01-24 17:09 UTC (permalink / raw)
  To: gdb-patches

On 1/22/19 10:42 AM, 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.
> 
> 	* 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 | 146 ++++++++++++++++++++++++++++++++++++++++++++++++
>  gdb/fbsd-tdep.h |  10 ++++
>  3 files changed, 166 insertions(+)
> 
> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> index d971d3a653..2e0f72d17b 100644
> --- a/gdb/fbsd-tdep.c
> +++ b/gdb/fbsd-tdep.c
> +
> +/* 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.  */
> +      data->off_linkmap = parse_and_eval_long ("&((Obj_Entry *)0)->linkmap");
> +      data->off_tlsindex = parse_and_eval_long ("&((Obj_Entry *)0)->tlsindex");
> +      data->rtld_offsets_valid = true;
> +      return;

I'm not really happy about using parse_and_eval_long with an open-coded
equivalent of offsetof() here.  It seems we don't already have existing
functionality for this though?  I think I could use 'lookup_struct' to find
the 'struct type *' for 'Obj_Entry', but if I used 'lookup_struct_elt_type'
to get the type of an element that doesn't give me anything that has the
offset.  We could perhaps instead add a new function 'lookup_struct_elt_offset'
that took the element name as a string and figured out the offset.  We could
then use this to provide an 'offsetof' builtin for the C language perhaps.
However, I suspect that lookup_struct_elt_offset would have to invoke a
language-specific function to do the actual computation (just as ptype /o
handling is language-specific).

-- 
John Baldwin

                                                                            

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

* Re: [PATCH 1/9] Support the fs_base and gs_base registers on i386.
  2019-01-22 18:43 ` [PATCH 1/9] Support the fs_base and gs_base registers on i386 John Baldwin
@ 2019-01-27  4:22   ` Simon Marchi
  2019-01-28 17:54     ` John Baldwin
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2019-01-27  4:22 UTC (permalink / raw)
  To: John Baldwin, gdb-patches

On 2019-01-22 1:42 p.m., John Baldwin wrote:
> As on amd64, these registers hold the base address of the fs and gs
> segments, respectively.  For i386 these two registers are 32 bits.

I just started to look at this, still trying to get my head around it and
connect all the dots.  Just a silly question for now:

> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index d4143ae155..1eb0bcdf17 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;
>      }

Can you explain what this change does and why it is needed?  I am a bit lost.

I see you add something equivalent in i386-tdep.c, but as far as I know, they are completely
separate.

Simon

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

* Re: [PATCH 8/9] Support TLS variables on FreeBSD/riscv.
  2019-01-22 18:52 ` [PATCH 8/9] Support TLS variables on FreeBSD/riscv John Baldwin
@ 2019-01-27 23:35   ` Andrew Burgess
  0 siblings, 0 replies; 28+ messages in thread
From: Andrew Burgess @ 2019-01-27 23:35 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

* John Baldwin <jhb@FreeBSD.org> [2019-01-22 10:43:01 -0800]:

> 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(+)

These look good to me.

Thanks,
Andrew



> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 06c637c80a..5d228614db 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,10 @@
> +2019-01-22  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-01-22  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

* Re: [PATCH 1/9] Support the fs_base and gs_base registers on i386.
  2019-01-27  4:22   ` Simon Marchi
@ 2019-01-28 17:54     ` John Baldwin
  2019-02-02 15:11       ` Simon Marchi
  0 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-01-28 17:54 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

On 1/26/19 7:47 PM, Simon Marchi wrote:
> On 2019-01-22 1:42 p.m., John Baldwin wrote:
>> As on amd64, these registers hold the base address of the fs and gs
>> segments, respectively.  For i386 these two registers are 32 bits.
> 
> I just started to look at this, still trying to get my head around it and
> connect all the dots.  Just a silly question for now:
> 
>> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
>> index d4143ae155..1eb0bcdf17 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;
>>      }
> 
> Can you explain what this change does and why it is needed?  I am a bit lost.
> 
> I see you add something equivalent in i386-tdep.c, but as far as I know, they are completely
> separate.

It took a while for me to understand this as well. :-/

The 64-bit (amd64) and 32-bit (i386) gdbarch's share a fair bit of code.
In particular, the 64-bit gdbarch is "inherited" from the 32-bit one, but
overrides various behaviors.  Specifically, see the comment above
i386_gdbarch_init.  amd64_init_abi is called from OSABI specific handlers
part way through i386_gdbarch_init from gdbarch_init_osabi() to apply
64-bit overrides.

One of the things that gets overridden is that 32-bit and 64-bit gdbarch's
use different register numbers.  This is because 64-bit x86 has r8-15.
The first 8 registers are the same, but the remaining registers for FPU,
SSE, etc. are all shifted by 8 for amd64.  There's a comment that hints at
this in i386-tdep.h:

/* GDB's i386 target supports both the 32-bit Intel Architecture
   (IA-32) and the 64-bit AMD x86-64 architecture.  Internally it uses
   a similar register layout for both.

   - General purpose registers
   - FPU data registers
   - FPU control registers
   - SSE data registers
   - SSE control register

   The general purpose registers for the x86-64 architecture are quite
   different from IA-32.  Therefore, gdbarch_fp0_regnum
   determines the register number at which the FPU data registers
   start.  The number of FPU data and control registers is the same
   for both architectures.  The number of SSE registers however,
   differs and is determined by the num_xmm_regs member of `struct
   gdbarch_tdep'.  */

As the comment notes, the way this is handled for the FPU registers is that
tdep->fp0_regnum holds the value of the first FPU register (AMD64_ST0_REGNUM
vs I386_ST0_REGNUM).  This same approach is then used for all the other
shared register banks (tdep->mm0_regnum, ymm0_regnum, zmm0_regnum, etc.).
Helper variables are also used to handle the different numbers of SSE/AVX
registers on 32-bit vs 64-bit.

i386_validate_tdesc_p() is called after gdbarch_init_osabi() to add the
shared register sets.  It checks for each feature being present.  If it is,
it first checks to see if the corresponding tdep fields (e.g. ymm0_regnum)
have been set.  If they aren't, it sets them to the I386 values.  Then it
adds the appropriate registers for each feature.  In the case of a 64-bit
architecture, amd64_init_abi() is called from gdbarch_init_osabi() before
i386_validate_tdesc_p() is called.  amd64_init_abi() sets the various tdep
variables for any features that are present to the AMD64 values.  Then when
i386_validate_tdesc_p() runs, it still adds the registers, but it does so
using the 64-bit register numbers instead of 32-bit register numbers.

When fs_base and gs_base were first added, they were only added on amd64,
so they were not treated as a shared register set.  Instead, they were
just added directly in amd64_init_abi() using the AMD64 register numbers.
This particular part of the patch (along with the change to
i386_validate_tdesc_p in i386-tdep.c) is changing the fs_base/gs_base
register set to be a shared register set like FPU, AVX/SSE, etc.  It does
this by adding a new tdep->fsbase_regnum to hold the base number of the
two registers.  It then changes amd64_init_abi() to just set the tdep
field as it does for other register sets and changes i386_validate_tdesc_p
to add the registers at the appropriate register number, defaulting to the
I386 values if the tdep field wasn't set.

Hopefully that isn't too confusing.  It took me a while to figure it out
myself.

-- 
John Baldwin

                                                                            

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

* Re: [PATCH 1/9] Support the fs_base and gs_base registers on i386.
  2019-01-28 17:54     ` John Baldwin
@ 2019-02-02 15:11       ` Simon Marchi
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Marchi @ 2019-02-02 15:11 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

On 2019-01-28 12:54, John Baldwin wrote:
> On 1/26/19 7:47 PM, Simon Marchi wrote:
>> On 2019-01-22 1:42 p.m., John Baldwin wrote:
>>> As on amd64, these registers hold the base address of the fs and gs
>>> segments, respectively.  For i386 these two registers are 32 bits.
>> 
>> I just started to look at this, still trying to get my head around it 
>> and
>> connect all the dots.  Just a silly question for now:
>> 
>>> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
>>> index d4143ae155..1eb0bcdf17 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;
>>>      }
>> 
>> Can you explain what this change does and why it is needed?  I am a 
>> bit lost.
>> 
>> I see you add something equivalent in i386-tdep.c, but as far as I 
>> know, they are completely
>> separate.
> 
> It took a while for me to understand this as well. :-/
> 
> The 64-bit (amd64) and 32-bit (i386) gdbarch's share a fair bit of 
> code.
> In particular, the 64-bit gdbarch is "inherited" from the 32-bit one, 
> but
> overrides various behaviors.  Specifically, see the comment above
> i386_gdbarch_init.  amd64_init_abi is called from OSABI specific 
> handlers
> part way through i386_gdbarch_init from gdbarch_init_osabi() to apply
> 64-bit overrides.
> 
> One of the things that gets overridden is that 32-bit and 64-bit 
> gdbarch's
> use different register numbers.  This is because 64-bit x86 has r8-15.
> The first 8 registers are the same, but the remaining registers for 
> FPU,
> SSE, etc. are all shifted by 8 for amd64.  There's a comment that hints 
> at
> this in i386-tdep.h:
> 
> /* GDB's i386 target supports both the 32-bit Intel Architecture
>    (IA-32) and the 64-bit AMD x86-64 architecture.  Internally it uses
>    a similar register layout for both.
> 
>    - General purpose registers
>    - FPU data registers
>    - FPU control registers
>    - SSE data registers
>    - SSE control register
> 
>    The general purpose registers for the x86-64 architecture are quite
>    different from IA-32.  Therefore, gdbarch_fp0_regnum
>    determines the register number at which the FPU data registers
>    start.  The number of FPU data and control registers is the same
>    for both architectures.  The number of SSE registers however,
>    differs and is determined by the num_xmm_regs member of `struct
>    gdbarch_tdep'.  */
> 
> As the comment notes, the way this is handled for the FPU registers is 
> that
> tdep->fp0_regnum holds the value of the first FPU register 
> (AMD64_ST0_REGNUM
> vs I386_ST0_REGNUM).  This same approach is then used for all the other
> shared register banks (tdep->mm0_regnum, ymm0_regnum, zmm0_regnum, 
> etc.).
> Helper variables are also used to handle the different numbers of 
> SSE/AVX
> registers on 32-bit vs 64-bit.
> 
> i386_validate_tdesc_p() is called after gdbarch_init_osabi() to add the
> shared register sets.  It checks for each feature being present.  If it 
> is,
> it first checks to see if the corresponding tdep fields (e.g. 
> ymm0_regnum)
> have been set.  If they aren't, it sets them to the I386 values.  Then 
> it
> adds the appropriate registers for each feature.  In the case of a 
> 64-bit
> architecture, amd64_init_abi() is called from gdbarch_init_osabi() 
> before
> i386_validate_tdesc_p() is called.  amd64_init_abi() sets the various 
> tdep
> variables for any features that are present to the AMD64 values.  Then 
> when
> i386_validate_tdesc_p() runs, it still adds the registers, but it does 
> so
> using the 64-bit register numbers instead of 32-bit register numbers.
> 
> When fs_base and gs_base were first added, they were only added on 
> amd64,
> so they were not treated as a shared register set.  Instead, they were
> just added directly in amd64_init_abi() using the AMD64 register 
> numbers.
> This particular part of the patch (along with the change to
> i386_validate_tdesc_p in i386-tdep.c) is changing the fs_base/gs_base
> register set to be a shared register set like FPU, AVX/SSE, etc.  It 
> does
> this by adding a new tdep->fsbase_regnum to hold the base number of the
> two registers.  It then changes amd64_init_abi() to just set the tdep
> field as it does for other register sets and changes 
> i386_validate_tdesc_p
> to add the registers at the appropriate register number, defaulting to 
> the
> I386 values if the tdep field wasn't set.
> 
> Hopefully that isn't too confusing.  It took me a while to figure it 
> out
> myself.

Thanks for the explanation, it does clear it up.

There was nothing else in this patch that stood out to me.

Simon

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

* Re: [PATCH 2/9] Support fs_base and gs_base on FreeBSD/i386.
  2019-01-22 18:43 ` [PATCH 2/9] Support fs_base and gs_base on FreeBSD/i386 John Baldwin
@ 2019-02-02 15:26   ` Simon Marchi
  2019-02-04 19:45     ` John Baldwin
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2019-02-02 15:26 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

On 2019-01-22 13:42, John Baldwin wrote:
> 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.

To be clear, this happens when debugging a 32-bits process on a 64-bits 
OS?  When debugging a 32-bits process on a 32-bits OS, the code in 
i386-bsd-nat.c would be used?

> 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 4afd5b664e..056a60fa23 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,17 @@
> +2019-01-22  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-01-22  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..0f47ff6c61 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 ());
> +#ifdef PT_GETFSBASE
> +  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +#endif

TDEP is used in both #ifdef PT_GETFSBASE and #ifdef PT_GETGSBASE, but is 
only declared here in an #ifdef PT_GETFSBASE.  I suppose it's not 
actually an issue because they will always be present together.  But we 
might as well use "#if defined(PT_GETFSBASE) || defined(PT_GETGSBASE)" 
for the declaration.

> 
>    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 ());
> +#ifdef PT_GETFSBASE
> +  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +#endif

Same here.

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

It's probably safer to clear the value to 0 as you did, so that's fine.  
But I would have thought that when debugging 32-bits processes, the high 
bits would be ignored at some point.  The kernel would know that this is 
a 32 bits register, so it would just take the 32 low bits of what it 
receives, and it magically works whatever the original data type is 
because it's little endian.

Otherwise, this LGTM.

Simon

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

* Re: [PATCH 3/9] Handle TLS variable lookups when using separate debug object files.
  2019-01-22 18:43 ` [PATCH 3/9] Handle TLS variable lookups when using separate debug object files John Baldwin
@ 2019-02-02 15:52   ` Simon Marchi
  2019-02-04 20:02     ` John Baldwin
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2019-02-02 15:52 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

On 2019-01-22 13:42, John Baldwin wrote:
> The object files stored in the shared object list are the original
> object files, not the separate debug object files.  However, when
> resolving a TLS variable, the variable is resolved against a separate
> debug object file if it exists.  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.

Does this solve an existing issue, or an issue that would come up with 
the following patches?

Simon

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

* Re: [PATCH 2/9] Support fs_base and gs_base on FreeBSD/i386.
  2019-02-02 15:26   ` Simon Marchi
@ 2019-02-04 19:45     ` John Baldwin
  2019-02-05 18:59       ` Simon Marchi
  0 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-02-04 19:45 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On 2/2/19 7:26 AM, Simon Marchi wrote:
> On 2019-01-22 13:42, John Baldwin wrote:
>> 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.
> 
> To be clear, this happens when debugging a 32-bits process on a 64-bits 
> OS?  When debugging a 32-bits process on a 32-bits OS, the code in 
> i386-bsd-nat.c would be used?

Correct.

>> 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 4afd5b664e..056a60fa23 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,3 +1,17 @@
>> +2019-01-22  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-01-22  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..0f47ff6c61 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 ());
>> +#ifdef PT_GETFSBASE
>> +  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>> +#endif
> 
> TDEP is used in both #ifdef PT_GETFSBASE and #ifdef PT_GETGSBASE, but is 
> only declared here in an #ifdef PT_GETFSBASE.  I suppose it's not 
> actually an issue because they will always be present together.  But we 
> might as well use "#if defined(PT_GETFSBASE) || defined(PT_GETGSBASE)" 
> for the declaration.

Ok.  I could also just perhaps collapse the other two #ifdef's instead of
using separate ones.  FreeBSD always provides both if it provides one.  None
of the other BSD's provide these currently.  I suspect if they did they
would always provide both.

>> @@ -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);
> 
> It's probably safer to clear the value to 0 as you did, so that's fine.  
> But I would have thought that when debugging 32-bits processes, the high 
> bits would be ignored at some point.  The kernel would know that this is 
> a 32 bits register, so it would just take the 32 low bits of what it 
> receives, and it magically works whatever the original data type is 
> because it's little endian.

I had originally just done this out of paranoia, but I checked and FreeBSD's
amd64 kernel will actually fail attempts to set the base address higher
than 4G with EINVAL rather than silently truncating.

-- 
John Baldwin

                                                                            

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

* Re: [PATCH 3/9] Handle TLS variable lookups when using separate debug object files.
  2019-02-02 15:52   ` Simon Marchi
@ 2019-02-04 20:02     ` John Baldwin
  2019-02-05 20:06       ` Simon Marchi
  0 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-02-04 20:02 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On 2/2/19 7:52 AM, Simon Marchi wrote:
> On 2019-01-22 13:42, John Baldwin wrote:
>> The object files stored in the shared object list are the original
>> object files, not the separate debug object files.  However, when
>> resolving a TLS variable, the variable is resolved against a separate
>> debug object file if it exists.  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.
> 
> Does this solve an existing issue, or an issue that would come up with 
> the following patches?

I only noticed while working on these patches, but I believe it is a
generic issue.  I tried to reproduce on a Linux box by compiling a small
library with separate debug symbols and a program that linked against it
and running it under gdb, but TLS variables didn't work for me even without
separate debug symbols in my testing. :(

$ cat foo.c
#include "foo.h"

static __thread int foo_id;

void
set_foo_id(int id)
{
  foo_id = id;
}

int
get_foo_id(void)
{
  return foo_id;
}
$ cat foo.h
void set_foo_id(int id);
int get_foo_id(void);
$ cat main.c
#include <stdio.h>

#include "foo.h"

int
main(void)
{

  set_foo_id(47);
  printf("foo id = %d\n", get_foo_id());
  return (0);
}
$ cc -g -fPIC -shared foo.c -o libfoo.so.full
$ objcopy --only-keep-debug libfoo.so.full libfoo.so.debug
$ objcopy --strip-debug --add-gnu-debuglink=libfoo.so.debug libfoo.so.full libfoo.so
$ cc -g main.c -o foo -lfoo -L.
$ gdb foo
GNU gdb (Ubuntu 8.1-0ubuntu3) 8.1.0.20180409-git
...
(gdb) start
Temporary breakpoint 1 at 0x7ae: file main.c, line 9.
Starting program: /home/john/tls_lib/foo 

Temporary breakpoint 1, main () at main.c:9
9         set_foo_id(47);
(gdb) p foo_id
Cannot find thread-local storage for process 3970, shared library libfoo.so:
Cannot find thread-local variables on this target

Then tried it without separate debug file, but that didn't work either:

$ cc -g -fPIC -shared foo.c -o libfoo.so
$ gdb foo
GNU gdb (Ubuntu 8.1-0ubuntu3) 8.1.0.20180409-git
...
(gdb) start
Temporary breakpoint 1 at 0x7ae: file main.c, line 9.
Starting program: /home/john/tls_lib/foo 

Temporary breakpoint 1, main () at main.c:9
9         set_foo_id(47);
(gdb) p foo_id
Cannot find thread-local storage for process 3982, shared library libfoo.so:
Cannot find thread-local variables on this target
(gdb) n
10        printf("foo id = %d\n", get_foo_id());
(gdb) 
foo id = 47
11        return (0);
(gdb) p foo_id
Cannot find thread-local storage for process 3982, shared library libfoo.so:
Cannot find thread-local variables on this target

I would have expected the second case to work and the first to fail and for
the patch to fix the first case.

-- 
John Baldwin

                                                                            

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

* Re: [PATCH 2/9] Support fs_base and gs_base on FreeBSD/i386.
  2019-02-04 19:45     ` John Baldwin
@ 2019-02-05 18:59       ` Simon Marchi
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Marchi @ 2019-02-05 18:59 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

On 2019-02-04 14:45, John Baldwin wrote:
> I had originally just done this out of paranoia, but I checked and 
> FreeBSD's
> amd64 kernel will actually fail attempts to set the base address higher
> than 4G with EINVAL rather than silently truncating.

That's even better!

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

* Re: [PATCH 3/9] Handle TLS variable lookups when using separate debug object files.
  2019-02-04 20:02     ` John Baldwin
@ 2019-02-05 20:06       ` Simon Marchi
  2019-02-05 22:21         ` John Baldwin
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2019-02-05 20:06 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 3797 bytes --]

On 2019-02-04 15:02, John Baldwin wrote:
> On 2/2/19 7:52 AM, Simon Marchi wrote:
>> On 2019-01-22 13:42, John Baldwin wrote:
>>> The object files stored in the shared object list are the original
>>> object files, not the separate debug object files.  However, when
>>> resolving a TLS variable, the variable is resolved against a separate
>>> debug object file if it exists.  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.
>> 
>> Does this solve an existing issue, or an issue that would come up with
>> the following patches?
> 
> I only noticed while working on these patches, but I believe it is a
> generic issue.  I tried to reproduce on a Linux box by compiling a 
> small
> library with separate debug symbols and a program that linked against 
> it
> and running it under gdb, but TLS variables didn't work for me even 
> without
> separate debug symbols in my testing. :(
> 
> $ cat foo.c
> #include "foo.h"
> 
> static __thread int foo_id;
> 
> void
> set_foo_id(int id)
> {
>   foo_id = id;
> }
> 
> int
> get_foo_id(void)
> {
>   return foo_id;
> }
> $ cat foo.h
> void set_foo_id(int id);
> int get_foo_id(void);
> $ cat main.c
> #include <stdio.h>
> 
> #include "foo.h"
> 
> int
> main(void)
> {
> 
>   set_foo_id(47);
>   printf("foo id = %d\n", get_foo_id());
>   return (0);
> }
> $ cc -g -fPIC -shared foo.c -o libfoo.so.full
> $ objcopy --only-keep-debug libfoo.so.full libfoo.so.debug
> $ objcopy --strip-debug --add-gnu-debuglink=libfoo.so.debug
> libfoo.so.full libfoo.so
> $ cc -g main.c -o foo -lfoo -L.
> $ gdb foo
> GNU gdb (Ubuntu 8.1-0ubuntu3) 8.1.0.20180409-git
> ...
> (gdb) start
> Temporary breakpoint 1 at 0x7ae: file main.c, line 9.
> Starting program: /home/john/tls_lib/foo
> 
> Temporary breakpoint 1, main () at main.c:9
> 9         set_foo_id(47);
> (gdb) p foo_id
> Cannot find thread-local storage for process 3970, shared library 
> libfoo.so:
> Cannot find thread-local variables on this target
> 
> Then tried it without separate debug file, but that didn't work either:
> 
> $ cc -g -fPIC -shared foo.c -o libfoo.so
> $ gdb foo
> GNU gdb (Ubuntu 8.1-0ubuntu3) 8.1.0.20180409-git
> ...
> (gdb) start
> Temporary breakpoint 1 at 0x7ae: file main.c, line 9.
> Starting program: /home/john/tls_lib/foo
> 
> Temporary breakpoint 1, main () at main.c:9
> 9         set_foo_id(47);
> (gdb) p foo_id
> Cannot find thread-local storage for process 3982, shared library 
> libfoo.so:
> Cannot find thread-local variables on this target
> (gdb) n
> 10        printf("foo id = %d\n", get_foo_id());
> (gdb)
> foo id = 47
> 11        return (0);
> (gdb) p foo_id
> Cannot find thread-local storage for process 3982, shared library 
> libfoo.so:
> Cannot find thread-local variables on this target
> 
> I would have expected the second case to work and the first to fail and 
> for
> the patch to fix the first case.

I did a similar test and hit the same message.  I figured it's because 
you need to build with -pthread.  With -pthread, GDB manages to print 
the value in both cases (with and without separate debug info).  That's 
why I ended up asking you this question, I thought that maybe I just 
hadn't reproduced the issue correctly.

I have attached my test case (so we don't have to rewrite the same test 
over and over), which you can easily build with and without debug info 
(see the Makefile).  Does it trigger the problem on FreeBSD?  Does it 
work for you in both cases on Linux, like it does for me?

Now that I take a look, there might be the gdb.threads/tls-shared.exp 
test that does the exact same thing.  But there doesn't seem to be a 
board file to test with separate debug files out of the box...

Simon

[-- Attachment #2: shared-lib-tls-test.tgz --]
[-- Type: application/x-gzip, Size: 690 bytes --]

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

* Re: [PATCH 3/9] Handle TLS variable lookups when using separate debug object files.
  2019-02-05 20:06       ` Simon Marchi
@ 2019-02-05 22:21         ` John Baldwin
  2019-02-05 22:33           ` John Baldwin
  0 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-02-05 22:21 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On 2/5/19 12:06 PM, Simon Marchi wrote:
> On 2019-02-04 15:02, John Baldwin wrote:
>> On 2/2/19 7:52 AM, Simon Marchi wrote:
>>> On 2019-01-22 13:42, John Baldwin wrote:
>>>> The object files stored in the shared object list are the original
>>>> object files, not the separate debug object files.  However, when
>>>> resolving a TLS variable, the variable is resolved against a separate
>>>> debug object file if it exists.  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.
>>>
>>> Does this solve an existing issue, or an issue that would come up with
>>> the following patches?
>>
>> I only noticed while working on these patches, but I believe it is a
>> generic issue.  I tried to reproduce on a Linux box by compiling a 
>> small
>> library with separate debug symbols and a program that linked against 
>> it
>> and running it under gdb, but TLS variables didn't work for me even 
>> without
>> separate debug symbols in my testing. :(
>>
>> $ cat foo.c
>> #include "foo.h"
>>
>> static __thread int foo_id;
>>
>> void
>> set_foo_id(int id)
>> {
>>   foo_id = id;
>> }
>>
>> int
>> get_foo_id(void)
>> {
>>   return foo_id;
>> }
>> $ cat foo.h
>> void set_foo_id(int id);
>> int get_foo_id(void);
>> $ cat main.c
>> #include <stdio.h>
>>
>> #include "foo.h"
>>
>> int
>> main(void)
>> {
>>
>>   set_foo_id(47);
>>   printf("foo id = %d\n", get_foo_id());
>>   return (0);
>> }
>> $ cc -g -fPIC -shared foo.c -o libfoo.so.full
>> $ objcopy --only-keep-debug libfoo.so.full libfoo.so.debug
>> $ objcopy --strip-debug --add-gnu-debuglink=libfoo.so.debug
>> libfoo.so.full libfoo.so
>> $ cc -g main.c -o foo -lfoo -L.
>> $ gdb foo
>> GNU gdb (Ubuntu 8.1-0ubuntu3) 8.1.0.20180409-git
>> ...
>> (gdb) start
>> Temporary breakpoint 1 at 0x7ae: file main.c, line 9.
>> Starting program: /home/john/tls_lib/foo
>>
>> Temporary breakpoint 1, main () at main.c:9
>> 9         set_foo_id(47);
>> (gdb) p foo_id
>> Cannot find thread-local storage for process 3970, shared library 
>> libfoo.so:
>> Cannot find thread-local variables on this target
>>
>> Then tried it without separate debug file, but that didn't work either:
>>
>> $ cc -g -fPIC -shared foo.c -o libfoo.so
>> $ gdb foo
>> GNU gdb (Ubuntu 8.1-0ubuntu3) 8.1.0.20180409-git
>> ...
>> (gdb) start
>> Temporary breakpoint 1 at 0x7ae: file main.c, line 9.
>> Starting program: /home/john/tls_lib/foo
>>
>> Temporary breakpoint 1, main () at main.c:9
>> 9         set_foo_id(47);
>> (gdb) p foo_id
>> Cannot find thread-local storage for process 3982, shared library 
>> libfoo.so:
>> Cannot find thread-local variables on this target
>> (gdb) n
>> 10        printf("foo id = %d\n", get_foo_id());
>> (gdb)
>> foo id = 47
>> 11        return (0);
>> (gdb) p foo_id
>> Cannot find thread-local storage for process 3982, shared library 
>> libfoo.so:
>> Cannot find thread-local variables on this target
>>
>> I would have expected the second case to work and the first to fail and 
>> for
>> the patch to fix the first case.
> 
> I did a similar test and hit the same message.  I figured it's because 
> you need to build with -pthread.  With -pthread, GDB manages to print 
> the value in both cases (with and without separate debug info).  That's 
> why I ended up asking you this question, I thought that maybe I just 
> hadn't reproduced the issue correctly.
> 
> I have attached my test case (so we don't have to rewrite the same test 
> over and over), which you can easily build with and without debug info 
> (see the Makefile).  Does it trigger the problem on FreeBSD?  Does it 
> work for you in both cases on Linux, like it does for me?
> 
> Now that I take a look, there might be the gdb.threads/tls-shared.exp 
> test that does the exact same thing.  But there doesn't seem to be a 
> board file to test with separate debug files out of the box...

So, it works fine for me without this patch on FreeBSD.  However, I dug
around a bit more as I had only noticed this before when trying to debug
a core dump for riscv (as FreeBSD/riscv64 core dumps have the tp register
in them).  That is when I get a separate objfile passed to
svr4_fetch_objfile_link_map:

Breakpoint 3, svr4_fetch_objfile_link_map (objfile=0x8038a1600) at ../../gdb/solib-svr4.c:1541
1541      struct svr4_info *info = get_svr4_info ();
(top-gdb) p *objfile
$3 = {next = 0x8038a1480, 
  original_name = 0x8038df010 "/usr/lib/debug//ufs/riscv64/rootfs/lib/libc.so.7.debug", addr_low = 0x0, flags = {
...
(top-gdb) where
#0  svr4_fetch_objfile_link_map (objfile=0x8038a1600)
    at ../../gdb/solib-svr4.c:1541
#1  0x00000000015085e7 in gdbarch_fetch_tls_load_module_address (gdbarch=0x803800010, objfile=0x8038a1600) at ../../gdb/gdbarch.c:3019
#2  0x00000000019ba415 in target_translate_tls_address (objfile=0x8038a1600, 
    offset=0x1800) at ../../gdb/target.c:705
#3  0x00000000016ea2a1 in find_minsym_type_and_address (msymbol=0x803cfcb18, 
    objfile=0x8038a1600, address_p=0x7fffffffd2e8) at ../../gdb/parse.c:500
#4  0x00000000014afeb7 in evaluate_var_msym_value (noside=EVAL_NORMAL, 
    objfile=0x8038a1600, msymbol=0x803cfcb18) at ../../gdb/eval.c:743
#5  0x00000000014b0290 in evaluate_subexp_standard (expect_type=0x0, 
    exp=0x8034f0430, pos=0x7fffffffde34, noside=EVAL_NORMAL)
    at ../../gdb/eval.c:1326
#6  0x00000000012a8562 in evaluate_subexp_c (expect_type=0x0, exp=0x8034f0430, 
    pos=0x7fffffffde34, noside=EVAL_NORMAL) at ../../gdb/c-lang.c:704
#7  0x00000000014ae17b in evaluate_subexp (expect_type=0x0, exp=0x8034f0430, 
    pos=0x7fffffffde34, noside=EVAL_NORMAL) at ../../gdb/eval.c:80
#8  0x00000000014ae48b in evaluate_expression (exp=0x8034f0430)
    at ../../gdb/eval.c:141
#9  0x000000000173184f in print_command_1 (
    exp=0x7fffffffe101 "__je_tsd_initialized", voidprint=1)
    at ../../gdb/printcmd.c:1187
#10 0x000000000172e5ff in print_command (
    exp=0x7fffffffe101 "__je_tsd_initialized", from_tty=1)
    at ../../gdb/printcmd.c:1200
(top-gdb) frame 5
#5  0x00000000014b0290 in evaluate_subexp_standard (expect_type=0x0, 
    exp=0x8034f0430, pos=0x7fffffffde34, noside=EVAL_NORMAL)
    at ../../gdb/eval.c:1326
1326            value *val = evaluate_var_msym_value (noside,
(top-gdb) l
1321        case OP_VAR_MSYM_VALUE:
1322          {
1323            (*pos) += 3;
1324
1325            minimal_symbol *msymbol = exp->elts[pc + 2].msymbol;
1326            value *val = evaluate_var_msym_value (noside,
1327                                                  exp->elts[pc + 1].objfile,
1328                                                  msymbol);
1329
1330            type = value_type (val);

whereas on a live amd64 process this was called from a different place:

(top-gdb) where
#0  svr4_fetch_objfile_link_map (objfile=0x803879380)
    at ../../gdb/solib-svr4.c:1541
#1  0x00000000015085e7 in gdbarch_fetch_tls_load_module_address (gdbarch=0x8039b7010, objfile=0x803879380) at ../../gdb/gdbarch.c:3019
#2  0x00000000019ba3f5 in target_translate_tls_address (objfile=0x803879380, 
    offset=6168) at ../../gdb/target.c:705
#3  0x000000000141f361 in dwarf_evaluate_loc_desc::get_tls_address (
    this=0x7fffffffc578, offset=6168) at ../../gdb/dwarf2loc.c:609
#4  0x00000000014077dd in dwarf_expr_context::execute_stack_op (
    this=0x7fffffffc578, op_ptr=0x803dea6fb "\002S\177\001", 
    op_end=0x803dea6fb "\002S\177\001") at ../../gdb/dwarf2expr.c:1175
#5  0x00000000014056b1 in dwarf_expr_context::eval (this=0x7fffffffc578, 
    addr=0x803dea6f1 "\016\030\030", len=10) at ../../gdb/dwarf2expr.c:301
#6  0x000000000140e99d in dwarf2_evaluate_loc_desc_full (type=0x8040ad930, 
    frame=0x0, data=0x803dea6f1 "\016\030\030", size=10, per_cu=0x8039cb190, 
    subobj_type=0x8040ad930, subobj_byte_offset=0)
    at ../../gdb/dwarf2loc.c:2170
#7  0x000000000140e7d2 in dwarf2_evaluate_loc_desc (type=0x8040ad930, 
    frame=0x0, data=0x803dea6f1 "\016\030\030", size=10, per_cu=0x8039cb190)
    at ../../gdb/dwarf2loc.c:2352
#8  0x0000000001412794 in locexpr_read_variable (symbol=0x80416c930, frame=0x0)
    at ../../gdb/dwarf2loc.c:3503
#9  0x00000000014e3743 in default_read_var_value (var=0x80416c930, 
    var_block=0x8041724e0, frame=0x0) at ../../gdb/findvar.c:610
#10 0x00000000014e4881 in read_var_value (var=0x80416c930, 
    var_block=0x8041724e0, frame=0x0) at ../../gdb/findvar.c:815
#11 0x0000000001a99abe in value_of_variable (var=0x80416c930, b=0x8041724e0) at ../../gdb/valops.c:1292
#12 0x00000000014afd7b in evaluate_var_value (noside=EVAL_NORMAL, blk=0x8041724e0, var=0x80416c930) at ../../gdb/eval.c:721
#13 0x00000000014b0217 in evaluate_subexp_standard (expect_type=0x0, exp=0x803507830, pos=0x7fffffffd504, noside=EVAL_NORMAL) at ../../gdb/eval.c:1312
#14 0x00000000012a8562 in evaluate_subexp_c (expect_type=0x0, exp=0x803507830, pos=0x7fffffffd504, noside=EVAL_NORMAL) at ../../gdb/c-lang.c:704
#15 0x00000000014ae17b in evaluate_subexp (expect_type=0x0, exp=0x803507830, pos=0x7fffffffd504, noside=EVAL_NORMAL) at ../../gdb/eval.c:80
#16 0x00000000014ae48b in evaluate_expression (exp=0x803507830) at ../../gdb/eval.c:141
#17 0x000000000173184f in print_command_1 (exp=0x7fffffffd7d1 "__je_tsd_initialized", voidprint=1) at ../../gdb/printcmd.c:1187
#18 0x000000000172e5ff in print_command (exp=0x7fffffffd7d1 "__je_tsd_initialized", from_tty=1) at ../../gdb/printcmd.c:1200
(top-gdb) frame 13
#13 0x00000000014b0217 in evaluate_subexp_standard (expect_type=0x0, 
    exp=0x803507830, pos=0x7fffffffd504, noside=EVAL_NORMAL)
    at ../../gdb/eval.c:1312
1312                return evaluate_var_value (noside, exp->elts[pc + 1].block, var);
(top-gdb) l
1307            (*pos) += 3;
1308            symbol *var = exp->elts[pc + 2].symbol;
1309            if (TYPE_CODE (SYMBOL_TYPE (var)) == TYPE_CODE_ERROR)
1310              error_unknown_type (SYMBOL_PRINT_NAME (var));
1311            if (noside != EVAL_SKIP)
1312                return evaluate_var_value (noside, exp->elts[pc + 1].block, var);
1313            else
1314              {
1315                /* Return a dummy value of the correct type when skipping, so
1316                   that parent functions know what is to be skipped.  */

So it seems that the OP_VAR_VALUE path calls down into the dwarf bits that
get the "original" objfile to pass to target_translate_tls_address, whereas
the OP_VAR_MSYM_VALUE case ends up using a separate object file.  This might
in fact be due to bugs in the RISCV GCC backend as the TLS symbols for RISCV
don't seem quite right.  I have to cast TLS variables to their types for
example:

(gdb) p __je_tsd_initialized
'__je_tsd_initialized` has unknown type; cast it to its declared type
(gdb) p (bool)__je_tsd_initialized
$2 = 1 '\001'

Also, for me TLS variables in the main executable did not work for me on
RISCV, only TLS variables in shared libraries, unlike on other architectures
I tested where TLS variables in both the executable and shared libraries
worked.

-- 
John Baldwin

                                                                            

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

* Re: [PATCH 3/9] Handle TLS variable lookups when using separate debug object files.
  2019-02-05 22:21         ` John Baldwin
@ 2019-02-05 22:33           ` John Baldwin
  2019-02-07  4:05             ` Simon Marchi
  0 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-02-05 22:33 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On 2/5/19 2:21 PM, John Baldwin wrote:
> So it seems that the OP_VAR_VALUE path calls down into the dwarf bits that
> get the "original" objfile to pass to target_translate_tls_address, whereas
> the OP_VAR_MSYM_VALUE case ends up using a separate object file.  This might
> in fact be due to bugs in the RISCV GCC backend as the TLS symbols for RISCV
> don't seem quite right.  I have to cast TLS variables to their types for
> example:
> 
> (gdb) p __je_tsd_initialized
> '__je_tsd_initialized` has unknown type; cast it to its declared type
> (gdb) p (bool)__je_tsd_initialized
> $2 = 1 '\001'
> 
> Also, for me TLS variables in the main executable did not work for me on
> RISCV, only TLS variables in shared libraries, unlike on other architectures
> I tested where TLS variables in both the executable and shared libraries
> worked.

I should have said: I think this means I probably need to rework the commit
message a bit to explain that this doesn't always happen with separate
debug files, but it can, and if so this fix is needed.

-- 
John Baldwin

                                                                            

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

* Re: [PATCH 3/9] Handle TLS variable lookups when using separate debug object files.
  2019-02-05 22:33           ` John Baldwin
@ 2019-02-07  4:05             ` Simon Marchi
  2019-02-07  4:08               ` Simon Marchi
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2019-02-07  4:05 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

On 2019-02-05 17:33, John Baldwin wrote:
> On 2/5/19 2:21 PM, John Baldwin wrote:
>> So it seems that the OP_VAR_VALUE path calls down into the dwarf bits 
>> that
>> get the "original" objfile to pass to target_translate_tls_address, 
>> whereas
>> the OP_VAR_MSYM_VALUE case ends up using a separate object file.  This 
>> might
>> in fact be due to bugs in the RISCV GCC backend as the TLS symbols for 
>> RISCV
>> don't seem quite right.  I have to cast TLS variables to their types 
>> for
>> example:
>> 
>> (gdb) p __je_tsd_initialized
>> '__je_tsd_initialized` has unknown type; cast it to its declared type
>> (gdb) p (bool)__je_tsd_initialized
>> $2 = 1 '\001'
>> 
>> Also, for me TLS variables in the main executable did not work for me 
>> on
>> RISCV, only TLS variables in shared libraries, unlike on other 
>> architectures
>> I tested where TLS variables in both the executable and shared 
>> libraries
>> worked.
> 
> I should have said: I think this means I probably need to rework the 
> commit
> message a bit to explain that this doesn't always happen with separate
> debug files, but it can, and if so this fix is needed.

After discussing with John on IRC and trying by all means to trigger 
this bug, this is what I ended up with.  On x86/GNU/Linux, it is 
possible to trigger this bug in a rather contrived way, but at least it 
shows that there is indeed a bug in GDB.  The requirements are:

1. Have a TLS variable in a shared library ("libfoo.so")
2. The .o containing the TLS variable should not be described with 
DWARF.  This can be done simply by compiling it without -g.
3. The shared lib should still have a separate debug info file 
("libfoo.so.debug").
4. The shared lib should be stripped of its unnecessary ELF symbols (ran 
through the strip utility)
5. The shared library should not be the first object file in the program 
to have TLS.  This can be done by adding a TLS variable in the main 
executable ("main").
6. Because we don't have debug info for the variable we try to print, we 
need to cast it to its expected type, e.g. "print (int)foo_id".

With all this, when parsing "(int)foo_id", we find a minimal symbol 
matching foo_id in the objfile representing libfoo.so.debug.  
find_minsym_type_and_address is eventually called with that objfile, 
which calls target_translate_tls_address, which calls 
svr4_fetch_objfile_link_map.  The latter obviously can't find a link_map 
matching the separate-debug-info objfile, and wrong things happen after.

Without #2 above, the DWARF symbol is found and some other code path is 
taken, where the separate-debug-info objfile is replaced with the "real" 
objfile at some point.

Without #3, we obviously wouldn't end up with a separate-debug-info 
objfile in svr4_fetch_objfile_link_map.

Without #4 above, we would find the objfile for libfoo.so first, so we 
wouldn't end up with the seaprate-debug-info objfile in 
svr4_fetch_objfile_link_map.

Without #5 we actually get the right result by chance.  This is because 
we end up in the special case "else" of 
thread_db_target::get_thread_local_address.  That special case hardcodes 
the module id to read from to 1.  If the shared lib is the only module 
to have TLS, then it happens to be the right one.  By making the main 
executable have some TLS, we will end up reading the TLS for the wrong 
module, and thus trigger the bug.

I have not yet found the motivation to write a proper test for this (in 
particular, I am not sure how to build the lib with separate debug info 
in the testsuite).  But I attached a reproducer for future reference.  
You should just need to "make" and "gdb -x run.gdb".  The wrong value is 
printed with today's GDB.

In John's specific case, apparently the DWARF debug info for TLS 
variables on riscv is broken, which brought him to roughly the same 
state.  I don't have any more info about that.

All this to say that I think this patch is fine.  I wondered whether it 
was better instead to make svr4_fetch_objfile_link_map assert that it 
receives a "real" objfile (objfile->separate_debug_objfile_backlink == 
NULL) and push the responsibility to the caller to provide a correct 
objfile.  But in the end I didn't find a compelling to do this rather 
than what John did in this patch.  So therefore, the patch LGTM.

Simon

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

* Re: [PATCH 3/9] Handle TLS variable lookups when using separate debug object files.
  2019-02-07  4:05             ` Simon Marchi
@ 2019-02-07  4:08               ` Simon Marchi
  0 siblings, 0 replies; 28+ messages in thread
From: Simon Marchi @ 2019-02-07  4:08 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 425 bytes --]

On 2019-02-06 23:05, Simon Marchi wrote:
> I have not yet found the motivation to write a proper test for this
> (in particular, I am not sure how to build the lib with separate debug
> info in the testsuite).  But I attached a reproducer for future
> reference.  You should just need to "make" and "gdb -x run.gdb".  The
> wrong value is printed with today's GDB.

And of course I forgot the attachment... here it is.

Simon

[-- Attachment #2: shared-lib-tls-test-2.tgz --]
[-- Type: application/x-gzip, Size: 700 bytes --]

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

* Re: [PATCH 5/9] Add a helper function to resolve TLS variable addresses for FreeBSD.
  2019-01-24 17:09   ` John Baldwin
@ 2019-02-07  5:05     ` Simon Marchi
  2019-02-07 17:02       ` John Baldwin
  0 siblings, 1 reply; 28+ messages in thread
From: Simon Marchi @ 2019-02-07  5:05 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

On 2019-01-24 12:08, John Baldwin wrote:
> On 1/22/19 10:42 AM, 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.
>> 
>> 	* 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 | 146 
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>  gdb/fbsd-tdep.h |  10 ++++
>>  3 files changed, 166 insertions(+)
>> 
>> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
>> index d971d3a653..2e0f72d17b 100644
>> --- a/gdb/fbsd-tdep.c
>> +++ b/gdb/fbsd-tdep.c
>> +
>> +/* 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.  */
>> +      data->off_linkmap = parse_and_eval_long ("&((Obj_Entry 
>> *)0)->linkmap");
>> +      data->off_tlsindex = parse_and_eval_long ("&((Obj_Entry 
>> *)0)->tlsindex");
>> +      data->rtld_offsets_valid = true;
>> +      return;
> 
> I'm not really happy about using parse_and_eval_long with an open-coded
> equivalent of offsetof() here.  It seems we don't already have existing
> functionality for this though?  I think I could use 'lookup_struct' to 
> find
> the 'struct type *' for 'Obj_Entry', but if I used 
> 'lookup_struct_elt_type'
> to get the type of an element that doesn't give me anything that has 
> the
> offset.  We could perhaps instead add a new function 
> 'lookup_struct_elt_offset'
> that took the element name as a string and figured out the offset.  We 
> could
> then use this to provide an 'offsetof' builtin for the C language 
> perhaps.
> However, I suspect that lookup_struct_elt_offset would have to invoke a
> language-specific function to do the actual computation (just as ptype 
> /o
> handling is language-specific).

Doesn't parse_and_eval_long also call in language-specific things?  The 
expression is parsed according to whatever is the current language, I 
suppose.  If needed, I guess we could change temporarily the current 
language to C, which is the language the dynamic linker is written in 
(until proven otherwise).

The offset of fields within structures is available (see macro 
FIELD_BITPOS).  I haven't looked too deep into it, but it sounds like it 
should be possible to implement what you need with that.

One question: does this work only when you have debug info for the 
dynamic linker data structures?  Will debug info always be available?  
On some Linux distributions, for example, I think it's rather common to 
not have the debug info for the system libraries (libc, libpthread) by 
default.

Simon

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

* Re: [PATCH 5/9] Add a helper function to resolve TLS variable addresses for FreeBSD.
  2019-02-07  5:05     ` Simon Marchi
@ 2019-02-07 17:02       ` John Baldwin
  2019-02-09  0:34         ` John Baldwin
  0 siblings, 1 reply; 28+ messages in thread
From: John Baldwin @ 2019-02-07 17:02 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches

On 2/6/19 9:04 PM, Simon Marchi wrote:
> On 2019-01-24 12:08, John Baldwin wrote:
>> On 1/22/19 10:42 AM, 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.
>>>
>>> 	* 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 | 146 
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>  gdb/fbsd-tdep.h |  10 ++++
>>>  3 files changed, 166 insertions(+)
>>>
>>> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
>>> index d971d3a653..2e0f72d17b 100644
>>> --- a/gdb/fbsd-tdep.c
>>> +++ b/gdb/fbsd-tdep.c
>>> +
>>> +/* 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.  */
>>> +      data->off_linkmap = parse_and_eval_long ("&((Obj_Entry 
>>> *)0)->linkmap");
>>> +      data->off_tlsindex = parse_and_eval_long ("&((Obj_Entry 
>>> *)0)->tlsindex");
>>> +      data->rtld_offsets_valid = true;
>>> +      return;
>>
>> I'm not really happy about using parse_and_eval_long with an open-coded
>> equivalent of offsetof() here.  It seems we don't already have existing
>> functionality for this though?  I think I could use 'lookup_struct' to 
>> find
>> the 'struct type *' for 'Obj_Entry', but if I used 
>> 'lookup_struct_elt_type'
>> to get the type of an element that doesn't give me anything that has 
>> the
>> offset.  We could perhaps instead add a new function 
>> 'lookup_struct_elt_offset'
>> that took the element name as a string and figured out the offset.  We 
>> could
>> then use this to provide an 'offsetof' builtin for the C language 
>> perhaps.
>> However, I suspect that lookup_struct_elt_offset would have to invoke a
>> language-specific function to do the actual computation (just as ptype 
>> /o
>> handling is language-specific).
> 
> Doesn't parse_and_eval_long also call in language-specific things?  The 
> expression is parsed according to whatever is the current language, I 
> suppose.  If needed, I guess we could change temporarily the current 
> language to C, which is the language the dynamic linker is written in 
> (until proven otherwise).

Mmm, that's true.

> The offset of fields within structures is available (see macro 
> FIELD_BITPOS).  I haven't looked too deep into it, but it sounds like it 
> should be possible to implement what you need with that.

Hmm.  When I looked at the code for ptype /o it seemed to keep a running
tally for the byte offset.  It wasn't clear to me if perhaps FIELD_BITPOS
was for bitfields and the bit offset within a byte/word.  However, the
Linux kernel patch series does have some code to do this.  I could perhaps
steal from that to implement lookup_struct_elt_offset.  I would probably
make it support nested fields as well (I think the Linux kernel patches are
using it for some nested fields).

> One question: does this work only when you have debug info for the 
> dynamic linker data structures?  Will debug info always be available?  
> On some Linux distributions, for example, I think it's rather common to 
> not have the debug info for the system libraries (libc, libpthread) by 
> default.

Yes, this requires debug symbols.  Recent releases of FreeBSD do include an
option to install them by default, but they are not always present.  I will
probably add global variables in rtld itself similar to the ones inside of
the thread library and then update GDB to fetch those instead once I've done
that.  I just haven't patched the runtime linker yet for that.

-- 
John Baldwin

                                                                            

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

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

On 2/7/19 9:02 AM, John Baldwin wrote:
> On 2/6/19 9:04 PM, Simon Marchi wrote:
>> On 2019-01-24 12:08, John Baldwin wrote:
>>> On 1/22/19 10:42 AM, 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.
>>>>
>>>> 	* 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 | 146 
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  gdb/fbsd-tdep.h |  10 ++++
>>>>  3 files changed, 166 insertions(+)
>>>>
>>>> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
>>>> index d971d3a653..2e0f72d17b 100644
>>>> --- a/gdb/fbsd-tdep.c
>>>> +++ b/gdb/fbsd-tdep.c
>>>> +
>>>> +/* 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.  */
>>>> +      data->off_linkmap = parse_and_eval_long ("&((Obj_Entry 
>>>> *)0)->linkmap");
>>>> +      data->off_tlsindex = parse_and_eval_long ("&((Obj_Entry 
>>>> *)0)->tlsindex");
>>>> +      data->rtld_offsets_valid = true;
>>>> +      return;
>>>
>>> I'm not really happy about using parse_and_eval_long with an open-coded
>>> equivalent of offsetof() here.  It seems we don't already have existing
>>> functionality for this though?  I think I could use 'lookup_struct' to 
>>> find
>>> the 'struct type *' for 'Obj_Entry', but if I used 
>>> 'lookup_struct_elt_type'
>>> to get the type of an element that doesn't give me anything that has 
>>> the
>>> offset.  We could perhaps instead add a new function 
>>> 'lookup_struct_elt_offset'
>>> that took the element name as a string and figured out the offset.  We 
>>> could
>>> then use this to provide an 'offsetof' builtin for the C language 
>>> perhaps.
>>> However, I suspect that lookup_struct_elt_offset would have to invoke a
>>> language-specific function to do the actual computation (just as ptype 
>>> /o
>>> handling is language-specific).
>>
>> Doesn't parse_and_eval_long also call in language-specific things?  The 
>> expression is parsed according to whatever is the current language, I 
>> suppose.  If needed, I guess we could change temporarily the current 
>> language to C, which is the language the dynamic linker is written in 
>> (until proven otherwise).
> 
> Mmm, that's true.

I chose to use lookup_symbol_by_language to force C.

>> The offset of fields within structures is available (see macro 
>> FIELD_BITPOS).  I haven't looked too deep into it, but it sounds like it 
>> should be possible to implement what you need with that.
> 
> Hmm.  When I looked at the code for ptype /o it seemed to keep a running
> tally for the byte offset.  It wasn't clear to me if perhaps FIELD_BITPOS
> was for bitfields and the bit offset within a byte/word.  However, the
> Linux kernel patch series does have some code to do this.  I could perhaps
> steal from that to implement lookup_struct_elt_offset.  I would probably
> make it support nested fields as well (I think the Linux kernel patches are
> using it for some nested fields).

So this was actually simpler than I expected.  The existing
lookup_struct_elt_type didn't work because you can't work back from the
type to get to the field.  So instead I added a new function that returns
the field and the offset (you have to compute an offset still to handle
anonymous union/struct members) that is fairly similar to the lk_find_field
function in the Linux kernel patchset (and hopefully can be used in place of
that with minimal fuss).  It does return bitpositions that the consumer has
to convert to bytes, but that seems consistent and is easy to fix in the
consumer.  I then used this to replace the parse_and_eval_long above.  I'll
post a v3 with that.  I also reworded the commit message a bit for the debug
file patch you looked at and cleaned up some cruft from lookup_struct_elt_type.

Some other cleanup we might consider would be to reimplement the
lookup_struct_typedef in terms of lookup_struct (or vice versa), and to also
perhaps add a common version of the lk_find_type function from the Linux
kernel patchset that lets you name the structure type either by the tag
("foo" of "struct foo") or by a typedef ("foo_t" from
"typedef struct { } foo_t").

-- 
John Baldwin

                                                                            

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

end of thread, other threads:[~2019-02-09  0:34 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-22 18:43 [PATCH 0/9] Support for thread-local variables on FreeBSD John Baldwin
2019-01-22 18:43 ` [PATCH 1/9] Support the fs_base and gs_base registers on i386 John Baldwin
2019-01-27  4:22   ` Simon Marchi
2019-01-28 17:54     ` John Baldwin
2019-02-02 15:11       ` Simon Marchi
2019-01-22 18:43 ` [PATCH 2/9] Support fs_base and gs_base on FreeBSD/i386 John Baldwin
2019-02-02 15:26   ` Simon Marchi
2019-02-04 19:45     ` John Baldwin
2019-02-05 18:59       ` Simon Marchi
2019-01-22 18:43 ` [PATCH 6/9] Support TLS variables on FreeBSD/amd64 John Baldwin
2019-01-22 18:43 ` [PATCH 7/9] Support TLS variables on FreeBSD/i386 John Baldwin
2019-01-22 18:43 ` [PATCH 9/9] Support TLS variables on FreeBSD/powerpc John Baldwin
2019-01-22 18:43 ` [PATCH 3/9] Handle TLS variable lookups when using separate debug object files John Baldwin
2019-02-02 15:52   ` Simon Marchi
2019-02-04 20:02     ` John Baldwin
2019-02-05 20:06       ` Simon Marchi
2019-02-05 22:21         ` John Baldwin
2019-02-05 22:33           ` John Baldwin
2019-02-07  4:05             ` Simon Marchi
2019-02-07  4:08               ` Simon Marchi
2019-01-22 18:52 ` [PATCH 8/9] Support TLS variables on FreeBSD/riscv John Baldwin
2019-01-27 23:35   ` Andrew Burgess
2019-01-22 18:52 ` [PATCH 5/9] Add a helper function to resolve TLS variable addresses for FreeBSD John Baldwin
2019-01-24 17:09   ` John Baldwin
2019-02-07  5:05     ` Simon Marchi
2019-02-07 17:02       ` John Baldwin
2019-02-09  0:34         ` John Baldwin
2019-01-22 18:52 ` [PATCH 4/9] Add a new gdbarch method to resolve the address of TLS variables John Baldwin

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