public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 2/4] record: set stop_pc in "record goto" command
  2015-07-08 12:54 [PATCH 1/4] btrace: fix build fail with 32-bit BFD Markus Metzger
  2015-07-08 12:54 ` [PATCH 3/4] ari, btrace: avoid unsigned long long Markus Metzger
@ 2015-07-08 12:54 ` Markus Metzger
  2015-07-08 13:42   ` Pedro Alves
  2015-07-08 12:54 ` [PATCH 4/4] btrace, pt: support new packets Markus Metzger
  2015-07-08 13:42 ` [PATCH 1/4] btrace: fix build fail with 32-bit BFD Pedro Alves
  3 siblings, 1 reply; 13+ messages in thread
From: Markus Metzger @ 2015-07-08 12:54 UTC (permalink / raw)
  To: palves; +Cc: gdb-patches

When navigating in the recorded execution trace via "record goto", we do not
set stop_pc.  This may trigger an internal error in infrun.c when stepping
from that location.  Set it.

(gdb) rec full
(gdb) c
Continuing.

Breakpoint 1, foo (void) at foo.c:42
42             x = y
(gdb) rn
foo (void)
    at foo.c:41
41             y = x
(gdb) rec go end
Go forward to insn number 98724
    at foo.c:42
42             x = y
(gdb) n
infrun.c:2382: internal-error: resume: Assertion `sig != GDB_SIGNAL_0' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)

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

gdb/
	* record-btrace.c (record_btrace_goto_begin, record_btrace_goto_end)
	record_btrace_goto): Set stop_pc.  Call reinit_frame_cache.
	* record-full.c (record_full_goto_entry): Set stop_pc.

testsuite/
	* gdb.btrace/record_goto-step.exp: New.
---
 gdb/record-btrace.c                           |  6 ++++
 gdb/record-full.c                             |  1 +
 gdb/testsuite/gdb.btrace/record_goto-step.exp | 46 +++++++++++++++++++++++++++
 3 files changed, 53 insertions(+)
 create mode 100644 gdb/testsuite/gdb.btrace/record_goto-step.exp

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 969e01b..3870400 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -2227,6 +2227,8 @@ record_btrace_goto_begin (struct target_ops *self)
   btrace_insn_begin (&begin, &tp->btrace);
   record_btrace_set_replay (tp, &begin);
 
+  reinit_frame_cache ();
+  stop_pc = regcache_read_pc (get_current_regcache ());
   print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
 }
 
@@ -2241,6 +2243,8 @@ record_btrace_goto_end (struct target_ops *ops)
 
   record_btrace_set_replay (tp, NULL);
 
+  reinit_frame_cache ();
+  stop_pc = regcache_read_pc (get_current_regcache ());
   print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
 }
 
@@ -2268,6 +2272,8 @@ record_btrace_goto (struct target_ops *self, ULONGEST insn)
 
   record_btrace_set_replay (tp, &it);
 
+  reinit_frame_cache ();
+  stop_pc = regcache_read_pc (get_current_regcache ());
   print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
 }
 
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 43e8be2..1520811 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1869,6 +1869,7 @@ record_full_goto_entry (struct record_full_entry *p)
 
   registers_changed ();
   reinit_frame_cache ();
+  stop_pc = regcache_read_pc (get_current_regcache ());
   print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
 }
 
diff --git a/gdb/testsuite/gdb.btrace/record_goto-step.exp b/gdb/testsuite/gdb.btrace/record_goto-step.exp
new file mode 100644
index 0000000..bbf0b77
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/record_goto-step.exp
@@ -0,0 +1,46 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2013-2015 Free Software Foundation, Inc.
+#
+# Contributed by Intel Corp. <markus.t.metzger@intel.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# check for btrace support
+if { [skip_btrace_tests] } { return -1 }
+
+standard_testfile record_goto.c
+
+# start inferior
+if [prepare_for_testing record_goto-step.exp $testfile $srcfile] {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+set bp [gdb_get_line_number "fun4.3" $srcfile]
+gdb_breakpoint $srcfile:$bp
+
+# record the execution until we hit a breakpoint
+gdb_test_no_output "record btrace"
+gdb_continue_to_breakpoint "cont to $bp" ".*$srcfile:$bp.*"
+
+# reverse-step, then jump to the end of the trace
+gdb_test "reverse-next" ".*fun4\.2.*"
+gdb_test "record goto end" ".*fun4\.3.*"
+
+# test that we can continue stepping
+gdb_test "next" ".*fun4\.4.*"
-- 
1.8.3.1

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

* [PATCH 3/4] ari, btrace: avoid unsigned long long
  2015-07-08 12:54 [PATCH 1/4] btrace: fix build fail with 32-bit BFD Markus Metzger
@ 2015-07-08 12:54 ` Markus Metzger
  2015-07-08 13:40   ` Pedro Alves
  2015-07-08 12:54 ` [PATCH 2/4] record: set stop_pc in "record goto" command Markus Metzger
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Markus Metzger @ 2015-07-08 12:54 UTC (permalink / raw)
  To: palves; +Cc: gdb-patches

Fix the ARI warning about the use of unsigned long long.  We can't use ULONGEST
as this is defined unsigned long on 32-bit systems.  Use unsigned long to hold
the buffer size inside GDB and __u64 when interfacing the kernel.

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

gdb/
	* nat/linux-btrace.c (perf_event_read): Change the type of DATA_HEAD.
	(perf_event_read_all): Change the type of SIZE and DATA_HEAD.
	(perf_event_read_bts): Change the type of SIZE and READ.
	(linux_enable_bts): Change the type of SIZE, PAGES, DATA_SIZE,
	and DATA_OFFSET.  Move DATA_SIZE declaration.  Restrict the buffer size
	to UINT_MAX.  Check for overflows when using DATA_HEAD from the perf
	mmap page.
	(linux_enable_pt): Change the type of PAGES and SIZE.  Restrict the
	buffer size to UINT_MAX.
	(linux_read_bts): Change the type of BUFFER_SIZE, SIZE, DATA_HEAD, and
	DATA_TAIL.
	* nat/linux-btrace.c (struct perf_event_buffer)<size, data_head>
	<last_head>: Change type.
---
 gdb/nat/linux-btrace.c | 70 ++++++++++++++++++++++++++++++++------------------
 gdb/nat/linux-btrace.h |  6 ++---
 2 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/gdb/nat/linux-btrace.c b/gdb/nat/linux-btrace.c
index b6e13d3..cc1e5a5 100644
--- a/gdb/nat/linux-btrace.c
+++ b/gdb/nat/linux-btrace.c
@@ -111,12 +111,13 @@ perf_event_new_data (const struct perf_event_buffer *pev)
    The caller is responsible for freeing the memory.  */
 
 static gdb_byte *
-perf_event_read (const struct perf_event_buffer *pev, unsigned long data_head,
+perf_event_read (const struct perf_event_buffer *pev, __u64 data_head,
 		 unsigned long size)
 {
   const gdb_byte *begin, *end, *start, *stop;
   gdb_byte *buffer;
-  unsigned long data_tail, buffer_size;
+  unsigned long buffer_size;
+  __u64 data_tail;
 
   if (size == 0)
     return NULL;
@@ -151,13 +152,14 @@ static void
 perf_event_read_all (struct perf_event_buffer *pev, gdb_byte **data,
 		     unsigned long *psize)
 {
-  unsigned long data_head, size;
+  unsigned long size;
+  __u64 data_head;
 
   data_head = *pev->data_head;
 
   size = pev->size;
   if (data_head < size)
-    size = data_head;
+    size = (unsigned long) data_head;
 
   *data = perf_event_read (pev, data_head, size);
   *psize = size;
@@ -270,11 +272,11 @@ perf_event_sample_ok (const struct perf_event_sample *sample)
 static VEC (btrace_block_s) *
 perf_event_read_bts (struct btrace_target_info* tinfo, const uint8_t *begin,
 		     const uint8_t *end, const uint8_t *start,
-		     unsigned long long size)
+		     unsigned long size)
 {
   VEC (btrace_block_s) *btrace = NULL;
   struct perf_event_sample sample;
-  unsigned long long read = 0;
+  unsigned long read = 0;
   struct btrace_block block = { 0, 0 };
   struct regcache *regcache;
 
@@ -642,7 +644,8 @@ linux_enable_bts (ptid_t ptid, const struct btrace_config_bts *conf)
   struct perf_event_mmap_page *header;
   struct btrace_target_info *tinfo;
   struct btrace_tinfo_bts *bts;
-  unsigned long long size, pages, data_offset, data_size;
+  unsigned long size, pages;
+  __u64 data_offset;
   int pid, pg;
 
   tinfo = xzalloc (sizeof (*tinfo));
@@ -674,7 +677,7 @@ linux_enable_bts (ptid_t ptid, const struct btrace_config_bts *conf)
     goto err_out;
 
   /* Convert the requested size in bytes to pages (rounding up).  */
-  pages = (((unsigned long long) conf->size) + PAGE_SIZE - 1) / PAGE_SIZE;
+  pages = (((unsigned long) conf->size) + PAGE_SIZE - 1) / PAGE_SIZE;
   /* We need at least one page.  */
   if (pages == 0)
     pages = 1;
@@ -692,12 +695,13 @@ linux_enable_bts (ptid_t ptid, const struct btrace_config_bts *conf)
       size_t length;
 
       size = pages * PAGE_SIZE;
-      length = size + PAGE_SIZE;
 
-      /* Check for overflows.  */
-      if ((unsigned long long) length < size)
+      /* Don't ask for more than we can represent in the configuration.  */
+      if (UINT_MAX < size)
 	continue;
 
+      length = size + PAGE_SIZE;
+
       /* The number of pages we request needs to be a power of two.  */
       header = mmap (NULL, length, PROT_READ, MAP_SHARED, bts->file, 0);
       if (header != MAP_FAILED)
@@ -708,23 +712,33 @@ linux_enable_bts (ptid_t ptid, const struct btrace_config_bts *conf)
     goto err_file;
 
   data_offset = PAGE_SIZE;
-  data_size = size;
 
 #if defined (PERF_ATTR_SIZE_VER5)
   if (offsetof (struct perf_event_mmap_page, data_size) <= header->size)
     {
+      __u64 data_size;
+
       data_offset = header->data_offset;
       data_size = header->data_size;
+
+      size = (unsigned int) data_size;
+
+      /* Check for overflows.  */
+      if ((__u64) size != data_size)
+	{
+	  munmap ((void *) header, (size_t) size + PAGE_SIZE);
+	  goto err_file;
+	}
     }
 #endif /* defined (PERF_ATTR_SIZE_VER5) */
 
   bts->header = header;
   bts->bts.mem = ((const uint8_t *) header) + data_offset;
-  bts->bts.size = data_size;
+  bts->bts.size = size;
   bts->bts.data_head = &header->data_head;
-  bts->bts.last_head = 0;
+  bts->bts.last_head = 0ull;
 
-  tinfo->conf.bts.size = data_size;
+  tinfo->conf.bts.size = (unsigned int) size;
   return tinfo;
 
  err_file:
@@ -746,7 +760,7 @@ linux_enable_pt (ptid_t ptid, const struct btrace_config_pt *conf)
   struct perf_event_mmap_page *header;
   struct btrace_target_info *tinfo;
   struct btrace_tinfo_pt *pt;
-  unsigned long long pages, size;
+  unsigned long pages, size;
   int pid, pg, errcode, type;
 
   if (conf->size == 0)
@@ -788,7 +802,7 @@ linux_enable_pt (ptid_t ptid, const struct btrace_config_pt *conf)
   header->aux_offset = header->data_offset + header->data_size;
 
   /* Convert the requested size in bytes to pages (rounding up).  */
-  pages = (((unsigned long long) conf->size) + PAGE_SIZE - 1) / PAGE_SIZE;
+  pages = (((unsigned long) conf->size) + PAGE_SIZE - 1) / PAGE_SIZE;
   /* We need at least one page.  */
   if (pages == 0)
     pages = 1;
@@ -806,12 +820,12 @@ linux_enable_pt (ptid_t ptid, const struct btrace_config_pt *conf)
       size_t length;
 
       size = pages * PAGE_SIZE;
-      length = size;
 
-      /* Check for overflows.  */
-      if ((unsigned long long) length < size)
+      /* Don't ask for more than we can represent in the configuration.  */
+      if (UINT_MAX < size)
 	continue;
 
+      length = size;
       header->aux_size = size;
 
       pt->pt.mem = mmap (NULL, length, PROT_READ, MAP_SHARED, pt->file,
@@ -827,7 +841,7 @@ linux_enable_pt (ptid_t ptid, const struct btrace_config_pt *conf)
   pt->pt.size = size;
   pt->pt.data_head = &header->aux_head;
 
-  tinfo->conf.pt.size = size;
+  tinfo->conf.pt.size = (unsigned int) size;
   return tinfo;
 
  err_conf:
@@ -938,7 +952,8 @@ linux_read_bts (struct btrace_data_bts *btrace,
 {
   struct perf_event_buffer *pevent;
   const uint8_t *begin, *end, *start;
-  unsigned long long data_head, data_tail, buffer_size, size;
+  unsigned long buffer_size, size;
+  __u64 data_head, data_tail;
   unsigned int retries = 5;
 
   pevent = &tinfo->variant.bts.bts;
@@ -961,6 +976,8 @@ linux_read_bts (struct btrace_data_bts *btrace,
 
       if (type == BTRACE_READ_DELTA)
 	{
+	  __u64 data_size;
+
 	  /* Determine the number of bytes to read and check for buffer
 	     overflows.  */
 
@@ -971,9 +988,12 @@ linux_read_bts (struct btrace_data_bts *btrace,
 	    return BTRACE_ERR_OVERFLOW;
 
 	  /* If the buffer is smaller than the trace delta, we overflowed.  */
-	  size = data_head - data_tail;
-	  if (buffer_size < size)
+	  data_size = data_head - data_tail;
+	  if (buffer_size < data_size)
 	    return BTRACE_ERR_OVERFLOW;
+
+	  /* DATA_SIZE <= BUFFER_SIZE and therefore fits into a long.  */
+	  size = (unsigned long) data_size;
 	}
       else
 	{
@@ -982,7 +1002,7 @@ linux_read_bts (struct btrace_data_bts *btrace,
 
 	  /* Adjust the size if the buffer has not overflowed, yet.  */
 	  if (data_head < size)
-	    size = data_head;
+	    size = (unsigned long) data_head;
 	}
 
       /* Data_head keeps growing; the buffer itself is circular.  */
diff --git a/gdb/nat/linux-btrace.h b/gdb/nat/linux-btrace.h
index b680bf5..5fcaf79 100644
--- a/gdb/nat/linux-btrace.h
+++ b/gdb/nat/linux-btrace.h
@@ -38,13 +38,13 @@ struct perf_event_buffer
   const uint8_t *mem;
 
   /* The size of the mapped memory in bytes.  */
-  unsigned long long size;
+  unsigned long size;
 
   /* A pointer to the data_head field for this buffer. */
-  volatile unsigned long long *data_head;
+  volatile __u64 *data_head;
 
   /* The data_head value from the last read.  */
-  unsigned long long last_head;
+  __u64 last_head;
 };
 
 /* Branch trace target information for BTS tracing.  */
-- 
1.8.3.1

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

* [PATCH 4/4] btrace, pt: support new packets
  2015-07-08 12:54 [PATCH 1/4] btrace: fix build fail with 32-bit BFD Markus Metzger
  2015-07-08 12:54 ` [PATCH 3/4] ari, btrace: avoid unsigned long long Markus Metzger
  2015-07-08 12:54 ` [PATCH 2/4] record: set stop_pc in "record goto" command Markus Metzger
@ 2015-07-08 12:54 ` Markus Metzger
  2015-07-08 13:45   ` Pedro Alves
  2015-07-08 13:42 ` [PATCH 1/4] btrace: fix build fail with 32-bit BFD Pedro Alves
  3 siblings, 1 reply; 13+ messages in thread
From: Markus Metzger @ 2015-07-08 12:54 UTC (permalink / raw)
  To: palves; +Cc: gdb-patches

Add support for dumping new Intel(R) Processor Trace packets in the
"maint btrace packet-history" command.

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

gdb/
	* btrace.c (pt_print_packet): Print stop, vmcs, tma, mtc, cyc, and
	mnt packets.
---
 gdb/btrace.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 1618e55..731d237 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -2309,7 +2309,8 @@ pt_print_packet (const struct pt_packet *packet)
       break;
 
     case ppt_pip:
-      printf_unfiltered (("pip %" PRIx64 ""), packet->payload.pip.cr3);
+      printf_unfiltered (("pip %" PRIx64 "%s"), packet->payload.pip.cr3,
+			 packet->payload.pip.nr ? (" nr") : (""));
       break;
 
     case ppt_tsc:
@@ -2349,6 +2350,30 @@ pt_print_packet (const struct pt_packet *packet)
       printf_unfiltered (("ovf"));
       break;
 
+    case ppt_stop:
+      printf_unfiltered (("stop"));
+      break;
+
+    case ppt_vmcs:
+      printf_unfiltered (("vmcs %" PRIx64 ""), packet->payload.vmcs.base);
+      break;
+
+    case ppt_tma:
+      printf_unfiltered (("tma %x %x"), packet->payload.tma.ctc,
+			 packet->payload.tma.fc);
+      break;
+
+    case ppt_mtc:
+      printf_unfiltered (("mtc %x"), packet->payload.mtc.ctc);
+      break;
+
+    case ppt_cyc:
+      printf_unfiltered (("cyc %" PRIx64 ""), packet->payload.cyc.value);
+      break;
+
+    case ppt_mnt:
+      printf_unfiltered (("mnt %" PRIx64 ""), packet->payload.mnt.payload);
+      break;
     }
 }
 
-- 
1.8.3.1

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

* [PATCH 1/4] btrace: fix build fail with 32-bit BFD
@ 2015-07-08 12:54 Markus Metzger
  2015-07-08 12:54 ` [PATCH 3/4] ari, btrace: avoid unsigned long long Markus Metzger
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Markus Metzger @ 2015-07-08 12:54 UTC (permalink / raw)
  To: palves; +Cc: gdb-patches

When compiling GDB with 32-bit BFD, the build fails with:

In file included from btrace.h:33:0,
                 from btrace.c:23:
/usr/include/intel-pt.h:1643:51: note: expected 'int (*)(uint8_t *, size_t,
 const struct pt_asid *, uint64_t, void *)' but argument is of type 'int
 (*)(gdb_byte *, size_t, const struct pt_asid *, CORE_ADDR, void *)' extern
 pt_export int pt_image_set_callback(struct pt_image *image, ^

Fix it.

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

gdb/
	* btrace.c (btrace_pt_readmem_callback): Change type of PC argument.
---
 gdb/btrace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index b924e7d..1618e55 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -824,14 +824,14 @@ ftrace_add_pt (struct pt_insn_decoder *decoder,
 
 static int
 btrace_pt_readmem_callback (gdb_byte *buffer, size_t size,
-			    const struct pt_asid *asid, CORE_ADDR pc,
+			    const struct pt_asid *asid, uint64_t pc,
 			    void *context)
 {
   int errcode;
 
   TRY
     {
-      errcode = target_read_code (pc, buffer, size);
+      errcode = target_read_code ((CORE_ADDR) pc, buffer, size);
       if (errcode != 0)
 	return -pte_nomap;
     }
-- 
1.8.3.1

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

* Re: [PATCH 3/4] ari, btrace: avoid unsigned long long
  2015-07-08 12:54 ` [PATCH 3/4] ari, btrace: avoid unsigned long long Markus Metzger
@ 2015-07-08 13:40   ` Pedro Alves
  2015-07-08 14:05     ` Metzger, Markus T
  0 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2015-07-08 13:40 UTC (permalink / raw)
  To: Markus Metzger; +Cc: gdb-patches

On 07/08/2015 01:54 PM, Markus Metzger wrote:
> Fix the ARI warning about the use of unsigned long long.  We can't use ULONGEST
> as this is defined unsigned long on 32-bit systems.

Hmm, it is?

I'm looking at common/common-types.h and not seeing it.

> Use unsigned long to hold
> the buffer size inside GDB and __u64 when interfacing the kernel.
> 

Thanks,
Pedro Alves

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

* Re: [PATCH 1/4] btrace: fix build fail with 32-bit BFD
  2015-07-08 12:54 [PATCH 1/4] btrace: fix build fail with 32-bit BFD Markus Metzger
                   ` (2 preceding siblings ...)
  2015-07-08 12:54 ` [PATCH 4/4] btrace, pt: support new packets Markus Metzger
@ 2015-07-08 13:42 ` Pedro Alves
  2015-07-13  7:39   ` Metzger, Markus T
  3 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2015-07-08 13:42 UTC (permalink / raw)
  To: Markus Metzger; +Cc: gdb-patches

On 07/08/2015 01:54 PM, Markus Metzger wrote:
> When compiling GDB with 32-bit BFD, the build fails with:
> 
> In file included from btrace.h:33:0,
>                  from btrace.c:23:
> /usr/include/intel-pt.h:1643:51: note: expected 'int (*)(uint8_t *, size_t,
>  const struct pt_asid *, uint64_t, void *)' but argument is of type 'int
>  (*)(gdb_byte *, size_t, const struct pt_asid *, CORE_ADDR, void *)' extern
>  pt_export int pt_image_set_callback(struct pt_image *image, ^
> 
> Fix it.
> 
> 2015-07-08  Markus Metzger <markus.t.metzger@intel.com>
> 
> gdb/
> 	* btrace.c (btrace_pt_readmem_callback): Change type of PC argument.

OK.

Thanks,
Pedro Alves

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

* Re: [PATCH 2/4] record: set stop_pc in "record goto" command
  2015-07-08 12:54 ` [PATCH 2/4] record: set stop_pc in "record goto" command Markus Metzger
@ 2015-07-08 13:42   ` Pedro Alves
  2015-07-08 15:08     ` Metzger, Markus T
  0 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2015-07-08 13:42 UTC (permalink / raw)
  To: Markus Metzger; +Cc: gdb-patches

On 07/08/2015 01:54 PM, Markus Metzger wrote:

> 2015-07-08  Markus Metzger <markus.t.metzger@intel.com>
> 
> gdb/
> 	* record-btrace.c (record_btrace_goto_begin, record_btrace_goto_end)
> 	record_btrace_goto): Set stop_pc.  Call reinit_frame_cache.
> 	* record-full.c (record_full_goto_entry): Set stop_pc.
> 
> testsuite/
> 	* gdb.btrace/record_goto-step.exp: New.
> ---
>  gdb/record-btrace.c                           |  6 ++++
>  gdb/record-full.c                             |  1 +
>  gdb/testsuite/gdb.btrace/record_goto-step.exp | 46 +++++++++++++++++++++++++++
>  3 files changed, 53 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.btrace/record_goto-step.exp
> 
> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> index 969e01b..3870400 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -2227,6 +2227,8 @@ record_btrace_goto_begin (struct target_ops *self)
>    btrace_insn_begin (&begin, &tp->btrace);
>    record_btrace_set_replay (tp, &begin);
>  
> +  reinit_frame_cache ();
> +  stop_pc = regcache_read_pc (get_current_regcache ());

I think you should do this within record_btrace_set_replay,
instead of doing the same in several places.

Also, I don't think you need the reinit_frame_cache call, as the
registers_changed_ptid calls in
record_btrace_set_replay/record_btrace_stop_replaying take care of
it already.

Thanks,
Pedro Alves

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

* Re: [PATCH 4/4] btrace, pt: support new packets
  2015-07-08 12:54 ` [PATCH 4/4] btrace, pt: support new packets Markus Metzger
@ 2015-07-08 13:45   ` Pedro Alves
  0 siblings, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2015-07-08 13:45 UTC (permalink / raw)
  To: Markus Metzger; +Cc: gdb-patches

On 07/08/2015 01:54 PM, Markus Metzger wrote:
> Add support for dumping new Intel(R) Processor Trace packets in the
> "maint btrace packet-history" command.
> 
> 2015-07-08  Markus Metzger <markus.t.metzger@intel.com>
> 
> gdb/
> 	* btrace.c (pt_print_packet): Print stop, vmcs, tma, mtc, cyc, and
> 	mnt packets.

OK.

Thanks,
Pedro Alves

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

* RE: [PATCH 3/4] ari, btrace: avoid unsigned long long
  2015-07-08 13:40   ` Pedro Alves
@ 2015-07-08 14:05     ` Metzger, Markus T
  0 siblings, 0 replies; 13+ messages in thread
From: Metzger, Markus T @ 2015-07-08 14:05 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Pedro Alves
> Sent: Wednesday, July 8, 2015 3:41 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 3/4] ari, btrace: avoid unsigned long long
> 
> On 07/08/2015 01:54 PM, Markus Metzger wrote:
> > Fix the ARI warning about the use of unsigned long long.  We can't use
> ULONGEST
> > as this is defined unsigned long on 32-bit systems.
> 
> Hmm, it is?
> 
> I'm looking at common/common-types.h and not seeing it.

Sorry, I confused this with CORE_ADDR.  The issue here is that
it is defined "unsigned long" on 64-bit systems, which is incompatible
with __u64 defined as "unsigned long long".

I'll update the commit message.

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Prof. Dr. Hermann Eul
Chairperson of the Supervisory Board: Tiffany Doon Silva
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

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

* RE: [PATCH 2/4] record: set stop_pc in "record goto" command
  2015-07-08 13:42   ` Pedro Alves
@ 2015-07-08 15:08     ` Metzger, Markus T
  2015-07-08 15:11       ` Pedro Alves
  0 siblings, 1 reply; 13+ messages in thread
From: Metzger, Markus T @ 2015-07-08 15:08 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Wednesday, July 8, 2015 3:42 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 2/4] record: set stop_pc in "record goto" command
> 
> On 07/08/2015 01:54 PM, Markus Metzger wrote:
> 
> > 2015-07-08  Markus Metzger <markus.t.metzger@intel.com>
> >
> > gdb/
> > 	* record-btrace.c (record_btrace_goto_begin,
> record_btrace_goto_end)
> > 	record_btrace_goto): Set stop_pc.  Call reinit_frame_cache.
> > 	* record-full.c (record_full_goto_entry): Set stop_pc.
> >
> > testsuite/
> > 	* gdb.btrace/record_goto-step.exp: New.
> > ---
> >  gdb/record-btrace.c                           |  6 ++++
> >  gdb/record-full.c                             |  1 +
> >  gdb/testsuite/gdb.btrace/record_goto-step.exp | 46
> +++++++++++++++++++++++++++
> >  3 files changed, 53 insertions(+)
> >  create mode 100644 gdb/testsuite/gdb.btrace/record_goto-step.exp
> >
> > diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> > index 969e01b..3870400 100644
> > --- a/gdb/record-btrace.c
> > +++ b/gdb/record-btrace.c
> > @@ -2227,6 +2227,8 @@ record_btrace_goto_begin (struct target_ops
> *self)
> >    btrace_insn_begin (&begin, &tp->btrace);
> >    record_btrace_set_replay (tp, &begin);
> >
> > +  reinit_frame_cache ();
> > +  stop_pc = regcache_read_pc (get_current_regcache ());
> 
> I think you should do this within record_btrace_set_replay,
> instead of doing the same in several places.

OK.  I also moved the call to print_stack_frame.


> Also, I don't think you need the reinit_frame_cache call, as the
> registers_changed_ptid calls in
> record_btrace_set_replay/record_btrace_stop_replaying take care of
> it already.

OK.

I'll reorder the patches, commit the approved ones, and send updated versions
of the two you had comments on.  Is that OK with you?

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Prof. Dr. Hermann Eul
Chairperson of the Supervisory Board: Tiffany Doon Silva
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

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

* Re: [PATCH 2/4] record: set stop_pc in "record goto" command
  2015-07-08 15:08     ` Metzger, Markus T
@ 2015-07-08 15:11       ` Pedro Alves
  0 siblings, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2015-07-08 15:11 UTC (permalink / raw)
  To: Metzger, Markus T; +Cc: gdb-patches

On 07/08/2015 04:07 PM, Metzger, Markus T wrote:

> I'll reorder the patches, commit the approved ones, and send updated versions
> of the two you had comments on.  Is that OK with you?

Sure, that's fine.

Thanks,
Pedro Alves

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

* RE: [PATCH 1/4] btrace: fix build fail with 32-bit BFD
  2015-07-08 13:42 ` [PATCH 1/4] btrace: fix build fail with 32-bit BFD Pedro Alves
@ 2015-07-13  7:39   ` Metzger, Markus T
  2015-07-13  9:39     ` Pedro Alves
  0 siblings, 1 reply; 13+ messages in thread
From: Metzger, Markus T @ 2015-07-13  7:39 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Joel Brobecker (brobecker@adacore.com)

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Wednesday, July 8, 2015 3:42 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH 1/4] btrace: fix build fail with 32-bit BFD
> 
> On 07/08/2015 01:54 PM, Markus Metzger wrote:
> > When compiling GDB with 32-bit BFD, the build fails with:
> >
> > In file included from btrace.h:33:0,
> >                  from btrace.c:23:
> > /usr/include/intel-pt.h:1643:51: note: expected 'int (*)(uint8_t *, size_t,
> >  const struct pt_asid *, uint64_t, void *)' but argument is of type 'int
> >  (*)(gdb_byte *, size_t, const struct pt_asid *, CORE_ADDR, void *)' extern
> >  pt_export int pt_image_set_callback(struct pt_image *image, ^
> >
> > Fix it.
> >
> > 2015-07-08  Markus Metzger <markus.t.metzger@intel.com>
> >
> > gdb/
> > 	* btrace.c (btrace_pt_readmem_callback): Change type of PC
> argument.
> 
> OK.

Would this make sense for GDB 7.10, as well?

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Prof. Dr. Hermann Eul
Chairperson of the Supervisory Board: Tiffany Doon Silva
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

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

* Re: [PATCH 1/4] btrace: fix build fail with 32-bit BFD
  2015-07-13  7:39   ` Metzger, Markus T
@ 2015-07-13  9:39     ` Pedro Alves
  0 siblings, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2015-07-13  9:39 UTC (permalink / raw)
  To: Metzger, Markus T; +Cc: gdb-patches, Joel Brobecker (brobecker@adacore.com)

On 07/13/2015 08:38 AM, Metzger, Markus T wrote:

>>> 2015-07-08  Markus Metzger <markus.t.metzger@intel.com>
>>>
>>> gdb/
>>> 	* btrace.c (btrace_pt_readmem_callback): Change type of PC
>> argument.
>>
>> OK.
> 
> Would this make sense for GDB 7.10, as well?

Fine with me.

Thanks,
Pedro Alves

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

end of thread, other threads:[~2015-07-13  9:39 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-08 12:54 [PATCH 1/4] btrace: fix build fail with 32-bit BFD Markus Metzger
2015-07-08 12:54 ` [PATCH 3/4] ari, btrace: avoid unsigned long long Markus Metzger
2015-07-08 13:40   ` Pedro Alves
2015-07-08 14:05     ` Metzger, Markus T
2015-07-08 12:54 ` [PATCH 2/4] record: set stop_pc in "record goto" command Markus Metzger
2015-07-08 13:42   ` Pedro Alves
2015-07-08 15:08     ` Metzger, Markus T
2015-07-08 15:11       ` Pedro Alves
2015-07-08 12:54 ` [PATCH 4/4] btrace, pt: support new packets Markus Metzger
2015-07-08 13:45   ` Pedro Alves
2015-07-08 13:42 ` [PATCH 1/4] btrace: fix build fail with 32-bit BFD Pedro Alves
2015-07-13  7:39   ` Metzger, Markus T
2015-07-13  9:39     ` Pedro Alves

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