[PATCH v2] btrace: kernel address filtering

[Markus Metzger <markus dot t dot metzger at intel dot com>]

For the BTS recording format, we sometimes get a FROM->TO record where the
FROM address lies in the kernel and the TO address lies in user space at
whatever address the user process was resumed.

GDB has a heuristic to filter out such records based on looking at the most
significant bit in the PC.  This works fine for 64-bit systems but it doesn't
always work for 32-bit systems.  Libraries that are loaded at fairly high
addresses might be mistaken for kernel code and branches inside the library
are filtered out.

Change the heuristic to (again heuristically) try to determine the lowest
address in kernel space.  Any PC that is smaller than that should be in
user space.

On today's systems, there should be a symbol "_text" at that address.
Read /proc/kallsyms and search for that symbol.

It is not guaranteed that /proc/kallsyms is readable on all systems.  On
64-bit systems, we fall back to check the most significant bit.  On 32-bit
systems, we refrain from filtering out addresses.

The filtering should really be done by the kernel.  And it soon will be:
https://lkml.org/lkml/2015/8/31/212.

2015-09-08  Markus Metzger <markus.t.metzger@intel.com>

gdb/
	* nat/linux-btrace.h (struct btrace_target_info) <ptr_bits>: Remove.
	* nat/linux-btrace.c: Include filestuff.h and inttypes.h.
	Remove include of sys/utsname.h.
	(linux_determine_kernel_ptr_bits): Remove.
	(linux_determine_kernel_start): New.
	(perf_event_is_kernel_addr): Remove tinfo argument.  Update users.
	Update check.
	(perf_event_skip_bts_record): Remove tinfo argument.  Update users.
	(linux_enable_bts, linux_enable_pt): Remove tinfo->ptr_bits
	initialization.
	* x86-linux-nat.c (x86_linux_enable_btrace): Remove ptr_bits
	assignment.

gdbserver/
	* linux-low.c (linux_low_enable_btrace): Remove.
	(linux_target_ops): Replace linux_low_enable_btrace with
	linux_enable_btrace.
---
 gdb/gdbserver/linux-low.c | 22 +------------
 gdb/nat/linux-btrace.c    | 83 +++++++++++++++++++++++++++++------------------
 gdb/nat/linux-btrace.h    |  5 ---
 gdb/x86-linux-nat.c       |  6 ----
 4 files changed, 52 insertions(+), 64 deletions(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index f4c6029..4256bc5 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -6663,26 +6663,6 @@ linux_qxfer_libraries_svr4 (const char *annex, unsigned char *readbuf,
 
 #ifdef HAVE_LINUX_BTRACE
 
-/* See to_enable_btrace target method.  */
-
-static struct btrace_target_info *
-linux_low_enable_btrace (ptid_t ptid, const struct btrace_config *conf)
-{
-  struct btrace_target_info *tinfo;
-
-  tinfo = linux_enable_btrace (ptid, conf);
-
-  if (tinfo != NULL && tinfo->ptr_bits == 0)
-    {
-      struct thread_info *thread = find_thread_ptid (ptid);
-      struct regcache *regcache = get_thread_regcache (thread, 0);
-
-      tinfo->ptr_bits = register_size (regcache->tdesc, 0) * 8;
-    }
-
-  return tinfo;
-}
-
 /* See to_disable_btrace target method.  */
 
 static int
@@ -6936,7 +6916,7 @@ static struct target_ops linux_target_ops = {
   linux_supports_agent,
 #ifdef HAVE_LINUX_BTRACE
   linux_supports_btrace,
-  linux_low_enable_btrace,
+  linux_enable_btrace,
   linux_low_disable_btrace,
   linux_low_read_btrace,
   linux_low_btrace_conf,
diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c
index 51725ff..08478d8 100644
--- a/gdb/nat/linux-btrace.c
+++ b/gdb/nat/linux-btrace.c
@@ -24,6 +24,9 @@
 #include "common-regcache.h"
 #include "gdb_wait.h"
 #include "x86-cpuid.h"
+#include "filestuff.h"
+
+#include <inttypes.h>
 
 #ifdef HAVE_SYS_SYSCALL_H
 #include <sys/syscall.h>
@@ -36,7 +39,6 @@
 #include "nat/gdb_ptrace.h"
 #include <sys/types.h>
 #include <signal.h>
-#include <sys/utsname.h>
 
 /* A branch trace record in perf_event.  */
 struct perf_event_bts
@@ -189,56 +191,75 @@ perf_event_pt_event_type (int *type)
   return -1;
 }
 
-static int
-linux_determine_kernel_ptr_bits (void)
+/* Try to determine the start address of the Linux kernel.  */
+
+static uint64_t
+linux_determine_kernel_start (void)
 {
-  struct utsname utsn;
-  int errcode;
+  static uint64_t kernel_start;
+  static int cached;
+  FILE *file;
 
-  memset (&utsn, 0, sizeof (utsn));
+  if (cached != 0)
+    return kernel_start;
 
-  errcode = uname (&utsn);
-  if (errcode < 0)
-    return 0;
+  cached = 1;
 
-  /* We only need to handle the 64-bit host case, here.  For 32-bit host,
-     the pointer size can be filled in later based on the inferior.  */
-  if (strcmp (utsn.machine, "x86_64") == 0)
-    return 64;
+  file = gdb_fopen_cloexec ("/proc/kallsyms", "r");
+  if (file == NULL)
+    return kernel_start;
 
-  return 0;
+  while (!feof (file))
+    {
+      char buffer[1024], symbol[8], *line;
+      uint64_t addr;
+      int match;
+
+      line = fgets (buffer, sizeof (buffer), file);
+      if (line == NULL)
+	break;
+
+      match = sscanf (line, "%" SCNx64 " %*[tT] %7s", &addr, symbol);
+      if (match != 2)
+	continue;
+
+      if (strcmp (symbol, "_text") == 0)
+	{
+	  kernel_start = addr;
+	  break;
+	}
+    }
+
+  fclose (file);
+
+  return kernel_start;
 }
 
 /* Check whether an address is in the kernel.  */
 
 static inline int
-perf_event_is_kernel_addr (const struct btrace_target_info *tinfo,
-			   uint64_t addr)
+perf_event_is_kernel_addr (uint64_t addr)
 {
-  uint64_t mask;
-
-  /* If we don't know the size of a pointer, we can't check.  Let's assume it's
-     not a kernel address in this case.  */
-  if (tinfo->ptr_bits == 0)
-    return 0;
+  uint64_t kernel_start;
 
-  /* A bit mask for the most significant bit in an address.  */
-  mask = (uint64_t) 1 << (tinfo->ptr_bits - 1);
+  kernel_start = linux_determine_kernel_start ();
+  if (kernel_start != 0ull)
+    return (addr >= kernel_start);
 
-  /* Check whether the most significant bit in the address is set.  */
-  return (addr & mask) != 0;
+  /* If we don't know the kernel's start address, let's check the most
+     significant bit.  This will work at least for 64-bit kernels.  */
+  return ((addr & (1ull << 63)) != 0);
 }
 
 /* Check whether a perf event record should be skipped.  */
 
 static inline int
-perf_event_skip_bts_record (const struct btrace_target_info *tinfo,
-			    const struct perf_event_bts *bts)
+perf_event_skip_bts_record (const struct perf_event_bts *bts)
 {
   /* The hardware may report branches from kernel into user space.  Branches
      from user into kernel space will be suppressed.  We filter the former to
      provide a consistent branch trace excluding kernel.  */
-  return perf_event_is_kernel_addr (tinfo, bts->from);
+  return perf_event_is_kernel_addr (bts->from);
 }
 
 /* Perform a few consistency checks on a perf event sample record.  This is
@@ -335,7 +356,7 @@ perf_event_read_bts (struct btrace_target_info* tinfo, const uint8_t *begin,
 	  break;
 	}
 
-      if (perf_event_skip_bts_record (tinfo, &psample->bts))
+      if (perf_event_skip_bts_record (&psample->bts))
 	continue;
 
       /* We found a valid sample, so we can complete the current block.  */
@@ -649,7 +670,6 @@ linux_enable_bts (ptid_t ptid, const struct btrace_config_bts *conf)
 
   tinfo = XCNEW (struct btrace_target_info);
   tinfo->ptid = ptid;
-  tinfo->ptr_bits = linux_determine_kernel_ptr_bits ();
 
   tinfo->conf.format = BTRACE_FORMAT_BTS;
   bts = &tinfo->variant.bts;
@@ -782,7 +802,6 @@ linux_enable_pt (ptid_t ptid, const struct btrace_config_pt *conf)
 
   tinfo = XCNEW (struct btrace_target_info);
   tinfo->ptid = ptid;
-  tinfo->ptr_bits = 0;
 
   tinfo->conf.format = BTRACE_FORMAT_PT;
   pt = &tinfo->variant.pt;
diff --git a/gdb/nat/linux-btrace.h b/gdb/nat/linux-btrace.h
index 5ea87a8..042878c 100644
--- a/gdb/nat/linux-btrace.h
+++ b/gdb/nat/linux-btrace.h
@@ -100,11 +100,6 @@ struct btrace_target_info
     struct btrace_tinfo_pt pt;
   } variant;
 #endif /* HAVE_LINUX_PERF_EVENT_H */
-
-  /* The size of a pointer in bits for this thread.
-     The information is used to identify kernel addresses in order to skip
-     records from/to kernel space.  */
-  int ptr_bits;
 };
 
 /* See to_supports_btrace in target.h.  */
diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index fe52c1f..fa5ef30 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -264,12 +264,6 @@ x86_linux_enable_btrace (struct target_ops *self, ptid_t ptid,
     error (_("Could not enable branch tracing for %s: %s."),
 	   target_pid_to_str (ptid), safe_strerror (errno));
 
-  /* Fill in the size of a pointer in bits.  */
-  if (tinfo->ptr_bits == 0)
-    {
-      gdbarch = target_thread_architecture (ptid);
-      tinfo->ptr_bits = gdbarch_ptr_bit (gdbarch);
-    }
   return tinfo;
 }
 
-- 
1.8.3.1