[PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames

[PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames

[Markus Metzger]

Following the practice in skip_artificial_frames, also use get_prev_frame_always
instead of get_prev_frame in skip_tailcall_frames.

2016-02-05  Markus Metzger  <markus.t.metzger@intel.com>

gdb/
	* frame.c (skip_tailcall_frames): Call get_prev_frame_always.
---
 gdb/frame.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index b7832c7..6ab8834 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -443,8 +443,12 @@ skip_artificial_frames (struct frame_info *frame)
 struct frame_info *
 skip_tailcall_frames (struct frame_info *frame)
 {
+  /* Note we use get_prev_frame_always, and not get_prev_frame.  The
+     latter will truncate the frame chain, leading to this function
+     unintentionally returning a null_frame_id (e.g., when the user
+     sets a backtrace limit).  */
   while (get_frame_type (frame) == TAILCALL_FRAME)
-    frame = get_prev_frame (frame);
+    frame = get_prev_frame_always (frame);
 
   return frame;
 }
-- 
1.8.3.1

[PATCH v2 1/3] frame: add skip_tailcall_frames

[Markus Metzger]

Add a new function skip_tailcall_frames to skip TAILCALL_FRAME frames.

2016-02-05  Markus Metzger  <markus.t.metzger@intel.com>

gdb/
	* frame.h (skip_tailcall_frames): New.
	* frame.c (skip_tailcall_frames): New.
	(frame_pop): Call skip_tailcall_frames.
	* infcmd.c: Call skip_tailcall_frames.
---
 gdb/frame.c  | 14 ++++++++++++--
 gdb/frame.h  |  4 ++++
 gdb/infcmd.c |  3 +--
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/gdb/frame.c b/gdb/frame.c
index 48c9b33..b7832c7 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -438,6 +438,17 @@ skip_artificial_frames (struct frame_info *frame)
   return frame;
 }
 
+/* See frame.h.  */
+
+struct frame_info *
+skip_tailcall_frames (struct frame_info *frame)
+{
+  while (get_frame_type (frame) == TAILCALL_FRAME)
+    frame = get_prev_frame (frame);
+
+  return frame;
+}
+
 /* Compute the frame's uniq ID that can be used to, later, re-find the
    frame.  */
 
@@ -972,8 +983,7 @@ frame_pop (struct frame_info *this_frame)
 
   /* Ignore TAILCALL_FRAME type frames, they were executed already before
      entering THISFRAME.  */
-  while (get_frame_type (prev_frame) == TAILCALL_FRAME)
-    prev_frame = get_prev_frame (prev_frame);
+  prev_frame = skip_tailcall_frames (prev_frame);
 
   /* Make a copy of all the register values unwound from this frame.
      Save them in a scratch buffer so that there isn't a race between
diff --git a/gdb/frame.h b/gdb/frame.h
index 2e05dfa..7e8b01e 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -820,5 +820,9 @@ extern int frame_unwinder_is (struct frame_info *fi,
 
 extern enum language get_frame_language (struct frame_info *frame);
 
+/* Return the first non-tailcall frame above FRAME or FRAME if it is not a
+   tailcall frame.  */
+
+extern struct frame_info *skip_tailcall_frames (struct frame_info *frame);
 
 #endif /* !defined (FRAME_H)  */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index df13896..930dc61 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2002,8 +2002,7 @@ finish_command (char *arg, int from_tty)
 
   /* Ignore TAILCALL_FRAME type frames, they were executed already before
      entering THISFRAME.  */
-  while (get_frame_type (frame) == TAILCALL_FRAME)
-    frame = get_prev_frame (frame);
+  frame = skip_tailcall_frames (frame);
 
   /* Find the function we will return from.  */
 
-- 
1.8.3.1

[PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Markus Metzger]

In skip_artificial_frames we repeatedly call get_prev_frame_always until we get
a non-inline and non-tailcall frame assuming that there must be such a frame
eventually.

For record targets, however, we may have a frame chain that consists only of
artificial frames.  This leads to a crash in get_frame_type when dereferencing a
NULL frame pointer.

Change skip_artificial_frames and skip_tailcall_frames to return NULL in such a
case and modify each caller to cope with a NULL return.

In infcmd further move the skip_tailcall_frames call to the forward-stepping
case since we don't need a frame for reverse execution and we don't want to fail
because of that.  Reverse-finish does make sense for a tailcall frame.

2016-02-05  Markus Metzger  <markus.t.metzger@intel.com>

gdb/
	* frame.h (skip_tailcall_frames): Update comment.
	* frame.c (skip_artificial_frames, skip_tailcall_frames): Return NULL
	if only	artificial frames are found.  Update comment.
	(frame_unwind_caller_id): Check NULL return.
	(frame_unwind_pc): Throw NOT_AVAILABLE_ERROR if frame argument is NULL.
	(frame_pop): Add an error if only tailcall frames are found.
	(frame_unwind_caller_arch): Use argument frame's gdbarch if only
	 artificial	frames are found.
	* infcmd.c (finish_command): Move tailcall-chasing loop into forward-
	execution case.  Add an error if only tailcall frames are found.

testsuite/
	* gdb.btrace/tailcall-only.exp: New.
	* gdb.btrace/tailcall-only.c: New.
	* gdb.btrace/x86_64-tailcall-only.S: New.
	* gdb.btrace/i686-tailcall-only.S: New.
---
 gdb/frame.c                                     |  53 ++-
 gdb/frame.h                                     |   3 +-
 gdb/infcmd.c                                    |  15 +-
 gdb/testsuite/gdb.btrace/i686-tailcall-only.S   | 447 ++++++++++++++++++++++++
 gdb/testsuite/gdb.btrace/tailcall-only.c        |  53 +++
 gdb/testsuite/gdb.btrace/tailcall-only.exp      |  93 +++++
 gdb/testsuite/gdb.btrace/x86_64-tailcall-only.S | 446 +++++++++++++++++++++++
 7 files changed, 1091 insertions(+), 19 deletions(-)
 create mode 100644 gdb/testsuite/gdb.btrace/i686-tailcall-only.S
 create mode 100644 gdb/testsuite/gdb.btrace/tailcall-only.c
 create mode 100644 gdb/testsuite/gdb.btrace/tailcall-only.exp
 create mode 100644 gdb/testsuite/gdb.btrace/x86_64-tailcall-only.S

diff --git a/gdb/frame.c b/gdb/frame.c
index 6ab8834..cea7003 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -420,7 +420,8 @@ fprint_frame (struct ui_file *file, struct frame_info *fi)
 
 /* Given FRAME, return the enclosing frame as found in real frames read-in from
    inferior memory.  Skip any previous frames which were made up by GDB.
-   Return the original frame if no immediate previous frames exist.  */
+   Return FRAME if FRAME is a non-artificial frame.
+   Return NULL if FRAME is NULL or the start of an artificial-only chain. */
 
 static struct frame_info *
 skip_artificial_frames (struct frame_info *frame)
@@ -428,11 +429,13 @@ skip_artificial_frames (struct frame_info *frame)
   /* Note we use get_prev_frame_always, and not get_prev_frame.  The
      latter will truncate the frame chain, leading to this function
      unintentionally returning a null_frame_id (e.g., when the user
-     sets a backtrace limit).  This is safe, because as these frames
-     are made up by GDB, there must be a real frame in the chain
-     below.  */
-  while (get_frame_type (frame) == INLINE_FRAME
-	 || get_frame_type (frame) == TAILCALL_FRAME)
+     sets a backtrace limit).
+
+     Note that for record targets we may get a frame chain that consists
+     of artificial frames only.  */
+  while (frame != NULL &&
+	 (get_frame_type (frame) == INLINE_FRAME
+	  || get_frame_type (frame) == TAILCALL_FRAME))
     frame = get_prev_frame_always (frame);
 
   return frame;
@@ -446,8 +449,11 @@ skip_tailcall_frames (struct frame_info *frame)
   /* Note we use get_prev_frame_always, and not get_prev_frame.  The
      latter will truncate the frame chain, leading to this function
      unintentionally returning a null_frame_id (e.g., when the user
-     sets a backtrace limit).  */
-  while (get_frame_type (frame) == TAILCALL_FRAME)
+     sets a backtrace limit).
+
+     Note that for record targets we may get a frame chain that consists
+     of artificial frames only.  */
+  while (frame != NULL && get_frame_type (frame) == TAILCALL_FRAME)
     frame = get_prev_frame_always (frame);
 
   return frame;
@@ -511,11 +517,12 @@ frame_unwind_caller_id (struct frame_info *next_frame)
      requests the frame ID of "main()"s caller.  */
 
   next_frame = skip_artificial_frames (next_frame);
-  this_frame = get_prev_frame_always (next_frame);
-  if (this_frame)
-    return get_frame_id (skip_artificial_frames (this_frame));
-  else
+  if (next_frame == NULL)
     return null_frame_id;
+
+  this_frame = get_prev_frame_always (next_frame);
+  this_frame = skip_artificial_frames (this_frame);
+  return get_frame_id (this_frame);
 }
 
 const struct frame_id null_frame_id = { 0 }; /* All zeros.  */
@@ -795,6 +802,9 @@ frame_find_by_id (struct frame_id id)
 static CORE_ADDR
 frame_unwind_pc (struct frame_info *this_frame)
 {
+  if (this_frame == NULL)
+    throw_error (NOT_AVAILABLE_ERROR, _("PC not available"));
+
   if (this_frame->prev_pc.status == CC_UNKNOWN)
     {
       if (gdbarch_unwind_pc_p (frame_unwind_arch (this_frame)))
@@ -989,6 +999,10 @@ frame_pop (struct frame_info *this_frame)
      entering THISFRAME.  */
   prev_frame = skip_tailcall_frames (prev_frame);
 
+  /* We cannot pop tailcall frames.  */
+  if (prev_frame == NULL)
+    error (_("Cannot pop a tailcall frame."));
+
   /* Make a copy of all the register values unwound from this frame.
      Save them in a scratch buffer so that there isn't a race between
      trying to extract the old values from the current regcache while
@@ -2575,7 +2589,20 @@ frame_unwind_arch (struct frame_info *next_frame)
 struct gdbarch *
 frame_unwind_caller_arch (struct frame_info *next_frame)
 {
-  return frame_unwind_arch (skip_artificial_frames (next_frame));
+  struct frame_info *caller;
+
+  /* If the frame chain consists only of artificial frames, use NEXT_FRAME's.
+
+     For tailcall frames, we (i.e. the DWARF frame unwinder) assume that they
+     have the gdbarch of the bottom (callee) frame.  If NEXT_FRAME is an
+     artificial frame, as well, we should drill down to the bottom in
+     frame_unwind_arch either directly via the tailcall unwinder or via a chain
+     of get_frame_arch calls.  */
+  caller = skip_artificial_frames (next_frame);
+  if (caller == NULL)
+    get_frame_arch (next_frame);
+
+  return frame_unwind_arch (next_frame);
 }
 
 /* Gets the language of FRAME.  */
diff --git a/gdb/frame.h b/gdb/frame.h
index 7e8b01e..7c6306d 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -821,7 +821,8 @@ extern int frame_unwinder_is (struct frame_info *fi,
 extern enum language get_frame_language (struct frame_info *frame);
 
 /* Return the first non-tailcall frame above FRAME or FRAME if it is not a
-   tailcall frame.  */
+   tailcall frame.  Return NULL if FRAME is NULL or the start of a tailcall-only
+   chain.  */
 
 extern struct frame_info *skip_tailcall_frames (struct frame_info *frame);
 
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 930dc61..938f8fa 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2000,10 +2000,6 @@ finish_command (char *arg, int from_tty)
       return;
     }
 
-  /* Ignore TAILCALL_FRAME type frames, they were executed already before
-     entering THISFRAME.  */
-  frame = skip_tailcall_frames (frame);
-
   /* Find the function we will return from.  */
 
   sm->function = find_pc_function (get_frame_pc (get_selected_frame (NULL)));
@@ -2030,7 +2026,16 @@ finish_command (char *arg, int from_tty)
   if (execution_direction == EXEC_REVERSE)
     finish_backward (sm);
   else
-    finish_forward (sm, frame);
+    {
+      /* Ignore TAILCALL_FRAME type frames, they were executed already before
+	 entering THISFRAME.  */
+      frame = skip_tailcall_frames (frame);
+
+      if (frame == NULL)
+	error (_("\"finish\" not meaningful for tailcall frames."));
+
+      finish_forward (sm, frame);
+    }
 }
 \f
 
diff --git a/gdb/testsuite/gdb.btrace/i686-tailcall-only.S b/gdb/testsuite/gdb.btrace/i686-tailcall-only.S
new file mode 100644
index 0000000..a89f2f2
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/i686-tailcall-only.S
@@ -0,0 +1,447 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 Free Software Foundation, Inc.
+
+   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/>.
+
+
+   This file has been generated using:
+   gcc -m32 -march=i686 -S -O2 -dA -g tailcall-only.c -o i686-tailcall-only.S
+ */
+
+	.file	"tailcall-only.c"
+	.text
+.Ltext0:
+	.p2align 4,,15
+	.type	bar_1, @function
+bar_1:
+.LFB0:
+	.file 1 "tailcall-only.c"
+	# tailcall-only.c:22
+	.loc 1 22 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:24
+	.loc 1 24 0
+	movl	$42, %eax
+# SUCC: EXIT [100.0%]
+	ret
+	.cfi_endproc
+.LFE0:
+	.size	bar_1, .-bar_1
+	.p2align 4,,15
+	.type	bar, @function
+bar:
+.LFB1:
+	# tailcall-only.c:28
+	.loc 1 28 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:29
+	.loc 1 29 0
+	jmp	bar_1
+# SUCC: EXIT [100.0%]  (ABNORMAL,SIBCALL)
+.LVL0:
+	.cfi_endproc
+.LFE1:
+	.size	bar, .-bar
+	.p2align 4,,15
+	.type	foo_1, @function
+foo_1:
+.LFB2:
+	# tailcall-only.c:34
+	.loc 1 34 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:35
+	.loc 1 35 0
+	jmp	bar
+# SUCC: EXIT [100.0%]  (ABNORMAL,SIBCALL)
+.LVL1:
+	.cfi_endproc
+.LFE2:
+	.size	foo_1, .-foo_1
+	.p2align 4,,15
+	.type	foo, @function
+foo:
+.LFB3:
+	# tailcall-only.c:40
+	.loc 1 40 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:41
+	.loc 1 41 0
+	jmp	foo_1
+# SUCC: EXIT [100.0%]  (ABNORMAL,SIBCALL)
+.LVL2:
+	.cfi_endproc
+.LFE3:
+	.size	foo, .-foo
+	.section	.text.startup,"ax",@progbits
+	.p2align 4,,15
+	.globl	main
+	.type	main, @function
+main:
+.LFB4:
+	# tailcall-only.c:46
+	.loc 1 46 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:49
+	.loc 1 49 0
+	call	foo
+.LVL3:
+	# tailcall-only.c:50
+	.loc 1 50 0
+	addl	$1, %eax
+.LVL4:
+# SUCC: EXIT [100.0%]
+	# tailcall-only.c:53
+	.loc 1 53 0
+	ret
+	.cfi_endproc
+.LFE4:
+	.size	main, .-main
+	.text
+.Letext0:
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.long	0xd5	# Length of Compilation Unit Info
+	.value	0x4	# DWARF version number
+	.long	.Ldebug_abbrev0	# Offset Into Abbrev. Section
+	.byte	0x4	# Pointer Size (in bytes)
+	.uleb128 0x1	# (DIE (0xb) DW_TAG_compile_unit)
+	.long	.LASF1	# DW_AT_producer: "GNU C 4.8.3 20140911 (Red Hat 4.8.3-9) -m32 -march=i686 -g -O2"
+	.byte	0x1	# DW_AT_language
+	.long	.LASF2	# DW_AT_name: "tailcall-only.c"
+	.long	.LASF3	# DW_AT_comp_dir: ""
+	.long	.Ldebug_ranges0+0	# DW_AT_ranges
+	.long	0	# DW_AT_low_pc
+	.long	.Ldebug_line0	# DW_AT_stmt_list
+	.uleb128 0x2	# (DIE (0x25) DW_TAG_subprogram)
+	.long	.LASF4	# DW_AT_name: "bar_1"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x15	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x3a	# DW_AT_type
+	.long	.LFB0	# DW_AT_low_pc
+	.long	.LFE0-.LFB0	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x3	# (DIE (0x3a) DW_TAG_base_type)
+	.byte	0x4	# DW_AT_byte_size
+	.byte	0x5	# DW_AT_encoding
+	.ascii "int\0"	# DW_AT_name
+	.uleb128 0x4	# (DIE (0x41) DW_TAG_subprogram)
+	.ascii "bar\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x1b	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x3a	# DW_AT_type
+	.long	.LFB1	# DW_AT_low_pc
+	.long	.LFE1-.LFB1	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.long	0x64	# DW_AT_sibling
+	.uleb128 0x5	# (DIE (0x5a) DW_TAG_GNU_call_site)
+	.long	.LVL0	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	0x25	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0x41
+	.uleb128 0x6	# (DIE (0x64) DW_TAG_subprogram)
+	.long	.LASF0	# DW_AT_name: "foo_1"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x21	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x3a	# DW_AT_type
+	.long	.LFB2	# DW_AT_low_pc
+	.long	.LFE2-.LFB2	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.long	0x87	# DW_AT_sibling
+	.uleb128 0x5	# (DIE (0x7d) DW_TAG_GNU_call_site)
+	.long	.LVL1	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	0x41	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0x64
+	.uleb128 0x4	# (DIE (0x87) DW_TAG_subprogram)
+	.ascii "foo\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x27	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x3a	# DW_AT_type
+	.long	.LFB3	# DW_AT_low_pc
+	.long	.LFE3-.LFB3	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.long	0xaa	# DW_AT_sibling
+	.uleb128 0x5	# (DIE (0xa0) DW_TAG_GNU_call_site)
+	.long	.LVL2	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	0x64	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0x87
+	.uleb128 0x7	# (DIE (0xaa) DW_TAG_subprogram)
+			# DW_AT_external
+	.long	.LASF5	# DW_AT_name: "main"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x2d	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x3a	# DW_AT_type
+	.long	.LFB4	# DW_AT_low_pc
+	.long	.LFE4-.LFB4	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x8	# (DIE (0xbf) DW_TAG_variable)
+	.long	.LASF6	# DW_AT_name: "answer"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x2f	# DW_AT_decl_line
+	.long	0x3a	# DW_AT_type
+	.long	.LLST0	# DW_AT_location
+	.uleb128 0x9	# (DIE (0xce) DW_TAG_GNU_call_site)
+	.long	.LVL3	# DW_AT_low_pc
+	.long	0x87	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0xaa
+	.byte	0	# end of children of DIE 0xb
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.uleb128 0x1	# (abbrev code)
+	.uleb128 0x11	# (TAG: DW_TAG_compile_unit)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x25	# (DW_AT_producer)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x13	# (DW_AT_language)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x1b	# (DW_AT_comp_dir)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x55	# (DW_AT_ranges)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x10	# (DW_AT_stmt_list)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.byte	0
+	.byte	0
+	.uleb128 0x2	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x6	# (DW_FORM_data4)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x3	# (abbrev code)
+	.uleb128 0x24	# (TAG: DW_TAG_base_type)
+	.byte	0	# DW_children_no
+	.uleb128 0xb	# (DW_AT_byte_size)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3e	# (DW_AT_encoding)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.byte	0
+	.byte	0
+	.uleb128 0x4	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x6	# (DW_FORM_data4)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x1	# (DW_AT_sibling)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x5	# (abbrev code)
+	.uleb128 0x4109	# (TAG: DW_TAG_GNU_call_site)
+	.byte	0	# DW_children_no
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x2115	# (DW_AT_GNU_tail_call)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x31	# (DW_AT_abstract_origin)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x6	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x6	# (DW_FORM_data4)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x1	# (DW_AT_sibling)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x7	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3f	# (DW_AT_external)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x6	# (DW_FORM_data4)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x8	# (abbrev code)
+	.uleb128 0x34	# (TAG: DW_TAG_variable)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x2	# (DW_AT_location)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.byte	0
+	.byte	0
+	.uleb128 0x9	# (abbrev code)
+	.uleb128 0x4109	# (TAG: DW_TAG_GNU_call_site)
+	.byte	0	# DW_children_no
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x31	# (DW_AT_abstract_origin)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.byte	0
+	.section	.debug_loc,"",@progbits
+.Ldebug_loc0:
+.LLST0:
+	.long	.LVL3	# Location list begin address (*.LLST0)
+	.long	.LVL4	# Location list end address (*.LLST0)
+	.value	0x3	# Location expression size
+	.byte	0x70	# DW_OP_breg0
+	.sleb128 1
+	.byte	0x9f	# DW_OP_stack_value
+	.long	.LVL4	# Location list begin address (*.LLST0)
+	.long	.LFE4	# Location list end address (*.LLST0)
+	.value	0x1	# Location expression size
+	.byte	0x50	# DW_OP_reg0
+	.long	0	# Location list terminator begin (*.LLST0)
+	.long	0	# Location list terminator end (*.LLST0)
+	.section	.debug_aranges,"",@progbits
+	.long	0x24	# Length of Address Ranges Info
+	.value	0x2	# DWARF Version
+	.long	.Ldebug_info0	# Offset of Compilation Unit Info
+	.byte	0x4	# Size of Address
+	.byte	0	# Size of Segment Descriptor
+	.value	0	# Pad to 8 byte boundary
+	.value	0
+	.long	.Ltext0	# Address
+	.long	.Letext0-.Ltext0	# Length
+	.long	.LFB4	# Address
+	.long	.LFE4-.LFB4	# Length
+	.long	0
+	.long	0
+	.section	.debug_ranges,"",@progbits
+.Ldebug_ranges0:
+	.long	.Ltext0	# Offset 0
+	.long	.Letext0
+	.long	.LFB4	# Offset 0x8
+	.long	.LFE4
+	.long	0
+	.long	0
+	.section	.debug_line,"",@progbits
+.Ldebug_line0:
+	.section	.debug_str,"MS",@progbits,1
+.LASF4:
+	.string	"bar_1"
+.LASF2:
+	.string	"tailcall-only.c"
+.LASF1:
+	.string	"GNU C 4.8.3 20140911 (Red Hat 4.8.3-9) -m32 -march=i686 -g -O2"
+.LASF6:
+	.string	"answer"
+.LASF5:
+	.string	"main"
+.LASF3:
+	.string	""
+.LASF0:
+	.string	"foo_1"
+	.ident	"GCC: (GNU) 4.8.3 20140911 (Red Hat 4.8.3-9)"
+	.section	.note.GNU-stack,"",@progbits
diff --git a/gdb/testsuite/gdb.btrace/tailcall-only.c b/gdb/testsuite/gdb.btrace/tailcall-only.c
new file mode 100644
index 0000000..5913ddc
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/tailcall-only.c
@@ -0,0 +1,53 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 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/>.  */
+
+static __attribute__ ((noinline)) int
+bar_1 (void)
+{
+  return 42;
+}
+
+static __attribute__ ((noinline)) int
+bar (void)
+{
+  return bar_1 ();
+}
+
+static __attribute__ ((noinline)) int
+foo_1 (void)
+{
+  return bar ();
+}
+
+static __attribute__ ((noinline)) int
+foo (void)
+{
+  return foo_1 ();
+}
+
+int
+main (void)
+{
+  int answer;
+
+  answer = foo ();
+  answer += 1;
+
+  return answer;
+}
diff --git a/gdb/testsuite/gdb.btrace/tailcall-only.exp b/gdb/testsuite/gdb.btrace/tailcall-only.exp
new file mode 100644
index 0000000..21064a4
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/tailcall-only.exp
@@ -0,0 +1,93 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2016 Free Software Foundation, Inc.
+#
+# 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/>.
+#
+#
+# This is a variant of tailcall.exp where the entire trace contains only tail
+# calls.  This used to cause a crash in get_frame_type.
+#
+
+# check for btrace support
+if { [skip_btrace_tests] } { return -1 }
+
+# This test requires the compiler to generate a tail call.  To guarantee that
+# we always get one, we use an assembly source file.
+#
+# We use different assembly sources based on the target architecture.
+#
+# Luckily, they are similar enough that a single test script can handle
+# both.
+set opts {}
+if [info exists COMPILE] {
+    # make check RUNTESTFLAGS="gdb.btrace/tailcall-only.exp COMPILE=1"
+    standard_testfile tailcall-only.c
+    lappend opts debug optimize=-O2
+} elseif {[istarget "x86_64-*-*"]} {
+	standard_testfile x86_64-tailcall-only.S
+} elseif {[istarget "i?86-*-*"]} {
+	standard_testfile i686-tailcall-only.S
+} else {
+    verbose "Skipping ${testfile}."
+    return
+}
+
+if [prepare_for_testing tailcall-only.exp $testfile $srcfile $opts] {
+    return -1
+}
+if ![runto_main] {
+    return -1
+}
+
+# we want to see the full trace for this test
+gdb_test_no_output "set record function-call-history-size 0"
+
+# trace foo
+gdb_test "step" ".*"
+gdb_test_no_output "record btrace"
+gdb_test "stepi 4" ".*"
+
+# for debugging
+gdb_test "info record" ".*"
+
+# show the branch trace with calls indented
+gdb_test "record function-call-history /c 1" [multi_line \
+  "1\tfoo" \
+  "2\t  foo_1" \
+  "3\t    bar" \
+  "4\t      bar_1"
+  ] "function-call-history"
+
+# We can step
+gdb_test "record goto begin" ".*foo.*"
+gdb_test "stepi" ".*foo_1.*"
+gdb_test "step" ".*bar.*"
+gdb_test "stepi" ".*bar_1.*"
+
+# We can neither finish nor return.
+gdb_test "finish" "\"finish\" not meaningful for tailcall frames.*"
+gdb_test_multiple "return" "return" {
+  -re "Make .* return now.*y or n. $" {
+    send_gdb "y\n"
+    exp_continue
+  }
+  -re "Cannot pop a tailcall frame.*$gdb_prompt $" {
+    pass "return"
+  }
+}
+
+# But we can reverse-finish
+gdb_test "reverse-finish" ".*bar.*"
+gdb_test "reverse-step" ".*foo_1.*"
diff --git a/gdb/testsuite/gdb.btrace/x86_64-tailcall-only.S b/gdb/testsuite/gdb.btrace/x86_64-tailcall-only.S
new file mode 100644
index 0000000..710288a
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/x86_64-tailcall-only.S
@@ -0,0 +1,446 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 Free Software Foundation, Inc.
+
+   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/>.
+
+
+   This file has been generated using:
+   gcc -S -O2 -dA -g tailcall-only.c -o x86_64-tailcall-only.S  */
+
+	.file	"tailcall-only.c"
+	.text
+.Ltext0:
+	.p2align 4,,15
+	.type	bar_1, @function
+bar_1:
+.LFB0:
+	.file 1 "tailcall-only.c"
+	# tailcall-only.c:22
+	.loc 1 22 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:24
+	.loc 1 24 0
+	movl	$42, %eax
+# SUCC: EXIT [100.0%]
+	ret
+	.cfi_endproc
+.LFE0:
+	.size	bar_1, .-bar_1
+	.p2align 4,,15
+	.type	bar, @function
+bar:
+.LFB1:
+	# tailcall-only.c:28
+	.loc 1 28 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:29
+	.loc 1 29 0
+	jmp	bar_1
+# SUCC: EXIT [100.0%]  (ABNORMAL,SIBCALL)
+.LVL0:
+	.cfi_endproc
+.LFE1:
+	.size	bar, .-bar
+	.p2align 4,,15
+	.type	foo_1, @function
+foo_1:
+.LFB2:
+	# tailcall-only.c:34
+	.loc 1 34 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:35
+	.loc 1 35 0
+	jmp	bar
+# SUCC: EXIT [100.0%]  (ABNORMAL,SIBCALL)
+.LVL1:
+	.cfi_endproc
+.LFE2:
+	.size	foo_1, .-foo_1
+	.p2align 4,,15
+	.type	foo, @function
+foo:
+.LFB3:
+	# tailcall-only.c:40
+	.loc 1 40 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:41
+	.loc 1 41 0
+	jmp	foo_1
+# SUCC: EXIT [100.0%]  (ABNORMAL,SIBCALL)
+.LVL2:
+	.cfi_endproc
+.LFE3:
+	.size	foo, .-foo
+	.section	.text.startup,"ax",@progbits
+	.p2align 4,,15
+	.globl	main
+	.type	main, @function
+main:
+.LFB4:
+	# tailcall-only.c:46
+	.loc 1 46 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:49
+	.loc 1 49 0
+	call	foo
+.LVL3:
+	# tailcall-only.c:50
+	.loc 1 50 0
+	addl	$1, %eax
+.LVL4:
+# SUCC: EXIT [100.0%]
+	# tailcall-only.c:53
+	.loc 1 53 0
+	ret
+	.cfi_endproc
+.LFE4:
+	.size	main, .-main
+	.text
+.Letext0:
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.long	0x111	# Length of Compilation Unit Info
+	.value	0x4	# DWARF version number
+	.long	.Ldebug_abbrev0	# Offset Into Abbrev. Section
+	.byte	0x8	# Pointer Size (in bytes)
+	.uleb128 0x1	# (DIE (0xb) DW_TAG_compile_unit)
+	.long	.LASF1	# DW_AT_producer: "GNU C 4.8.3 20140911 (Red Hat 4.8.3-9) -mtune=generic -march=x86-64 -g -O2"
+	.byte	0x1	# DW_AT_language
+	.long	.LASF2	# DW_AT_name: "tailcall-only.c"
+	.long	.LASF3	# DW_AT_comp_dir: ""
+	.long	.Ldebug_ranges0+0	# DW_AT_ranges
+	.quad	0	# DW_AT_low_pc
+	.long	.Ldebug_line0	# DW_AT_stmt_list
+	.uleb128 0x2	# (DIE (0x29) DW_TAG_subprogram)
+	.long	.LASF4	# DW_AT_name: "bar_1"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x15	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x46	# DW_AT_type
+	.quad	.LFB0	# DW_AT_low_pc
+	.quad	.LFE0-.LFB0	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x3	# (DIE (0x46) DW_TAG_base_type)
+	.byte	0x4	# DW_AT_byte_size
+	.byte	0x5	# DW_AT_encoding
+	.ascii "int\0"	# DW_AT_name
+	.uleb128 0x4	# (DIE (0x4d) DW_TAG_subprogram)
+	.ascii "bar\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x1b	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x46	# DW_AT_type
+	.quad	.LFB1	# DW_AT_low_pc
+	.quad	.LFE1-.LFB1	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.long	0x7c	# DW_AT_sibling
+	.uleb128 0x5	# (DIE (0x6e) DW_TAG_GNU_call_site)
+	.quad	.LVL0	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	0x29	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0x4d
+	.uleb128 0x6	# (DIE (0x7c) DW_TAG_subprogram)
+	.long	.LASF0	# DW_AT_name: "foo_1"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x21	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x46	# DW_AT_type
+	.quad	.LFB2	# DW_AT_low_pc
+	.quad	.LFE2-.LFB2	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.long	0xab	# DW_AT_sibling
+	.uleb128 0x5	# (DIE (0x9d) DW_TAG_GNU_call_site)
+	.quad	.LVL1	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	0x4d	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0x7c
+	.uleb128 0x4	# (DIE (0xab) DW_TAG_subprogram)
+	.ascii "foo\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x27	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x46	# DW_AT_type
+	.quad	.LFB3	# DW_AT_low_pc
+	.quad	.LFE3-.LFB3	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.long	0xda	# DW_AT_sibling
+	.uleb128 0x5	# (DIE (0xcc) DW_TAG_GNU_call_site)
+	.quad	.LVL2	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	0x7c	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0xab
+	.uleb128 0x7	# (DIE (0xda) DW_TAG_subprogram)
+			# DW_AT_external
+	.long	.LASF5	# DW_AT_name: "main"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x2d	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x46	# DW_AT_type
+	.quad	.LFB4	# DW_AT_low_pc
+	.quad	.LFE4-.LFB4	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x8	# (DIE (0xf7) DW_TAG_variable)
+	.long	.LASF6	# DW_AT_name: "answer"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x2f	# DW_AT_decl_line
+	.long	0x46	# DW_AT_type
+	.long	.LLST0	# DW_AT_location
+	.uleb128 0x9	# (DIE (0x106) DW_TAG_GNU_call_site)
+	.quad	.LVL3	# DW_AT_low_pc
+	.long	0xab	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0xda
+	.byte	0	# end of children of DIE 0xb
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.uleb128 0x1	# (abbrev code)
+	.uleb128 0x11	# (TAG: DW_TAG_compile_unit)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x25	# (DW_AT_producer)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x13	# (DW_AT_language)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x1b	# (DW_AT_comp_dir)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x55	# (DW_AT_ranges)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x10	# (DW_AT_stmt_list)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.byte	0
+	.byte	0
+	.uleb128 0x2	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x3	# (abbrev code)
+	.uleb128 0x24	# (TAG: DW_TAG_base_type)
+	.byte	0	# DW_children_no
+	.uleb128 0xb	# (DW_AT_byte_size)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3e	# (DW_AT_encoding)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.byte	0
+	.byte	0
+	.uleb128 0x4	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x1	# (DW_AT_sibling)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x5	# (abbrev code)
+	.uleb128 0x4109	# (TAG: DW_TAG_GNU_call_site)
+	.byte	0	# DW_children_no
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x2115	# (DW_AT_GNU_tail_call)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x31	# (DW_AT_abstract_origin)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x6	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x1	# (DW_AT_sibling)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x7	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3f	# (DW_AT_external)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x8	# (abbrev code)
+	.uleb128 0x34	# (TAG: DW_TAG_variable)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x2	# (DW_AT_location)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.byte	0
+	.byte	0
+	.uleb128 0x9	# (abbrev code)
+	.uleb128 0x4109	# (TAG: DW_TAG_GNU_call_site)
+	.byte	0	# DW_children_no
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x31	# (DW_AT_abstract_origin)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.byte	0
+	.section	.debug_loc,"",@progbits
+.Ldebug_loc0:
+.LLST0:
+	.quad	.LVL3	# Location list begin address (*.LLST0)
+	.quad	.LVL4	# Location list end address (*.LLST0)
+	.value	0x3	# Location expression size
+	.byte	0x70	# DW_OP_breg0
+	.sleb128 1
+	.byte	0x9f	# DW_OP_stack_value
+	.quad	.LVL4	# Location list begin address (*.LLST0)
+	.quad	.LFE4	# Location list end address (*.LLST0)
+	.value	0x1	# Location expression size
+	.byte	0x50	# DW_OP_reg0
+	.quad	0	# Location list terminator begin (*.LLST0)
+	.quad	0	# Location list terminator end (*.LLST0)
+	.section	.debug_aranges,"",@progbits
+	.long	0x3c	# Length of Address Ranges Info
+	.value	0x2	# DWARF Version
+	.long	.Ldebug_info0	# Offset of Compilation Unit Info
+	.byte	0x8	# Size of Address
+	.byte	0	# Size of Segment Descriptor
+	.value	0	# Pad to 16 byte boundary
+	.value	0
+	.quad	.Ltext0	# Address
+	.quad	.Letext0-.Ltext0	# Length
+	.quad	.LFB4	# Address
+	.quad	.LFE4-.LFB4	# Length
+	.quad	0
+	.quad	0
+	.section	.debug_ranges,"",@progbits
+.Ldebug_ranges0:
+	.quad	.Ltext0	# Offset 0
+	.quad	.Letext0
+	.quad	.LFB4	# Offset 0x10
+	.quad	.LFE4
+	.quad	0
+	.quad	0
+	.section	.debug_line,"",@progbits
+.Ldebug_line0:
+	.section	.debug_str,"MS",@progbits,1
+.LASF4:
+	.string	"bar_1"
+.LASF2:
+	.string	"tailcall-only.c"
+.LASF1:
+	.string	"GNU C 4.8.3 20140911 (Red Hat 4.8.3-9) -mtune=generic -march=x86-64 -g -O2"
+.LASF6:
+	.string	"answer"
+.LASF5:
+	.string	"main"
+.LASF3:
+	.string	""
+.LASF0:
+	.string	"foo_1"
+	.ident	"GCC: (GNU) 4.8.3 20140911 (Red Hat 4.8.3-9)"
+	.section	.note.GNU-stack,"",@progbits
-- 
1.8.3.1

Re: [PATCH v2 1/3] frame: add skip_tailcall_frames

[Joel Brobecker]

> Add a new function skip_tailcall_frames to skip TAILCALL_FRAME frames.
> 
> 2016-02-05  Markus Metzger  <markus.t.metzger@intel.com>
> 
> gdb/
> 	* frame.h (skip_tailcall_frames): New.
> 	* frame.c (skip_tailcall_frames): New.
> 	(frame_pop): Call skip_tailcall_frames.
> 	* infcmd.c: Call skip_tailcall_frames.

OK!

-- 
Joel

Re: [PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames

[Joel Brobecker]

> Following the practice in skip_artificial_frames, also use
> get_prev_frame_always instead of get_prev_frame in
> skip_tailcall_frames.
> 
> 2016-02-05  Markus Metzger  <markus.t.metzger@intel.com>
> 
> gdb/
> 	* frame.c (skip_tailcall_frames): Call get_prev_frame_always.

This seems OK; but would you be able to create a testcase for this?

Thanks,
-- 
Joel

RE: [PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames

[Metzger, Markus T]

> -----Original Message-----
> From: Joel Brobecker [mailto:brobecker@adacore.com]
> Sent: Sunday, February 7, 2016 2:01 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: palves@redhat.com; gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 2/3] frame: use get_prev_frame_always in
> skip_tailcall_frames

Hi Joel,

Thanks for your review.


> > Following the practice in skip_artificial_frames, also use
> > get_prev_frame_always instead of get_prev_frame in
> > skip_tailcall_frames.
> >
> > 2016-02-05  Markus Metzger  <markus.t.metzger@intel.com>
> >
> > gdb/
> > 	* frame.c (skip_tailcall_frames): Call get_prev_frame_always.
> 
> This seems OK; but would you be able to create a testcase for this?

I modified an existing test case to cover the changes.  GDB dies with the modified
test and without the changes to skip_tailcall_frames.  This also showed another place
where we want to use get_prev_frame_always.

Here's the modified version of this patch:


commit d426b734a965ef21d03bf7ed30f20be7d7ed31fa
Author: Markus Metzger <markus.t.metzger@intel.com>
Date:   Fri Feb 5 09:39:05 2016 +0100

    frame: use get_prev_frame_always in skip_tailcall_frames and finish_command
    
    Following the practice in skip_artificial_frames, also use get_prev_frame_always
    instead of get_prev_frame in skip_tailcall_frames and in finish_command.
    
    2016-02-08  Markus Metzger  <markus.t.metzger@intel.com>
    
    gdb/
    	* frame.c (skip_tailcall_frames): Call get_prev_frame_always.
    	* infcmd.c (finish_command): Call get_prev_frame_always.
    
    testsuite/
    	* gdb.arch/amd64-tailcall-ret.exp: Set backtrace limit.

diff --git a/gdb/frame.c b/gdb/frame.c
index b7832c7..6ab8834 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -443,8 +443,12 @@ skip_artificial_frames (struct frame_info *frame)
 struct frame_info *
 skip_tailcall_frames (struct frame_info *frame)
 {
+  /* Note we use get_prev_frame_always, and not get_prev_frame.  The
+     latter will truncate the frame chain, leading to this function
+     unintentionally returning a null_frame_id (e.g., when the user
+     sets a backtrace limit).  */
   while (get_frame_type (frame) == TAILCALL_FRAME)
-    frame = get_prev_frame (frame);
+    frame = get_prev_frame_always (frame);
 
   return frame;
 }
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 930dc61..e98f6f5 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1958,7 +1958,10 @@ finish_command (char *arg, int from_tty)
   /* Done with ARGS.  */
   do_cleanups (args_chain);
 
-  frame = get_prev_frame (get_selected_frame (_("No selected frame.")));
+  /* Note we use get_prev_frame_always, and not get_prev_frame.  The latter
+     will truncate the frame chain, leading to this function unintentionally
+     throwing an error (e.g., when the user sets a backtrace limit).  */
+  frame = get_prev_frame_always (get_selected_frame (_("No selected frame.")));
   if (frame == 0)
     error (_("\"finish\" not meaningful in the outermost frame."));
 
diff --git a/gdb/testsuite/gdb.arch/amd64-tailcall-ret.exp b/gdb/testsuite/gdb.arch/amd64-tailcall-ret.exp
index 2642cdd..0334b54 100644
--- a/gdb/testsuite/gdb.arch/amd64-tailcall-ret.exp
+++ b/gdb/testsuite/gdb.arch/amd64-tailcall-ret.exp
@@ -36,6 +36,11 @@ if ![runto_main] {
 gdb_breakpoint "g"
 gdb_continue_to_breakpoint "g" ".* v = 2;"
 
+# Severely limit the back trace.  The limit should be ignored for "return"
+# and "finish" commands.
+#
+gdb_test "set backtrace limit 1"
+
 gdb_test "return" { f \(\); /\* second \*/} "return" \
          {Make g return now\? \(y or n\) } "y"

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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames

[Pedro Alves]

On 02/08/2016 08:14 AM, Metzger, Markus T wrote:

> I modified an existing test case to cover the changes.  GDB dies with the modified
> test and without the changes to skip_tailcall_frames.  This also showed another place
> where we want to use get_prev_frame_always.
> 
> Here's the modified version of this patch:

Thanks.  Could you do the limiting test in e.g., gdb.base/finish.exp
and gdb.base/return.exp, so that it'd be covered on all archs?

Thanks,
Pedro Alves

Re: [PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames

[Joel Brobecker]

> > I modified an existing test case to cover the changes.  GDB dies with the modified
> > test and without the changes to skip_tailcall_frames.  This also showed another place
> > where we want to use get_prev_frame_always.
> > 
> > Here's the modified version of this patch:
> 
> Thanks.  Could you do the limiting test in e.g., gdb.base/finish.exp
> and gdb.base/return.exp, so that it'd be covered on all archs?

I was going to ask the very same :-). The fact that adding your test
showed we missed a spot raised the question as to how much of the
initial patch we were testing :).

-- 
Joel

Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Pedro Alves]

On 02/05/2016 02:18 PM, Markus Metzger wrote:

> diff --git a/gdb/frame.c b/gdb/frame.c
> index 6ab8834..cea7003 100644
> --- a/gdb/frame.c
> +++ b/gdb/frame.c
> @@ -420,7 +420,8 @@ fprint_frame (struct ui_file *file, struct frame_info *fi)
>  
>  /* Given FRAME, return the enclosing frame as found in real frames read-in from
>     inferior memory.  Skip any previous frames which were made up by GDB.
> -   Return the original frame if no immediate previous frames exist.  */
> +   Return FRAME if FRAME is a non-artificial frame.
> +   Return NULL if FRAME is NULL or the start of an artificial-only chain. */

Missing double-space after period.  But, most importantly, I'm not sure I like
that this accepts a NULL frame.

>  
>  static struct frame_info *
>  skip_artificial_frames (struct frame_info *frame)
> @@ -428,11 +429,13 @@ skip_artificial_frames (struct frame_info *frame)
>    /* Note we use get_prev_frame_always, and not get_prev_frame.  The
>       latter will truncate the frame chain, leading to this function
>       unintentionally returning a null_frame_id (e.g., when the user
> -     sets a backtrace limit).  This is safe, because as these frames
> -     are made up by GDB, there must be a real frame in the chain
> -     below.  */
> -  while (get_frame_type (frame) == INLINE_FRAME
> -	 || get_frame_type (frame) == TAILCALL_FRAME)
> +     sets a backtrace limit).
> +
> +     Note that for record targets we may get a frame chain that consists
> +     of artificial frames only.  */
> +  while (frame != NULL &&
> +	 (get_frame_type (frame) == INLINE_FRAME
> +	  || get_frame_type (frame) == TAILCALL_FRAME))
>      frame = get_prev_frame_always (frame);

&& goes on the next line, but can we make these look like:

static struct frame_info *
skip_artificial_frames (struct frame_info *frame)
{
...
  while (get_frame_type (frame) == INLINE_FRAME
	 || get_frame_type (frame) == TAILCALL_FRAME)
    {
      frame = get_prev_frame_always (frame);
      if (frame == NULL)
        break;
    }

  return frame;
}

> @@ -511,11 +517,12 @@ frame_unwind_caller_id (struct frame_info *next_frame)
>       requests the frame ID of "main()"s caller.  */
>  
>    next_frame = skip_artificial_frames (next_frame);
> -  this_frame = get_prev_frame_always (next_frame);
> -  if (this_frame)
> -    return get_frame_id (skip_artificial_frames (this_frame));
> -  else
> +  if (next_frame == NULL)
>      return null_frame_id;
> +
> +  this_frame = get_prev_frame_always (next_frame);
> +  this_frame = skip_artificial_frames (this_frame);
> +  return get_frame_id (this_frame);
>  }
>  
>  const struct frame_id null_frame_id = { 0 }; /* All zeros.  */
> @@ -795,6 +802,9 @@ frame_find_by_id (struct frame_id id)
>  static CORE_ADDR
>  frame_unwind_pc (struct frame_info *this_frame)
>  {
> +  if (this_frame == NULL)
> +    throw_error (NOT_AVAILABLE_ERROR, _("PC not available"));

How can this happen?

> +
>    if (this_frame->prev_pc.status == CC_UNKNOWN)
>      {
>        if (gdbarch_unwind_pc_p (frame_unwind_arch (this_frame)))



> +
>    /* Make a copy of all the register values unwound from this frame.
>       Save them in a scratch buffer so that there isn't a race between
>       trying to extract the old values from the current regcache while
> @@ -2575,7 +2589,20 @@ frame_unwind_arch (struct frame_info *next_frame)
>  struct gdbarch *
>  frame_unwind_caller_arch (struct frame_info *next_frame)
>  {
> -  return frame_unwind_arch (skip_artificial_frames (next_frame));
> +  struct frame_info *caller;
> +
> +  /* If the frame chain consists only of artificial frames, use NEXT_FRAME's.
> +
> +     For tailcall frames, we (i.e. the DWARF frame unwinder) assume that they
> +     have the gdbarch of the bottom (callee) frame.  If NEXT_FRAME is an
> +     artificial frame, as well, we should drill down to the bottom in
> +     frame_unwind_arch either directly via the tailcall unwinder or via a chain
> +     of get_frame_arch calls.  */
> +  caller = skip_artificial_frames (next_frame);
> +  if (caller == NULL)
> +    get_frame_arch (next_frame);
> +
> +  return frame_unwind_arch (next_frame);

Hmm, this looks odd.  Is the get_frame_arch call there for side
effects, or did you mean to make that "return get_frame_arch" ?

> +# We can step
> +gdb_test "record goto begin" ".*foo.*"
> +gdb_test "stepi" ".*foo_1.*"
> +gdb_test "step" ".*bar.*"
> +gdb_test "stepi" ".*bar_1.*"

Please watch out for duplicate test messages:
https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_messages_are_unique

Thanks,
Pedro Alves

RE: [PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames

[Metzger, Markus T]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Joel Brobecker
> Sent: Tuesday, February 9, 2016 12:58 PM
> To: Pedro Alves <palves@redhat.com>
> Cc: Metzger, Markus T <markus.t.metzger@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH v2 2/3] frame: use get_prev_frame_always in
> skip_tailcall_frames
> 
> > > I modified an existing test case to cover the changes.  GDB dies
> > > with the modified test and without the changes to
> > > skip_tailcall_frames.  This also showed another place where we want to
> use get_prev_frame_always.
> > >
> > > Here's the modified version of this patch:
> >
> > Thanks.  Could you do the limiting test in e.g., gdb.base/finish.exp
> > and gdb.base/return.exp, so that it'd be covered on all archs?
> 
> I was going to ask the very same :-). The fact that adding your test showed
> we missed a spot raised the question as to how much of the initial patch we
> were testing :).

I don't get your comment.

I'm beginning to wonder if not all-but-the-backtrace-command-related
get_prev_frame calls should really be calling get_prev_frame_always.

The _always extension isn't very intuitive, though, given that this should be
the standard function to use.  Should get_prev_frame maybe be renamed to
something like get_prev_frame_within_limit and get_prev_frame_always
to get_prev_frame?

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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames

[Pedro Alves]

On 02/09/2016 02:25 PM, Metzger, Markus T wrote:

> I'm beginning to wonder if not all-but-the-backtrace-command-related
> get_prev_frame calls should really be calling get_prev_frame_always.
> 
> The _always extension isn't very intuitive, though, given that this should be
> the standard function to use.  Should get_prev_frame maybe be renamed to
> something like get_prev_frame_within_limit and get_prev_frame_always
> to get_prev_frame?

Maybe a good idea.  See also:

 https://sourceware.org/bugzilla/show_bug.cgi?id=15558

I just noticed/remembered something -- the check for backtracing past
main and the entry point is in get_prev_frame, get_prev_frame_always
bypasses it.

This means that with your change, I think gdb now allows "finish" in
"main" or in "_start".

Maybe not a bad change, but I though it'd call it out explicitly.

Thanks,
Pedro Alves

RE: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Tuesday, February 9, 2016 1:06 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type
> 
> On 02/05/2016 02:18 PM, Markus Metzger wrote:
> 
> > diff --git a/gdb/frame.c b/gdb/frame.c index 6ab8834..cea7003 100644
> > --- a/gdb/frame.c
> > +++ b/gdb/frame.c
> > @@ -420,7 +420,8 @@ fprint_frame (struct ui_file *file, struct
> > frame_info *fi)
> >
> >  /* Given FRAME, return the enclosing frame as found in real frames read-
> in from
> >     inferior memory.  Skip any previous frames which were made up by GDB.
> > -   Return the original frame if no immediate previous frames exist.  */
> > +   Return FRAME if FRAME is a non-artificial frame.
> > +   Return NULL if FRAME is NULL or the start of an artificial-only
> > + chain. */
> 
> Missing double-space after period.  But, most importantly, I'm not sure I like
> that this accepts a NULL frame.

Fine with me.


> > @@ -511,11 +517,12 @@ frame_unwind_caller_id (struct frame_info
> *next_frame)
> >       requests the frame ID of "main()"s caller.  */
> >
> >    next_frame = skip_artificial_frames (next_frame);
> > -  this_frame = get_prev_frame_always (next_frame);
> > -  if (this_frame)
> > -    return get_frame_id (skip_artificial_frames (this_frame));
> > -  else
> > +  if (next_frame == NULL)
> >      return null_frame_id;
> > +
> > +  this_frame = get_prev_frame_always (next_frame);  this_frame =
> > + skip_artificial_frames (this_frame);  return get_frame_id
> > + (this_frame);
> >  }
> >
> >  const struct frame_id null_frame_id = { 0 }; /* All zeros.  */ @@
> > -795,6 +802,9 @@ frame_find_by_id (struct frame_id id)  static
> > CORE_ADDR  frame_unwind_pc (struct frame_info *this_frame)  {
> > +  if (this_frame == NULL)
> > +    throw_error (NOT_AVAILABLE_ERROR, _("PC not available"));
> 
> How can this happen?

One of its callers, frame_unwind_caller_pc, calls it with the result of
skip_artificial_frames like this:

CORE_ADDR
frame_unwind_caller_pc (struct frame_info *this_frame)
{
  return frame_unwind_pc (skip_artificial_frames (this_frame));
}

Rather than handling the skip_artificial_frames() NULL return here,
I made frame_unwind_pc handle a NULL frame argument.

I can move the check into frame_unwind_caller_pc if you prefer.


> > +  /* If the frame chain consists only of artificial frames, use
> NEXT_FRAME's.
> > +
> > +     For tailcall frames, we (i.e. the DWARF frame unwinder) assume that
> they
> > +     have the gdbarch of the bottom (callee) frame.  If NEXT_FRAME is an
> > +     artificial frame, as well, we should drill down to the bottom in
> > +     frame_unwind_arch either directly via the tailcall unwinder or via a
> chain
> > +     of get_frame_arch calls.  */
> > +  caller = skip_artificial_frames (next_frame);  if (caller == NULL)
> > +    get_frame_arch (next_frame);
> > +
> > +  return frame_unwind_arch (next_frame);
> 
> Hmm, this looks odd.  Is the get_frame_arch call there for side effects, or did
> you mean to make that "return get_frame_arch" ?

Ouch.  Yes, I meant to return that frame.


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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames

[Joel Brobecker]

> > I was going to ask the very same :-). The fact that adding your test showed
> > we missed a spot raised the question as to how much of the initial patch we
> > were testing :).
> 
> I don't get your comment.

This is the logic behind it: Presumably, your initial patch did
fix something. It would be nice to have that tested, hence the
suggestion to add that. You then added a test, but I think it
only partially overlaps with the situation your initial patch
was trying to cover, because the test you added uncovered a spot
that you didn't need to change before. That's why I think there
is a strong chance that adding one more test would increase coverage
of your patch.

Or said differently, if we undid any hunk in your commit, would
a test immediately regress?

> I'm beginning to wonder if not all-but-the-backtrace-command-related
> get_prev_frame calls should really be calling get_prev_frame_always.
> 
> The _always extension isn't very intuitive, though, given that this should be
> the standard function to use.  Should get_prev_frame maybe be renamed to
> something like get_prev_frame_within_limit and get_prev_frame_always
> to get_prev_frame?

(need more time to answer that question)

-- 
Joel

RE: [PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames

[Metzger, Markus T]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Pedro Alves
> Sent: Tuesday, February 9, 2016 3:41 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>; Joel Brobecker
> <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 2/3] frame: use get_prev_frame_always in
> skip_tailcall_frames
> 
> On 02/09/2016 02:25 PM, Metzger, Markus T wrote:
> 
> > I'm beginning to wonder if not all-but-the-backtrace-command-related
> > get_prev_frame calls should really be calling get_prev_frame_always.
> >
> > The _always extension isn't very intuitive, though, given that this
> > should be the standard function to use.  Should get_prev_frame maybe
> > be renamed to something like get_prev_frame_within_limit and
> > get_prev_frame_always to get_prev_frame?
> 
> Maybe a good idea.  See also:
> 
>  https://sourceware.org/bugzilla/show_bug.cgi?id=15558
> 
> I just noticed/remembered something -- the check for backtracing past main
> and the entry point is in get_prev_frame, get_prev_frame_always bypasses
> it.
> 
> This means that with your change, I think gdb now allows "finish" in "main" or
> in "_start".
> 
> Maybe not a bad change, but I though it'd call it out explicitly.

There are 46 calls to get_prev_frame ignoring this patch.  Each of them should
maybe be modified to call get_prev_frame_always, instead.  And none of this
is currently covered by the test suite.

The first step would probably be to look at all those calls and derive test cases
for each of them.  Then fix the failing tests one by one.

This is unrelated to this patch series.  Let me therefore drop this patch.

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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Pedro Alves]

On 02/09/2016 02:42 PM, Metzger, Markus T wrote:

>>> CORE_ADDR  frame_unwind_pc (struct frame_info *this_frame)  {
>>> +  if (this_frame == NULL)
>>> +    throw_error (NOT_AVAILABLE_ERROR, _("PC not available"));
>>
>> How can this happen?
> 
> One of its callers, frame_unwind_caller_pc, calls it with the result of
> skip_artificial_frames like this:
> 
> CORE_ADDR
> frame_unwind_caller_pc (struct frame_info *this_frame)
> {
>   return frame_unwind_pc (skip_artificial_frames (this_frame));
> }
> 
> Rather than handling the skip_artificial_frames() NULL return here,
> I made frame_unwind_pc handle a NULL frame argument.
> 
> I can move the check into frame_unwind_caller_pc if you prefer.

Yes, please.

Though, I think all these frame_unwind_caller_XXX methods should be
consistent in how they handle skip_artificial_frames (this_frame)
returning NULL, because they're all called together, assuming they're
referring to the same frame.  If we throw error here, then I think
we should throw in frame_unwind_caller_arch too, instead of having
that one return the arch of the next frame.

Thanks,
Pedro Alves

RE: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Tuesday, February 9, 2016 11:02 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type
> 
> On 02/09/2016 02:42 PM, Metzger, Markus T wrote:
> 
> >>> CORE_ADDR  frame_unwind_pc (struct frame_info *this_frame)  {
> >>> +  if (this_frame == NULL)
> >>> +    throw_error (NOT_AVAILABLE_ERROR, _("PC not available"));
> >>
> >> How can this happen?
> >
> > One of its callers, frame_unwind_caller_pc, calls it with the result
> > of skip_artificial_frames like this:
> >
> > CORE_ADDR
> > frame_unwind_caller_pc (struct frame_info *this_frame) {
> >   return frame_unwind_pc (skip_artificial_frames (this_frame)); }
> >
> > Rather than handling the skip_artificial_frames() NULL return here, I
> > made frame_unwind_pc handle a NULL frame argument.
> >
> > I can move the check into frame_unwind_caller_pc if you prefer.
> 
> Yes, please.
> 
> Though, I think all these frame_unwind_caller_XXX methods should be
> consistent in how they handle skip_artificial_frames (this_frame) returning
> NULL, because they're all called together, assuming they're referring to the
> same frame.  If we throw error here, then I think we should throw in
> frame_unwind_caller_arch too, instead of having that one return the arch of
> the next frame.

get_frame_arch and frame_unwind_arch don't seem to throw any error today.
I'd rather not introduce new exceptions if not strictly necessary.  Its callers may
not be prepared to handle them.

In the absence of an arch unwinder, frame_unwind_arch uses the gdbarch of
the next frame.  And DWARF tailcall frames use the gdbarch of the bottom
non-tailcall frame.  This is consistent with the proposed changes.

We may want to return frame_unwind_arch (next_frame) instead of
get_frame_arch (next_frame).  OTOH gdb/dwarf2-frame-tailcall.c's
tailcall_frame_prev_arch returns get_frame_arch (cache->next_bottom_frame).
I'm currently mimicking that behavior.

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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Pedro Alves]

On 02/10/2016 07:40 AM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: Pedro Alves [mailto:palves@redhat.com]
>> Sent: Tuesday, February 9, 2016 11:02 PM
>> To: Metzger, Markus T <markus.t.metzger@intel.com>
>> Cc: gdb-patches@sourceware.org
>> Subject: Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type
>>
>> On 02/09/2016 02:42 PM, Metzger, Markus T wrote:
>>
>>>>> CORE_ADDR  frame_unwind_pc (struct frame_info *this_frame)  {
>>>>> +  if (this_frame == NULL)
>>>>> +    throw_error (NOT_AVAILABLE_ERROR, _("PC not available"));
>>>>
>>>> How can this happen?
>>>
>>> One of its callers, frame_unwind_caller_pc, calls it with the result
>>> of skip_artificial_frames like this:
>>>
>>> CORE_ADDR
>>> frame_unwind_caller_pc (struct frame_info *this_frame) {
>>>   return frame_unwind_pc (skip_artificial_frames (this_frame)); }
>>>
>>> Rather than handling the skip_artificial_frames() NULL return here, I
>>> made frame_unwind_pc handle a NULL frame argument.
>>>
>>> I can move the check into frame_unwind_caller_pc if you prefer.
>>
>> Yes, please.
>>
>> Though, I think all these frame_unwind_caller_XXX methods should be
>> consistent in how they handle skip_artificial_frames (this_frame) returning
>> NULL, because they're all called together, assuming they're referring to the
>> same frame.  If we throw error here, then I think we should throw in
>> frame_unwind_caller_arch too, instead of having that one return the arch of
>> the next frame.
> 
> get_frame_arch and frame_unwind_arch don't seem to throw any error today.

Because they assume there is always a prev frame.

The case under discussion is different, it's about getting at the "real
caller frame".  Something has to change.

> I'd rather not introduce new exceptions if not strictly necessary.  Its callers may
> not be prepared to handle them.

Callers shouldn't be handed back potentially bogus results.  If there's no
return that makes sense, then throw and let the callers handle it (or don't
handle it and let the user know what she tried to do doesn't make sense),
or internal-error.

> 
> In the absence of an arch unwinder, frame_unwind_arch uses the gdbarch of
> the next frame.  And DWARF tailcall frames use the gdbarch of the bottom
> non-tailcall frame.  This is consistent with the proposed changes.

In that case, there _is_ a prev frame, and then we default to assuming the unwound
arch is the same as the next frame's arch.  But in this situation at hand, the
difference is that we found that there's _no_ "real caller frame" at all.

> 
> We may want to return frame_unwind_arch (next_frame) instead of
> get_frame_arch (next_frame).  OTOH gdb/dwarf2-frame-tailcall.c's
> tailcall_frame_prev_arch returns get_frame_arch (cache->next_bottom_frame).
> I'm currently mimicking that behavior.

See above.  That situation looks similar on the surface, but the
distinction is important.  The "frame_unwind_CALLER" methods want to drill
down past artificial frames, and return info on the "real caller", while
the "frame_unwind" methods want to unwind one frame only, no matter
artificial or not.

All the frame_unwind_caller_XXX methods should retrieve information
about the same frame that frame_unwind_caller_id returns.  They should
all really be guarded by a frame_unwind_caller_id check, like:

      if (frame_id_p (frame_unwind_caller_id (frame)))
	{
	  scope_breakpoint
	    = create_internal_breakpoint (frame_unwind_caller_arch (frame),
					  frame_unwind_caller_pc (frame),
					  bp_watchpoint_scope,
					  &momentary_breakpoint_ops);

and indeed most are.  Those that aren't, either should be, or are
not because they're called when we know the current frame is a
non-artificial frame.

Put another way, all the frame_unwind_caller_xxx methods should behave
merely as convenience to writing the alternative form that has the
caller skip artificial frames itself:

      frame = skip_artificial_frames (frame);
      if (frame_id_p (get_frame_id (frame)))
	{
 	  scope_breakpoint
	    = create_internal_breakpoint (get_frame_arch (frame),
					  get_frame_pc (frame),
					  bp_watchpoint_scope,
					  &momentary_breakpoint_ops);


As such, if something calls frame_unwind_caller_pc|arch directly
without checking whether frame_unwind_caller_id returns a valid
frame, then the caller code should not continue as if there was
indeed a valid caller frame.  Throwing an error in that case is
better than silently continuing with mocked info.

So, what would break if we internal_error'ed in frame_unwind_caller_arch/pc
instead or normal error, in the case that there's no non-artificial frame?

I'm thinking that maybe only "info frame" (frame_info) would trip on it,
but then that one should probably check frame_unwind_caller_id itself
too.

Thanks,
Pedro Alves

RE: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Wednesday, February 10, 2016 11:00 AM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type


> >>>>> CORE_ADDR  frame_unwind_pc (struct frame_info *this_frame)  {
> >>>>> +  if (this_frame == NULL)
> >>>>> +    throw_error (NOT_AVAILABLE_ERROR, _("PC not available"));
> >>>>
> >>>> How can this happen?
> >>>
> >>> One of its callers, frame_unwind_caller_pc, calls it with the result
> >>> of skip_artificial_frames like this:
> >>>
> >>> CORE_ADDR
> >>> frame_unwind_caller_pc (struct frame_info *this_frame) {
> >>>   return frame_unwind_pc (skip_artificial_frames (this_frame)); }
> >>>
> >>> Rather than handling the skip_artificial_frames() NULL return here,
> >>> I made frame_unwind_pc handle a NULL frame argument.
> >>>
> >>> I can move the check into frame_unwind_caller_pc if you prefer.
> >>
> >> Yes, please.
> >>
> >> Though, I think all these frame_unwind_caller_XXX methods should be
> >> consistent in how they handle skip_artificial_frames (this_frame)
> >> returning NULL, because they're all called together, assuming they're
> >> referring to the same frame.  If we throw error here, then I think we
> >> should throw in frame_unwind_caller_arch too, instead of having that
> >> one return the arch of the next frame.
> >
> > get_frame_arch and frame_unwind_arch don't seem to throw any error
> today.
> 
> Because they assume there is always a prev frame.
> 
> The case under discussion is different, it's about getting at the "real caller
> frame".  Something has to change.
> 
> > I'd rather not introduce new exceptions if not strictly necessary.
> > Its callers may not be prepared to handle them.
> 
> Callers shouldn't be handed back potentially bogus results.  If there's no
> return that makes sense, then throw and let the callers handle it (or don't
> handle it and let the user know what she tried to do doesn't make sense), or
> internal-error.
> 
> >
> > In the absence of an arch unwinder, frame_unwind_arch uses the gdbarch
> > of the next frame.  And DWARF tailcall frames use the gdbarch of the
> > bottom non-tailcall frame.  This is consistent with the proposed changes.
> 
> In that case, there _is_ a prev frame, and then we default to assuming the
> unwound arch is the same as the next frame's arch.  But in this situation at
> hand, the difference is that we found that there's _no_ "real caller frame" at
> all.
> 
> >
> > We may want to return frame_unwind_arch (next_frame) instead of
> > get_frame_arch (next_frame).  OTOH gdb/dwarf2-frame-tailcall.c's
> > tailcall_frame_prev_arch returns get_frame_arch (cache-
> >next_bottom_frame).
> > I'm currently mimicking that behavior.
> 
> See above.  That situation looks similar on the surface, but the distinction is
> important.  The "frame_unwind_CALLER" methods want to drill down past
> artificial frames, and return info on the "real caller", while the
> "frame_unwind" methods want to unwind one frame only, no matter
> artificial or not.
> 
> All the frame_unwind_caller_XXX methods should retrieve information
> about the same frame that frame_unwind_caller_id returns.  They should all
> really be guarded by a frame_unwind_caller_id check, like:
> 
>       if (frame_id_p (frame_unwind_caller_id (frame)))
> 	{
> 	  scope_breakpoint
> 	    = create_internal_breakpoint (frame_unwind_caller_arch (frame),
> 					  frame_unwind_caller_pc (frame),
> 					  bp_watchpoint_scope,
> 					  &momentary_breakpoint_ops);
> 
> and indeed most are.  Those that aren't, either should be, or are not because
> they're called when we know the current frame is a non-artificial frame.
> 
> Put another way, all the frame_unwind_caller_xxx methods should behave
> merely as convenience to writing the alternative form that has the caller skip
> artificial frames itself:
> 
>       frame = skip_artificial_frames (frame);
>       if (frame_id_p (get_frame_id (frame)))
> 	{
>  	  scope_breakpoint
> 	    = create_internal_breakpoint (get_frame_arch (frame),
> 					  get_frame_pc (frame),
> 					  bp_watchpoint_scope,
> 					  &momentary_breakpoint_ops);
> 
> 
> As such, if something calls frame_unwind_caller_pc|arch directly without
> checking whether frame_unwind_caller_id returns a valid frame, then the
> caller code should not continue as if there was indeed a valid caller frame.
> Throwing an error in that case is better than silently continuing with mocked
> info.
> 
> So, what would break if we internal_error'ed in
> frame_unwind_caller_arch/pc instead or normal error, in the case that
> there's no non-artificial frame?

Likely nothing.  The case of only-artificial-frames is not covered by the test
suite.  I'm not sure it is even possible during live debug (ignoring the discussion
about get_prev_frame_always).

The changes were not motivated by fails but by my attempt to make the callers
of skip_artificial_frames handle the now-possible NULL return gracefully.

I ran the btrace suite including the new tests I added to cover the record btrace
tailcall-only-frames case and they pass.  I'm running the full test suite now.

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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Metzger, Markus T]

> -----Original Message-----
> From: Metzger, Markus T
> Sent: Wednesday, February 10, 2016 11:29 AM
> To: Pedro Alves <palves@redhat.com>
> Cc: gdb-patches@sourceware.org
> Subject: RE: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type


> > So, what would break if we internal_error'ed in
> > frame_unwind_caller_arch/pc instead or normal error, in the case that
> > there's no non-artificial frame?
> 
> Likely nothing.  The case of only-artificial-frames is not covered by the test
> suite.  I'm not sure it is even possible during live debug (ignoring the
> discussion about get_prev_frame_always).
> 
> The changes were not motivated by fails but by my attempt to make the
> callers of skip_artificial_frames handle the now-possible NULL return
> gracefully.
> 
> I ran the btrace suite including the new tests I added to cover the record
> btrace tailcall-only-frames case and they pass.  I'm running the full test suite
> now.

No new fails there, as well (64-bit IA).

I added a comment based on your statement that frame_unwind_caller_xxx
callers should check frame_unwind_caller_id and assert that skip_artificial_frames
does not return NULL.

Info frame doesn't crash.

	(gdb) info frame
	Stack level 0, frame at 0x0:
	 rip = 0x4005b0 in bar (tailcall-only.c:29); saved rip = 0x4005c2
	 called by frame at 0x0
	 source 	language c.
	 Arglist at unknown address.
	 Locals at unknown address,Registers are not available in btrace record history

This is from a tailcall-only frame stack in replay mode using the tailcall-only test.
The real caller has not been recorded.

The output isn't very helpful for record btrace since we don't record register and
memory changes.

Here's the patch:

commit a9ba851aa9a126da90f12eaa1788f2d271530535
Author: Markus Metzger <markus.t.metzger@intel.com>
Date:   Tue Jan 26 14:58:44 2016 +0100

    btrace, frame: fix crash in get_frame_type
    
    In skip_artificial_frames we repeatedly call get_prev_frame_always until we get
    a non-inline and non-tailcall frame assuming that there must be such a frame
    eventually.
    
    For record targets, however, we may have a frame chain that consists only of
    artificial frames.  This leads to a crash in get_frame_type when dereferencing a
    NULL frame pointer.
    
    Change skip_artificial_frames and skip_tailcall_frames to return NULL in such a
    case and modify each caller to cope with a NULL return.
    
    In frame_unwind_caller_pc and frame_unwind_caller_arch, we simply assert that
    the returned value is not NULL.  Their caller was supposed to check
    frame_unwind_caller_id before calling those functions.
    
    In other cases, we thrown an error.
    
    In infcmd further move the skip_tailcall_frames call to the forward-stepping
    case since we don't need a frame for reverse execution and we don't want to fail
    because of that.  Reverse-finish does make sense for a tailcall frame.
    
    2016-02-10  Markus Metzger  <markus.t.metzger@intel.com>
    
    gdb/
    	* frame.h (skip_tailcall_frames): Update comment.
    	* frame.c (skip_artificial_frames, skip_tailcall_frames): Return NULL
    	if only	artificial frames are found.  Update comment.
    	(frame_unwind_caller_id): Handle NULL return.
    	(frame_unwind_caller_pc, frame_unwind_caller_arch): Assert that
    	skip_artificial_frames does not return NULL.
    	(frame_pop): Add an error if only tailcall frames are found.
    	* infcmd.c (finish_command): Move skip_tailcall_frames call into forward-
    	execution case.  Add an error if only tailcall frames are found.
    
    testsuite/
    	* gdb.btrace/tailcall-only.exp: New.
    	* gdb.btrace/tailcall-only.c: New.
    	* gdb.btrace/x86_64-tailcall-only.S: New.
    	* gdb.btrace/i686-tailcall-only.S: New.

diff --git a/gdb/frame.c b/gdb/frame.c
index b7832c7..9bb493e 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -420,7 +420,8 @@ fprint_frame (struct ui_file *file, struct frame_info *fi)
 
 /* Given FRAME, return the enclosing frame as found in real frames read-in from
    inferior memory.  Skip any previous frames which were made up by GDB.
-   Return the original frame if no immediate previous frames exist.  */
+   Return FRAME if FRAME is a non-artificial frame.
+   Return NULL if FRAME is the start of an artificial-only chain.  */
 
 static struct frame_info *
 skip_artificial_frames (struct frame_info *frame)
@@ -428,12 +429,17 @@ skip_artificial_frames (struct frame_info *frame)
   /* Note we use get_prev_frame_always, and not get_prev_frame.  The
      latter will truncate the frame chain, leading to this function
      unintentionally returning a null_frame_id (e.g., when the user
-     sets a backtrace limit).  This is safe, because as these frames
-     are made up by GDB, there must be a real frame in the chain
-     below.  */
+     sets a backtrace limit).
+
+     Note that for record targets we may get a frame chain that consists
+     of artificial frames only.  */
   while (get_frame_type (frame) == INLINE_FRAME
 	 || get_frame_type (frame) == TAILCALL_FRAME)
-    frame = get_prev_frame_always (frame);
+    {
+      frame = get_prev_frame_always (frame);
+      if (frame == NULL)
+	break;
+    }
 
   return frame;
 }
@@ -444,7 +450,13 @@ struct frame_info *
 skip_tailcall_frames (struct frame_info *frame)
 {
   while (get_frame_type (frame) == TAILCALL_FRAME)
-    frame = get_prev_frame (frame);
+    {
+      /* Note that for record targets we may get a frame chain that consists of
+	 tailcall frames only.  */
+      frame = get_prev_frame (frame);
+      if (frame == NULL)
+	break;
+    }
 
   return frame;
 }
@@ -507,6 +519,9 @@ frame_unwind_caller_id (struct frame_info *next_frame)
      requests the frame ID of "main()"s caller.  */
 
   next_frame = skip_artificial_frames (next_frame);
+  if (next_frame == NULL)
+    return null_frame_id;
+
   this_frame = get_prev_frame_always (next_frame);
   if (this_frame)
     return get_frame_id (skip_artificial_frames (this_frame));
@@ -880,7 +895,14 @@ frame_unwind_pc (struct frame_info *this_frame)
 CORE_ADDR
 frame_unwind_caller_pc (struct frame_info *this_frame)
 {
-  return frame_unwind_pc (skip_artificial_frames (this_frame));
+  this_frame = skip_artificial_frames (this_frame);
+
+  /* We must have a non-artificial frame.  The caller is supposed to check
+     the result of frame_unwind_caller_id (), which returns NULL_FRAME_ID
+     in this case.  */
+  gdb_assert (this_frame != NULL);
+
+  return frame_unwind_pc (this_frame);
 }
 
 int
@@ -985,6 +1007,10 @@ frame_pop (struct frame_info *this_frame)
      entering THISFRAME.  */
   prev_frame = skip_tailcall_frames (prev_frame);
 
+  /* We cannot pop tailcall frames.  */
+  if (prev_frame == NULL)
+    error (_("Cannot pop a tailcall frame."));
+
   /* Make a copy of all the register values unwound from this frame.
      Save them in a scratch buffer so that there isn't a race between
      trying to extract the old values from the current regcache while
@@ -2571,7 +2597,14 @@ frame_unwind_arch (struct frame_info *next_frame)
 struct gdbarch *
 frame_unwind_caller_arch (struct frame_info *next_frame)
 {
-  return frame_unwind_arch (skip_artificial_frames (next_frame));
+  next_frame = skip_artificial_frames (next_frame);
+
+  /* We must have a non-artificial frame.  The caller is supposed to check
+     the result of frame_unwind_caller_id (), which returns NULL_FRAME_ID
+     in this case.  */
+  gdb_assert (next_frame != NULL);
+
+  return frame_unwind_arch (next_frame);
 }
 
 /* Gets the language of FRAME.  */
diff --git a/gdb/frame.h b/gdb/frame.h
index 7e8b01e..8ee64f1 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -821,7 +821,8 @@ extern int frame_unwinder_is (struct frame_info *fi,
 extern enum language get_frame_language (struct frame_info *frame);
 
 /* Return the first non-tailcall frame above FRAME or FRAME if it is not a
-   tailcall frame.  */
+   tailcall frame.  Return NULL if FRAME is the start of a tailcall-only
+   chain.  */
 
 extern struct frame_info *skip_tailcall_frames (struct frame_info *frame);
 
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 930dc61..938f8fa 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2000,10 +2000,6 @@ finish_command (char *arg, int from_tty)
       return;
     }
 
-  /* Ignore TAILCALL_FRAME type frames, they were executed already before
-     entering THISFRAME.  */
-  frame = skip_tailcall_frames (frame);
-
   /* Find the function we will return from.  */
 
   sm->function = find_pc_function (get_frame_pc (get_selected_frame (NULL)));
@@ -2030,7 +2026,16 @@ finish_command (char *arg, int from_tty)
   if (execution_direction == EXEC_REVERSE)
     finish_backward (sm);
   else
-    finish_forward (sm, frame);
+    {
+      /* Ignore TAILCALL_FRAME type frames, they were executed already before
+	 entering THISFRAME.  */
+      frame = skip_tailcall_frames (frame);
+
+      if (frame == NULL)
+	error (_("\"finish\" not meaningful for tailcall frames."));
+
+      finish_forward (sm, frame);
+    }
 }
 

 
diff --git a/gdb/testsuite/gdb.btrace/i686-tailcall-only.S b/gdb/testsuite/gdb.btrace/i686-tailcall-only.S
new file mode 100644
index 0000000..a89f2f2
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/i686-tailcall-only.S
@@ -0,0 +1,447 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 Free Software Foundation, Inc.
+
+   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/>.
+
+
+   This file has been generated using:
+   gcc -m32 -march=i686 -S -O2 -dA -g tailcall-only.c -o i686-tailcall-only.S
+ */
+
+	.file	"tailcall-only.c"
+	.text
+.Ltext0:
+	.p2align 4,,15
+	.type	bar_1, @function
+bar_1:
+.LFB0:
+	.file 1 "tailcall-only.c"
+	# tailcall-only.c:22
+	.loc 1 22 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:24
+	.loc 1 24 0
+	movl	$42, %eax
+# SUCC: EXIT [100.0%]
+	ret
+	.cfi_endproc
+.LFE0:
+	.size	bar_1, .-bar_1
+	.p2align 4,,15
+	.type	bar, @function
+bar:
+.LFB1:
+	# tailcall-only.c:28
+	.loc 1 28 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:29
+	.loc 1 29 0
+	jmp	bar_1
+# SUCC: EXIT [100.0%]  (ABNORMAL,SIBCALL)
+.LVL0:
+	.cfi_endproc
+.LFE1:
+	.size	bar, .-bar
+	.p2align 4,,15
+	.type	foo_1, @function
+foo_1:
+.LFB2:
+	# tailcall-only.c:34
+	.loc 1 34 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:35
+	.loc 1 35 0
+	jmp	bar
+# SUCC: EXIT [100.0%]  (ABNORMAL,SIBCALL)
+.LVL1:
+	.cfi_endproc
+.LFE2:
+	.size	foo_1, .-foo_1
+	.p2align 4,,15
+	.type	foo, @function
+foo:
+.LFB3:
+	# tailcall-only.c:40
+	.loc 1 40 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:41
+	.loc 1 41 0
+	jmp	foo_1
+# SUCC: EXIT [100.0%]  (ABNORMAL,SIBCALL)
+.LVL2:
+	.cfi_endproc
+.LFE3:
+	.size	foo, .-foo
+	.section	.text.startup,"ax",@progbits
+	.p2align 4,,15
+	.globl	main
+	.type	main, @function
+main:
+.LFB4:
+	# tailcall-only.c:46
+	.loc 1 46 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:49
+	.loc 1 49 0
+	call	foo
+.LVL3:
+	# tailcall-only.c:50
+	.loc 1 50 0
+	addl	$1, %eax
+.LVL4:
+# SUCC: EXIT [100.0%]
+	# tailcall-only.c:53
+	.loc 1 53 0
+	ret
+	.cfi_endproc
+.LFE4:
+	.size	main, .-main
+	.text
+.Letext0:
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.long	0xd5	# Length of Compilation Unit Info
+	.value	0x4	# DWARF version number
+	.long	.Ldebug_abbrev0	# Offset Into Abbrev. Section
+	.byte	0x4	# Pointer Size (in bytes)
+	.uleb128 0x1	# (DIE (0xb) DW_TAG_compile_unit)
+	.long	.LASF1	# DW_AT_producer: "GNU C 4.8.3 20140911 (Red Hat 4.8.3-9) -m32 -march=i686 -g -O2"
+	.byte	0x1	# DW_AT_language
+	.long	.LASF2	# DW_AT_name: "tailcall-only.c"
+	.long	.LASF3	# DW_AT_comp_dir: ""
+	.long	.Ldebug_ranges0+0	# DW_AT_ranges
+	.long	0	# DW_AT_low_pc
+	.long	.Ldebug_line0	# DW_AT_stmt_list
+	.uleb128 0x2	# (DIE (0x25) DW_TAG_subprogram)
+	.long	.LASF4	# DW_AT_name: "bar_1"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x15	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x3a	# DW_AT_type
+	.long	.LFB0	# DW_AT_low_pc
+	.long	.LFE0-.LFB0	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x3	# (DIE (0x3a) DW_TAG_base_type)
+	.byte	0x4	# DW_AT_byte_size
+	.byte	0x5	# DW_AT_encoding
+	.ascii "int\0"	# DW_AT_name
+	.uleb128 0x4	# (DIE (0x41) DW_TAG_subprogram)
+	.ascii "bar\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x1b	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x3a	# DW_AT_type
+	.long	.LFB1	# DW_AT_low_pc
+	.long	.LFE1-.LFB1	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.long	0x64	# DW_AT_sibling
+	.uleb128 0x5	# (DIE (0x5a) DW_TAG_GNU_call_site)
+	.long	.LVL0	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	0x25	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0x41
+	.uleb128 0x6	# (DIE (0x64) DW_TAG_subprogram)
+	.long	.LASF0	# DW_AT_name: "foo_1"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x21	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x3a	# DW_AT_type
+	.long	.LFB2	# DW_AT_low_pc
+	.long	.LFE2-.LFB2	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.long	0x87	# DW_AT_sibling
+	.uleb128 0x5	# (DIE (0x7d) DW_TAG_GNU_call_site)
+	.long	.LVL1	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	0x41	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0x64
+	.uleb128 0x4	# (DIE (0x87) DW_TAG_subprogram)
+	.ascii "foo\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x27	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x3a	# DW_AT_type
+	.long	.LFB3	# DW_AT_low_pc
+	.long	.LFE3-.LFB3	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.long	0xaa	# DW_AT_sibling
+	.uleb128 0x5	# (DIE (0xa0) DW_TAG_GNU_call_site)
+	.long	.LVL2	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	0x64	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0x87
+	.uleb128 0x7	# (DIE (0xaa) DW_TAG_subprogram)
+			# DW_AT_external
+	.long	.LASF5	# DW_AT_name: "main"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x2d	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x3a	# DW_AT_type
+	.long	.LFB4	# DW_AT_low_pc
+	.long	.LFE4-.LFB4	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x8	# (DIE (0xbf) DW_TAG_variable)
+	.long	.LASF6	# DW_AT_name: "answer"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x2f	# DW_AT_decl_line
+	.long	0x3a	# DW_AT_type
+	.long	.LLST0	# DW_AT_location
+	.uleb128 0x9	# (DIE (0xce) DW_TAG_GNU_call_site)
+	.long	.LVL3	# DW_AT_low_pc
+	.long	0x87	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0xaa
+	.byte	0	# end of children of DIE 0xb
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.uleb128 0x1	# (abbrev code)
+	.uleb128 0x11	# (TAG: DW_TAG_compile_unit)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x25	# (DW_AT_producer)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x13	# (DW_AT_language)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x1b	# (DW_AT_comp_dir)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x55	# (DW_AT_ranges)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x10	# (DW_AT_stmt_list)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.byte	0
+	.byte	0
+	.uleb128 0x2	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x6	# (DW_FORM_data4)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x3	# (abbrev code)
+	.uleb128 0x24	# (TAG: DW_TAG_base_type)
+	.byte	0	# DW_children_no
+	.uleb128 0xb	# (DW_AT_byte_size)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3e	# (DW_AT_encoding)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.byte	0
+	.byte	0
+	.uleb128 0x4	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x6	# (DW_FORM_data4)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x1	# (DW_AT_sibling)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x5	# (abbrev code)
+	.uleb128 0x4109	# (TAG: DW_TAG_GNU_call_site)
+	.byte	0	# DW_children_no
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x2115	# (DW_AT_GNU_tail_call)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x31	# (DW_AT_abstract_origin)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x6	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x6	# (DW_FORM_data4)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x1	# (DW_AT_sibling)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x7	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3f	# (DW_AT_external)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x6	# (DW_FORM_data4)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x8	# (abbrev code)
+	.uleb128 0x34	# (TAG: DW_TAG_variable)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x2	# (DW_AT_location)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.byte	0
+	.byte	0
+	.uleb128 0x9	# (abbrev code)
+	.uleb128 0x4109	# (TAG: DW_TAG_GNU_call_site)
+	.byte	0	# DW_children_no
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x31	# (DW_AT_abstract_origin)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.byte	0
+	.section	.debug_loc,"",@progbits
+.Ldebug_loc0:
+.LLST0:
+	.long	.LVL3	# Location list begin address (*.LLST0)
+	.long	.LVL4	# Location list end address (*.LLST0)
+	.value	0x3	# Location expression size
+	.byte	0x70	# DW_OP_breg0
+	.sleb128 1
+	.byte	0x9f	# DW_OP_stack_value
+	.long	.LVL4	# Location list begin address (*.LLST0)
+	.long	.LFE4	# Location list end address (*.LLST0)
+	.value	0x1	# Location expression size
+	.byte	0x50	# DW_OP_reg0
+	.long	0	# Location list terminator begin (*.LLST0)
+	.long	0	# Location list terminator end (*.LLST0)
+	.section	.debug_aranges,"",@progbits
+	.long	0x24	# Length of Address Ranges Info
+	.value	0x2	# DWARF Version
+	.long	.Ldebug_info0	# Offset of Compilation Unit Info
+	.byte	0x4	# Size of Address
+	.byte	0	# Size of Segment Descriptor
+	.value	0	# Pad to 8 byte boundary
+	.value	0
+	.long	.Ltext0	# Address
+	.long	.Letext0-.Ltext0	# Length
+	.long	.LFB4	# Address
+	.long	.LFE4-.LFB4	# Length
+	.long	0
+	.long	0
+	.section	.debug_ranges,"",@progbits
+.Ldebug_ranges0:
+	.long	.Ltext0	# Offset 0
+	.long	.Letext0
+	.long	.LFB4	# Offset 0x8
+	.long	.LFE4
+	.long	0
+	.long	0
+	.section	.debug_line,"",@progbits
+.Ldebug_line0:
+	.section	.debug_str,"MS",@progbits,1
+.LASF4:
+	.string	"bar_1"
+.LASF2:
+	.string	"tailcall-only.c"
+.LASF1:
+	.string	"GNU C 4.8.3 20140911 (Red Hat 4.8.3-9) -m32 -march=i686 -g -O2"
+.LASF6:
+	.string	"answer"
+.LASF5:
+	.string	"main"
+.LASF3:
+	.string	""
+.LASF0:
+	.string	"foo_1"
+	.ident	"GCC: (GNU) 4.8.3 20140911 (Red Hat 4.8.3-9)"
+	.section	.note.GNU-stack,"",@progbits
diff --git a/gdb/testsuite/gdb.btrace/tailcall-only.c b/gdb/testsuite/gdb.btrace/tailcall-only.c
new file mode 100644
index 0000000..5913ddc
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/tailcall-only.c
@@ -0,0 +1,53 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 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/>.  */
+
+static __attribute__ ((noinline)) int
+bar_1 (void)
+{
+  return 42;
+}
+
+static __attribute__ ((noinline)) int
+bar (void)
+{
+  return bar_1 ();
+}
+
+static __attribute__ ((noinline)) int
+foo_1 (void)
+{
+  return bar ();
+}
+
+static __attribute__ ((noinline)) int
+foo (void)
+{
+  return foo_1 ();
+}
+
+int
+main (void)
+{
+  int answer;
+
+  answer = foo ();
+  answer += 1;
+
+  return answer;
+}
diff --git a/gdb/testsuite/gdb.btrace/tailcall-only.exp b/gdb/testsuite/gdb.btrace/tailcall-only.exp
new file mode 100644
index 0000000..1137be1
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/tailcall-only.exp
@@ -0,0 +1,93 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2016 Free Software Foundation, Inc.
+#
+# 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/>.
+#
+#
+# This is a variant of tailcall.exp where the entire trace contains only tail
+# calls.  This used to cause a crash in get_frame_type.
+#
+
+# check for btrace support
+if { [skip_btrace_tests] } { return -1 }
+
+# This test requires the compiler to generate a tail call.  To guarantee that
+# we always get one, we use an assembly source file.
+#
+# We use different assembly sources based on the target architecture.
+#
+# Luckily, they are similar enough that a single test script can handle
+# both.
+set opts {}
+if [info exists COMPILE] {
+    # make check RUNTESTFLAGS="gdb.btrace/tailcall-only.exp COMPILE=1"
+    standard_testfile tailcall-only.c
+    lappend opts debug optimize=-O2
+} elseif {[istarget "x86_64-*-*"]} {
+	standard_testfile x86_64-tailcall-only.S
+} elseif {[istarget "i?86-*-*"]} {
+	standard_testfile i686-tailcall-only.S
+} else {
+    verbose "Skipping ${testfile}."
+    return
+}
+
+if [prepare_for_testing tailcall-only.exp $testfile $srcfile $opts] {
+    return -1
+}
+if ![runto_main] {
+    return -1
+}
+
+# we want to see the full trace for this test
+gdb_test_no_output "set record function-call-history-size 0"
+
+# trace foo
+gdb_test "step" ".*" "prepare for recording"
+gdb_test_no_output "record btrace"
+gdb_test "stepi 4" ".*" "record branch trace"
+
+# for debugging
+gdb_test "info record" ".*"
+
+# show the branch trace with calls indented
+gdb_test "record function-call-history /c 1" [multi_line \
+  "1\tfoo" \
+  "2\t  foo_1" \
+  "3\t    bar" \
+  "4\t      bar_1"
+  ] "function-call-history"
+
+# We can step
+gdb_test "record goto begin" ".*foo.*"
+gdb_test "stepi" ".*foo_1.*" "step into foo_1"
+gdb_test "step" ".*bar.*" "step into bar"
+gdb_test "stepi" ".*bar_1.*" "step into bar_1"
+
+# We can neither finish nor return.
+gdb_test "finish" "\"finish\" not meaningful for tailcall frames.*"
+gdb_test_multiple "return" "return" {
+  -re "Make .* return now.*y or n. $" {
+    send_gdb "y\n"
+    exp_continue
+  }
+  -re "Cannot pop a tailcall frame.*$gdb_prompt $" {
+    pass "return"
+  }
+}
+
+# But we can reverse-finish
+gdb_test "reverse-finish" ".*bar.*"
+gdb_test "reverse-step" ".*foo_1.*"
diff --git a/gdb/testsuite/gdb.btrace/x86_64-tailcall-only.S b/gdb/testsuite/gdb.btrace/x86_64-tailcall-only.S
new file mode 100644
index 0000000..710288a
--- /dev/null
+++ b/gdb/testsuite/gdb.btrace/x86_64-tailcall-only.S
@@ -0,0 +1,446 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2016 Free Software Foundation, Inc.
+
+   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/>.
+
+
+   This file has been generated using:
+   gcc -S -O2 -dA -g tailcall-only.c -o x86_64-tailcall-only.S  */
+
+	.file	"tailcall-only.c"
+	.text
+.Ltext0:
+	.p2align 4,,15
+	.type	bar_1, @function
+bar_1:
+.LFB0:
+	.file 1 "tailcall-only.c"
+	# tailcall-only.c:22
+	.loc 1 22 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:24
+	.loc 1 24 0
+	movl	$42, %eax
+# SUCC: EXIT [100.0%]
+	ret
+	.cfi_endproc
+.LFE0:
+	.size	bar_1, .-bar_1
+	.p2align 4,,15
+	.type	bar, @function
+bar:
+.LFB1:
+	# tailcall-only.c:28
+	.loc 1 28 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:29
+	.loc 1 29 0
+	jmp	bar_1
+# SUCC: EXIT [100.0%]  (ABNORMAL,SIBCALL)
+.LVL0:
+	.cfi_endproc
+.LFE1:
+	.size	bar, .-bar
+	.p2align 4,,15
+	.type	foo_1, @function
+foo_1:
+.LFB2:
+	# tailcall-only.c:34
+	.loc 1 34 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:35
+	.loc 1 35 0
+	jmp	bar
+# SUCC: EXIT [100.0%]  (ABNORMAL,SIBCALL)
+.LVL1:
+	.cfi_endproc
+.LFE2:
+	.size	foo_1, .-foo_1
+	.p2align 4,,15
+	.type	foo, @function
+foo:
+.LFB3:
+	# tailcall-only.c:40
+	.loc 1 40 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:41
+	.loc 1 41 0
+	jmp	foo_1
+# SUCC: EXIT [100.0%]  (ABNORMAL,SIBCALL)
+.LVL2:
+	.cfi_endproc
+.LFE3:
+	.size	foo, .-foo
+	.section	.text.startup,"ax",@progbits
+	.p2align 4,,15
+	.globl	main
+	.type	main, @function
+main:
+.LFB4:
+	# tailcall-only.c:46
+	.loc 1 46 0
+	.cfi_startproc
+# BLOCK 2 freq:10000 seq:0
+# PRED: ENTRY [100.0%]  (FALLTHRU)
+	# tailcall-only.c:49
+	.loc 1 49 0
+	call	foo
+.LVL3:
+	# tailcall-only.c:50
+	.loc 1 50 0
+	addl	$1, %eax
+.LVL4:
+# SUCC: EXIT [100.0%]
+	# tailcall-only.c:53
+	.loc 1 53 0
+	ret
+	.cfi_endproc
+.LFE4:
+	.size	main, .-main
+	.text
+.Letext0:
+	.section	.debug_info,"",@progbits
+.Ldebug_info0:
+	.long	0x111	# Length of Compilation Unit Info
+	.value	0x4	# DWARF version number
+	.long	.Ldebug_abbrev0	# Offset Into Abbrev. Section
+	.byte	0x8	# Pointer Size (in bytes)
+	.uleb128 0x1	# (DIE (0xb) DW_TAG_compile_unit)
+	.long	.LASF1	# DW_AT_producer: "GNU C 4.8.3 20140911 (Red Hat 4.8.3-9) -mtune=generic -march=x86-64 -g -O2"
+	.byte	0x1	# DW_AT_language
+	.long	.LASF2	# DW_AT_name: "tailcall-only.c"
+	.long	.LASF3	# DW_AT_comp_dir: ""
+	.long	.Ldebug_ranges0+0	# DW_AT_ranges
+	.quad	0	# DW_AT_low_pc
+	.long	.Ldebug_line0	# DW_AT_stmt_list
+	.uleb128 0x2	# (DIE (0x29) DW_TAG_subprogram)
+	.long	.LASF4	# DW_AT_name: "bar_1"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x15	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x46	# DW_AT_type
+	.quad	.LFB0	# DW_AT_low_pc
+	.quad	.LFE0-.LFB0	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x3	# (DIE (0x46) DW_TAG_base_type)
+	.byte	0x4	# DW_AT_byte_size
+	.byte	0x5	# DW_AT_encoding
+	.ascii "int\0"	# DW_AT_name
+	.uleb128 0x4	# (DIE (0x4d) DW_TAG_subprogram)
+	.ascii "bar\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x1b	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x46	# DW_AT_type
+	.quad	.LFB1	# DW_AT_low_pc
+	.quad	.LFE1-.LFB1	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.long	0x7c	# DW_AT_sibling
+	.uleb128 0x5	# (DIE (0x6e) DW_TAG_GNU_call_site)
+	.quad	.LVL0	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	0x29	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0x4d
+	.uleb128 0x6	# (DIE (0x7c) DW_TAG_subprogram)
+	.long	.LASF0	# DW_AT_name: "foo_1"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x21	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x46	# DW_AT_type
+	.quad	.LFB2	# DW_AT_low_pc
+	.quad	.LFE2-.LFB2	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.long	0xab	# DW_AT_sibling
+	.uleb128 0x5	# (DIE (0x9d) DW_TAG_GNU_call_site)
+	.quad	.LVL1	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	0x4d	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0x7c
+	.uleb128 0x4	# (DIE (0xab) DW_TAG_subprogram)
+	.ascii "foo\0"	# DW_AT_name
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x27	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x46	# DW_AT_type
+	.quad	.LFB3	# DW_AT_low_pc
+	.quad	.LFE3-.LFB3	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.long	0xda	# DW_AT_sibling
+	.uleb128 0x5	# (DIE (0xcc) DW_TAG_GNU_call_site)
+	.quad	.LVL2	# DW_AT_low_pc
+			# DW_AT_GNU_tail_call
+	.long	0x7c	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0xab
+	.uleb128 0x7	# (DIE (0xda) DW_TAG_subprogram)
+			# DW_AT_external
+	.long	.LASF5	# DW_AT_name: "main"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x2d	# DW_AT_decl_line
+			# DW_AT_prototyped
+	.long	0x46	# DW_AT_type
+	.quad	.LFB4	# DW_AT_low_pc
+	.quad	.LFE4-.LFB4	# DW_AT_high_pc
+	.uleb128 0x1	# DW_AT_frame_base
+	.byte	0x9c	# DW_OP_call_frame_cfa
+			# DW_AT_GNU_all_call_sites
+	.uleb128 0x8	# (DIE (0xf7) DW_TAG_variable)
+	.long	.LASF6	# DW_AT_name: "answer"
+	.byte	0x1	# DW_AT_decl_file (tailcall-only.c)
+	.byte	0x2f	# DW_AT_decl_line
+	.long	0x46	# DW_AT_type
+	.long	.LLST0	# DW_AT_location
+	.uleb128 0x9	# (DIE (0x106) DW_TAG_GNU_call_site)
+	.quad	.LVL3	# DW_AT_low_pc
+	.long	0xab	# DW_AT_abstract_origin
+	.byte	0	# end of children of DIE 0xda
+	.byte	0	# end of children of DIE 0xb
+	.section	.debug_abbrev,"",@progbits
+.Ldebug_abbrev0:
+	.uleb128 0x1	# (abbrev code)
+	.uleb128 0x11	# (TAG: DW_TAG_compile_unit)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x25	# (DW_AT_producer)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x13	# (DW_AT_language)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x1b	# (DW_AT_comp_dir)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x55	# (DW_AT_ranges)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x10	# (DW_AT_stmt_list)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.byte	0
+	.byte	0
+	.uleb128 0x2	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x3	# (abbrev code)
+	.uleb128 0x24	# (TAG: DW_TAG_base_type)
+	.byte	0	# DW_children_no
+	.uleb128 0xb	# (DW_AT_byte_size)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3e	# (DW_AT_encoding)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.byte	0
+	.byte	0
+	.uleb128 0x4	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0x8	# (DW_FORM_string)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x1	# (DW_AT_sibling)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x5	# (abbrev code)
+	.uleb128 0x4109	# (TAG: DW_TAG_GNU_call_site)
+	.byte	0	# DW_children_no
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x2115	# (DW_AT_GNU_tail_call)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x31	# (DW_AT_abstract_origin)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x6	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x1	# (DW_AT_sibling)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.uleb128 0x7	# (abbrev code)
+	.uleb128 0x2e	# (TAG: DW_TAG_subprogram)
+	.byte	0x1	# DW_children_yes
+	.uleb128 0x3f	# (DW_AT_external)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x27	# (DW_AT_prototyped)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x12	# (DW_AT_high_pc)
+	.uleb128 0x7	# (DW_FORM_data8)
+	.uleb128 0x40	# (DW_AT_frame_base)
+	.uleb128 0x18	# (DW_FORM_exprloc)
+	.uleb128 0x2117	# (DW_AT_GNU_all_call_sites)
+	.uleb128 0x19	# (DW_FORM_flag_present)
+	.byte	0
+	.byte	0
+	.uleb128 0x8	# (abbrev code)
+	.uleb128 0x34	# (TAG: DW_TAG_variable)
+	.byte	0	# DW_children_no
+	.uleb128 0x3	# (DW_AT_name)
+	.uleb128 0xe	# (DW_FORM_strp)
+	.uleb128 0x3a	# (DW_AT_decl_file)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x3b	# (DW_AT_decl_line)
+	.uleb128 0xb	# (DW_FORM_data1)
+	.uleb128 0x49	# (DW_AT_type)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.uleb128 0x2	# (DW_AT_location)
+	.uleb128 0x17	# (DW_FORM_sec_offset)
+	.byte	0
+	.byte	0
+	.uleb128 0x9	# (abbrev code)
+	.uleb128 0x4109	# (TAG: DW_TAG_GNU_call_site)
+	.byte	0	# DW_children_no
+	.uleb128 0x11	# (DW_AT_low_pc)
+	.uleb128 0x1	# (DW_FORM_addr)
+	.uleb128 0x31	# (DW_AT_abstract_origin)
+	.uleb128 0x13	# (DW_FORM_ref4)
+	.byte	0
+	.byte	0
+	.byte	0
+	.section	.debug_loc,"",@progbits
+.Ldebug_loc0:
+.LLST0:
+	.quad	.LVL3	# Location list begin address (*.LLST0)
+	.quad	.LVL4	# Location list end address (*.LLST0)
+	.value	0x3	# Location expression size
+	.byte	0x70	# DW_OP_breg0
+	.sleb128 1
+	.byte	0x9f	# DW_OP_stack_value
+	.quad	.LVL4	# Location list begin address (*.LLST0)
+	.quad	.LFE4	# Location list end address (*.LLST0)
+	.value	0x1	# Location expression size
+	.byte	0x50	# DW_OP_reg0
+	.quad	0	# Location list terminator begin (*.LLST0)
+	.quad	0	# Location list terminator end (*.LLST0)
+	.section	.debug_aranges,"",@progbits
+	.long	0x3c	# Length of Address Ranges Info
+	.value	0x2	# DWARF Version
+	.long	.Ldebug_info0	# Offset of Compilation Unit Info
+	.byte	0x8	# Size of Address
+	.byte	0	# Size of Segment Descriptor
+	.value	0	# Pad to 16 byte boundary
+	.value	0
+	.quad	.Ltext0	# Address
+	.quad	.Letext0-.Ltext0	# Length
+	.quad	.LFB4	# Address
+	.quad	.LFE4-.LFB4	# Length
+	.quad	0
+	.quad	0
+	.section	.debug_ranges,"",@progbits
+.Ldebug_ranges0:
+	.quad	.Ltext0	# Offset 0
+	.quad	.Letext0
+	.quad	.LFB4	# Offset 0x10
+	.quad	.LFE4
+	.quad	0
+	.quad	0
+	.section	.debug_line,"",@progbits
+.Ldebug_line0:
+	.section	.debug_str,"MS",@progbits,1
+.LASF4:
+	.string	"bar_1"
+.LASF2:
+	.string	"tailcall-only.c"
+.LASF1:
+	.string	"GNU C 4.8.3 20140911 (Red Hat 4.8.3-9) -mtune=generic -march=x86-64 -g -O2"
+.LASF6:
+	.string	"answer"
+.LASF5:
+	.string	"main"
+.LASF3:
+	.string	""
+.LASF0:
+	.string	"foo_1"
+	.ident	"GCC: (GNU) 4.8.3 20140911 (Red Hat 4.8.3-9)"
+	.section	.note.GNU-stack,"",@progbits

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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Pedro Alves]

On 02/10/2016 03:02 PM, Metzger, Markus T wrote:

> No new fails there, as well (64-bit IA).
> 
> I added a comment based on your statement that frame_unwind_caller_xxx
> callers should check frame_unwind_caller_id and assert that skip_artificial_frames
> does not return NULL.
> 
> Info frame doesn't crash.
> 
> 	(gdb) info frame
> 	Stack level 0, frame at 0x0:
> 	 rip = 0x4005b0 in bar (tailcall-only.c:29); saved rip = 0x4005c2
> 	 called by frame at 0x0
         ^^^^^^^^^^^^^^^

> 	 source 	language c.
> 	 Arglist at unknown address.
> 	 Locals at unknown address,Registers are not available in btrace record history
> 
> This is from a tailcall-only frame stack in replay mode using the tailcall-only test.
> The real caller has not been recorded.

Not sure how you got that, since "called by frame" seems to indicates that
the frame was not TAILCALL_FRAME:

  else if (get_frame_type (fi) == TAILCALL_FRAME)
    puts_filtered (" tail call frame");
  else if (get_frame_type (fi) == INLINE_FRAME)
    printf_filtered (" inlined into frame %d",
		     frame_relative_level (get_prev_frame (fi)));
  else
    {
      printf_filtered (" called by frame at ");
      fputs_filtered (paddress (gdbarch, get_frame_base (calling_frame_info)),
		      gdb_stdout);
    }


> 
> The output isn't very helpful for record btrace since we don't record register and
> memory changes.


So I'm mostly OK with the patch now, but I think you should dig a bit more
into the "info frame" output, since I think you _will_ internal error with a
TAILCALL_FRAME.

My remaining issue is now with the user-visible strings.

> @@ -985,6 +1007,10 @@ frame_pop (struct frame_info *this_frame)
>       entering THISFRAME.  */
>    prev_frame = skip_tailcall_frames (prev_frame);
>  
> +  /* We cannot pop tailcall frames.  */
> +  if (prev_frame == NULL)
> +    error (_("Cannot pop a tailcall frame."));
> +

I find this confusing, from a user perspective.  AFAIK, you can pop a tailcall
frame; what you can't do is pop when you don't know anything about the frame
that started the tail calling.  How about:

  if (prev_frame == NULL)
    error (_("Cannot return: tailcall caller frame not found."));

s/pop/return/, as "pop" is an internal implementation detail.

(I suggest also dropping the redundant comment.)


> +    {
> +      /* Ignore TAILCALL_FRAME type frames, they were executed already before
> +	 entering THISFRAME.  */
> +      frame = skip_tailcall_frames (frame);
> +
> +      if (frame == NULL)
> +	error (_("\"finish\" not meaningful for tailcall frames."));
> +

  if (frame == NULL)
    error (_("Cannot finish: tailcall caller frame not found."));


(I'd also be fine with dropping the "Cannot xxx:" part to make
the error messages the same in both cases.)


> +      finish_forward (sm, frame);
> +    }
>  }

Thanks,
Pedro Alves

RE: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Wednesday, February 10, 2016 4:34 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type
> 
> On 02/10/2016 03:02 PM, Metzger, Markus T wrote:
> 
> > No new fails there, as well (64-bit IA).
> >
> > I added a comment based on your statement that
> frame_unwind_caller_xxx
> > callers should check frame_unwind_caller_id and assert that
> > skip_artificial_frames does not return NULL.
> >
> > Info frame doesn't crash.
> >
> > 	(gdb) info frame
> > 	Stack level 0, frame at 0x0:
> > 	 rip = 0x4005b0 in bar (tailcall-only.c:29); saved rip = 0x4005c2
> > 	 called by frame at 0x0
>          ^^^^^^^^^^^^^^^
> 
> > 	 source 	language c.
> > 	 Arglist at unknown address.
> > 	 Locals at unknown address,Registers are not available in btrace
> > record history
> >
> > This is from a tailcall-only frame stack in replay mode using the tailcall-only
> test.
> > The real caller has not been recorded.
> 
> Not sure how you got that, since "called by frame" seems to indicates that
> the frame was not TAILCALL_FRAME:

That's the sentinel frame.  I forgot to "up".  Now it crashes;-)

There are other cases where frame_unwind_caller_xxx callers don't check
frame_unwind_caller_id:

	gdb/mips-linux-tdep.c
	gdb/glibc-tdep.c
	gdb/obsd-tdep.c
	gdb/tic6x-linux-tdep.c
	gdb/sol2-tdep.c
	gdb/nios2-linux-tdep.c

They're used for skipping syscalls and ld.so.

The latter should be called via gdbarch_skip_solib_resolver (gdbarch, stop_pc)
from infrun.c.

Who is supposed to do the check in those cases?  Maybe they are already OK?

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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Pedro Alves]

On 02/11/2016 09:51 AM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: Pedro Alves [mailto:palves@redhat.com]
>> Sent: Wednesday, February 10, 2016 4:34 PM
>> To: Metzger, Markus T <markus.t.metzger@intel.com>
>> Cc: gdb-patches@sourceware.org
>> Subject: Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type
>>
>> On 02/10/2016 03:02 PM, Metzger, Markus T wrote:
>>
>>> No new fails there, as well (64-bit IA).
>>>
>>> I added a comment based on your statement that
>> frame_unwind_caller_xxx
>>> callers should check frame_unwind_caller_id and assert that
>>> skip_artificial_frames does not return NULL.
>>>
>>> Info frame doesn't crash.
>>>
>>> 	(gdb) info frame
>>> 	Stack level 0, frame at 0x0:
>>> 	 rip = 0x4005b0 in bar (tailcall-only.c:29); saved rip = 0x4005c2
>>> 	 called by frame at 0x0
>>          ^^^^^^^^^^^^^^^
>>
>>> 	 source 	language c.
>>> 	 Arglist at unknown address.
>>> 	 Locals at unknown address,Registers are not available in btrace
>>> record history
>>>
>>> This is from a tailcall-only frame stack in replay mode using the tailcall-only
>> test.
>>> The real caller has not been recorded.
>>
>> Not sure how you got that, since "called by frame" seems to indicates that
>> the frame was not TAILCALL_FRAME:
> 
> That's the sentinel frame.  I forgot to "up".  Now it crashes;-)
> 
> There are other cases where frame_unwind_caller_xxx callers don't check
> frame_unwind_caller_id:
> 
> 	gdb/mips-linux-tdep.c
> 	gdb/glibc-tdep.c
> 	gdb/obsd-tdep.c
> 	gdb/tic6x-linux-tdep.c
> 	gdb/sol2-tdep.c
> 	gdb/nios2-linux-tdep.c
> 
> They're used for skipping syscalls and ld.so.
> 
> The latter should be called via gdbarch_skip_solib_resolver (gdbarch, stop_pc)
> from infrun.c.
> 
> Who is supposed to do the check in those cases?  Maybe they are already OK?

In the syscall cases, we're trying to determine the next PC where to place a
breakpoint, in order to do a software single-step.  If we don't know where the
caller is, we can't single-step, so we should probably error out.  OTOH, if the
target_ops is record-like and we're single-stepping through the trace log,
we shouldn't be trying to use software single-step at all.  So I think those
are probably OK.

In the glibc_skip_solib_resolver case -- in theory, I guess it would be
possible to construct a branch trace that records a tailcall to _dl_fixup,
and that doesn't have any frame above that one?

If we don't know where the caller is, we can't skip the resolver
in one go, so best to do is probably to return 0, and let infrun's
stepping logic continue single-stepping.

Thanks,
Pedro Alves

RE: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Metzger, Markus T]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Pedro Alves
> Sent: Thursday, February 11, 2016 2:39 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type


> > There are other cases where frame_unwind_caller_xxx callers don't
> > check
> > frame_unwind_caller_id:
> >
> > 	gdb/mips-linux-tdep.c
> > 	gdb/glibc-tdep.c
> > 	gdb/obsd-tdep.c
> > 	gdb/tic6x-linux-tdep.c
> > 	gdb/sol2-tdep.c
> > 	gdb/nios2-linux-tdep.c
> >
> > They're used for skipping syscalls and ld.so.
> >
> > The latter should be called via gdbarch_skip_solib_resolver (gdbarch,
> > stop_pc) from infrun.c.
> >
> > Who is supposed to do the check in those cases?  Maybe they are already
> OK?
> 
> In the syscall cases, we're trying to determine the next PC where to place a
> breakpoint, in order to do a software single-step.  If we don't know where
> the caller is, we can't single-step, so we should probably error out.  OTOH, if
> the target_ops is record-like and we're single-stepping through the trace log,
> we shouldn't be trying to use software single-step at all.  So I think those are
> probably OK.
> 
> In the glibc_skip_solib_resolver case -- in theory, I guess it would be possible
> to construct a branch trace that records a tailcall to _dl_fixup, and that
> doesn't have any frame above that one?

_dl_runtime_resolve would need to tail-call _dl_fixup.  I don't think it can since
it needs to (tail-)call the resolved function after _dl_fixup returns.  This should
be safe, as well, then.


> If we don't know where the caller is, we can't skip the resolver in one go, so
> best to do is probably to return 0, and let infrun's stepping logic continue
> single-stepping.

You think we should add the check nevertheless in gdb/glibc-tdep.c?

The others I won't be able to test.  I could do the changes and rely on buildbot
to flag issues.  If we really want to change them.


I put the "info frame" changes into a separate patch and a test into the
tailcall-only.exp test.  Here are the stack.c changes:


commit 0dc54d83f375e8901061244807e078f281fca070
Author: Markus Metzger <markus.t.metzger@intel.com>
Date:   Thu Feb 11 11:07:09 2016 +0100

    stack: check frame_unwind_caller_id
    
    Callers of frame_unwind_caller_* functions are supposed to check
    frame_unwind_caller_id.
    
    Add such a check to frame_info and treat an invalid caller ID as if the caller
    PC were not available.
    
    2016-02-11  Markus Metzger  <markus.t.metzger@intel.com>
    
    gdb/
    	* stack.c (frame_info): Check frame_unwind_caller_id.

diff --git a/gdb/stack.c b/gdb/stack.c
index 89879f3..6e3acc7 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1509,27 +1509,32 @@ frame_info (char *addr_exp, int from_tty)
   wrap_here ("    ");
   printf_filtered ("saved %s = ", pc_regname);
 
-  TRY
-    {
-      caller_pc = frame_unwind_caller_pc (fi);
-      caller_pc_p = 1;
-    }
-  CATCH (ex, RETURN_MASK_ERROR)
+  if (!frame_id_p (frame_unwind_caller_id (fi)))
+    val_print_unavailable (gdb_stdout);
+  else
     {
-      switch (ex.error)
+      TRY
 	{
-	case NOT_AVAILABLE_ERROR:
-	  val_print_unavailable (gdb_stdout);
-	  break;
-	case OPTIMIZED_OUT_ERROR:
-	  val_print_not_saved (gdb_stdout);
-	  break;
-	default:
-	  fprintf_filtered (gdb_stdout, _("<error: %s>"), ex.message);
-	  break;
+	  caller_pc = frame_unwind_caller_pc (fi);
+	  caller_pc_p = 1;
 	}
+      CATCH (ex, RETURN_MASK_ERROR)
+	{
+	  switch (ex.error)
+	    {
+	    case NOT_AVAILABLE_ERROR:
+	      val_print_unavailable (gdb_stdout);
+	      break;
+	    case OPTIMIZED_OUT_ERROR:
+	      val_print_not_saved (gdb_stdout);
+	      break;
+	    default:
+	      fprintf_filtered (gdb_stdout, _("<error: %s>"), ex.message);
+	      break;
+	    }
+	}
+      END_CATCH
     }
-  END_CATCH
 
   if (caller_pc_p)
     fputs_filtered (paddress (gdbarch, caller_pc), gdb_stdout);


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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Pedro Alves]

On 02/11/2016 03:42 PM, Metzger, Markus T wrote:

>> If we don't know where the caller is, we can't skip the resolver in one go, so
>> best to do is probably to return 0, and let infrun's stepping logic continue
>> single-stepping.
> 
> You think we should add the check nevertheless in gdb/glibc-tdep.c?

Let's leave it be.  If may be that the use case that trips on it would
be best fixed differently.

> 
> The others I won't be able to test.  I could do the changes and rely on buildbot
> to flag issues.  If we really want to change them.

I don't think we should change the others without evidence that they'd need it.
If we do trip on problems on those, I think the fix should be elsewhere.

I'd still early in the release cycle, let's wait and see.

> 
> commit 0dc54d83f375e8901061244807e078f281fca070
> Author: Markus Metzger <markus.t.metzger@intel.com>
> Date:   Thu Feb 11 11:07:09 2016 +0100
> 
>     stack: check frame_unwind_caller_id
>     
>     Callers of frame_unwind_caller_* functions are supposed to check
>     frame_unwind_caller_id.
>     
>     Add such a check to frame_info and treat an invalid caller ID as if the caller
>     PC were not available.
>     
>     2016-02-11  Markus Metzger  <markus.t.metzger@intel.com>
>     
>     gdb/
>     	* stack.c (frame_info): Check frame_unwind_caller_id.
> 

OK.

Thanks,
Pedro Alves

RE: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Thursday, February 11, 2016 4:58 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 3/3] btrace, frame: fix crash in get_frame_type


> >> If we don't know where the caller is, we can't skip the resolver in
> >> one go, so best to do is probably to return 0, and let infrun's
> >> stepping logic continue single-stepping.
> >
> > You think we should add the check nevertheless in gdb/glibc-tdep.c?
> 
> Let's leave it be.  If may be that the use case that trips on it would be best
> fixed differently.
> 
> >
> > The others I won't be able to test.  I could do the changes and rely
> > on buildbot to flag issues.  If we really want to change them.
> 
> I don't think we should change the others without evidence that they'd need
> it.
> If we do trip on problems on those, I think the fix should be elsewhere.
> 
> I'd still early in the release cycle, let's wait and see.

OK.  I'll send the complete series again.

Pathes 1 and 2 are unchanged and you've seen almost everything of patch 3,
already.  Actually, patch 2 has been replaced by a completely unrelated patch,
but you've OK'ed it already in your last reply.

Thanks a lot for your review and for pointing out issues!

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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

RE: [PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames

[Metzger, Markus T]

> -----Original Message-----
> From: Metzger, Markus T
> Sent: Tuesday, February 9, 2016 4:20 PM
> To: Pedro Alves <palves@redhat.com>; Joel Brobecker
> <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> Subject: RE: [PATCH v2 2/3] frame: use get_prev_frame_always in
> skip_tailcall_frames


> > > I'm beginning to wonder if not all-but-the-backtrace-command-related
> > > get_prev_frame calls should really be calling get_prev_frame_always.
> > >
> > > The _always extension isn't very intuitive, though, given that this
> > > should be the standard function to use.  Should get_prev_frame maybe
> > > be renamed to something like get_prev_frame_within_limit and
> > > get_prev_frame_always to get_prev_frame?
> >
> > Maybe a good idea.  See also:
> >
> >  https://sourceware.org/bugzilla/show_bug.cgi?id=15558
> >
> > I just noticed/remembered something -- the check for backtracing past
> > main and the entry point is in get_prev_frame, get_prev_frame_always
> > bypasses it.
> >
> > This means that with your change, I think gdb now allows "finish" in
> > "main" or in "_start".
> >
> > Maybe not a bad change, but I though it'd call it out explicitly.
> 
> There are 46 calls to get_prev_frame ignoring this patch.  Each of them
> should maybe be modified to call get_prev_frame_always, instead.  And
> none of this is currently covered by the test suite.

I'm wondering in which cases GDB should ignore the user-defined backtrace
limit.  And if GDB should ignore it at all.

If the limit is set, some aspects of GDB may not function any longer.  But that's
to be expected, isn't it?

GDB shouldn't crash, of course.  But I'm not sure if it should ignore user settings
in too many cases.  Maybe we should even switch back to get_prev_frame in
skip_artificial_frames and rely on handling the NULL return if we exceed the
backtrace limit?

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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames

[Pedro Alves]

On 02/15/2016 09:50 AM, Metzger, Markus T wrote:

> I'm wondering in which cases GDB should ignore the user-defined backtrace
> limit.  And if GDB should ignore it at all.
> 
> If the limit is set, some aspects of GDB may not function any longer.  But that's
> to be expected, isn't it?
> 
> GDB shouldn't crash, of course.  But I'm not sure if it should ignore user settings
> in too many cases.  

I'm starting to think the same way.  Want to give it a try and see what breaks?

> Maybe we should even switch back to get_prev_frame in
> skip_artificial_frames and rely on handling the NULL return if we exceed the
> backtrace limit?

If all places will end up throwing an error, it may be the better to
make skip_artificial_frames itself throw.  We'd have to take a deeper look at each
case though.

We need to also keep in mind that there may be cases where skip_artificial_frames might
be used in internal-facing code, where it might still be necessary get past inline
frames to reach the real stack frame.  I guess sticking a "set backtrace limit 1" in
some of the inline tests would expose this.

Thanks,
Pedro Alves

RE: [PATCH v2 2/3] frame: use get_prev_frame_always in skip_tailcall_frames

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Wednesday, February 17, 2016 4:32 PM
> To: Metzger, Markus T <markus.t.metzger@intel.com>; Joel Brobecker
> <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v2 2/3] frame: use get_prev_frame_always in
> skip_tailcall_frames
> 
> On 02/15/2016 09:50 AM, Metzger, Markus T wrote:
> 
> > I'm wondering in which cases GDB should ignore the user-defined
> > backtrace limit.  And if GDB should ignore it at all.
> >
> > If the limit is set, some aspects of GDB may not function any longer.
> > But that's to be expected, isn't it?
> >
> > GDB shouldn't crash, of course.  But I'm not sure if it should ignore
> > user settings in too many cases.
> 
> I'm starting to think the same way.  Want to give it a try and see what breaks?

diff --git a/gdb/frame.c b/gdb/frame.c
index d621dd7..f436010 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -436,7 +436,7 @@ skip_artificial_frames (struct frame_info *frame)
   while (get_frame_type (frame) == INLINE_FRAME
         || get_frame_type (frame) == TAILCALL_FRAME)
     {
-      frame = get_prev_frame_always (frame);
+      frame = get_prev_frame (frame);
       if (frame == NULL)
        break;
     }

With this change, the regression tests for PR backtrace/15558 in
gdb.opt/inline-bt.exp and gdb.python/py-frame-inline.exp still pass.

I have not debugged it but with the recent changes skip_artificial_frames
should return NULL which should be handled by its callers and this seems
to do the right thing for those tests.

Or maybe they just rely on get_prev_frame_always in inline_frame_this_id.
Which makes you wonder why skip_artificial_frames was changed as part
of the bug-fix.

With skip_artificial_frames not unwinding past the backtrace limit, it
doesn't really make sense to call get_prev_frame_always in
frame_unwind_caller_id and frame_pop.

I changed those back to get_prev_frame but I left get_prev_frame_always
in inline_frame_this_id.  Curiously, tailcall_frame_this_id uses get_prev_frame
instead of get_prev_frame_always.  I somehow expected those to be aligned.

When I add a backtrace limit of 1 to the following tests...

#       modified:   gdb/testsuite/gdb.ada/out_of_line_in_inlined.exp
#       modified:   gdb/testsuite/gdb.arch/amd64-tailcall-cxx.exp
#       modified:   gdb/testsuite/gdb.arch/amd64-tailcall-noret.exp
#       modified:   gdb/testsuite/gdb.arch/amd64-tailcall-ret.exp
#       modified:   gdb/testsuite/gdb.arch/amd64-tailcall-self.exp
#       modified:   gdb/testsuite/gdb.base/finish.exp
#       modified:   gdb/testsuite/gdb.base/return.exp
#       modified:   gdb/testsuite/gdb.base/return2.exp
#       modified:   gdb/testsuite/gdb.dwarf2/dw2-inline-param.exp

(gdb.opt/inline-bt.exp already sets a backtrace limit)

...I get a few new fails in

gdb.base/finish.exp
gdb.base/return.exp
gdb.base/return2.exp
gdb.arch/amd64-tailcall-ret.exp

	Just needs handling the error messages we now get when the
	finish and return commands fail when they can no longer skip
	the tailcall frames.

gdb.arch/amd64-tailcall-cxx.exp

	This one is more interesting.  The backtrace output changes from:

	#0  g (x=x@entry=2) at gdb.arch/amd64-tailcall-cxx1.cc:23

	to

	#0  g (x=2) at gdb.arch/amd64-tailcall-cxx1.cc:23

	I have not looked into it.

The rest doesn't give any new fails.

Adding a backtrace limit of 1 to gdb.opt/inline-cmds.exp causes most of the
tests to fail.  It looks like stepping commands are affected by such an aggressive
backtrace limit.

Looking at infrun, GDB uses frame_unwind_caller_id and get_prev_frame (in
stepped_in_from) to detect calls.  Those should depend on the backtrace limit.


> We need to also keep in mind that there may be cases where
> skip_artificial_frames might be used in internal-facing code, where it might
> still be necessary get past inline frames to reach the real stack frame.  I guess
> sticking a "set backtrace limit 1" in some of the inline tests would expose this.

Stepping should break with such an aggressive backtrace limit.  So I looked into
tail-called and inlined functions.

For tailcalls, I was not able to "next" over a tail-called function.  I always ended
up in the function I called.  Even without a backtrace limit and with
skip_artificial_frames use get_prev_frame_always.  Is this the expected behavior?

For inlined functions, GDB crashes in an inline-frame-skipping loop that uses
get_prev_frame in dwarf2_frame_cfa.

With that fixed I would imagine that for every backtrace limit you could construct
a case where "next" over an inlined function breaks by stacking enough inlined
function calls.  I have not tried it.

For normal calls, a backtrace limit of 2 should suffice.  I have not tried it.

The question remains whether the backtrace limit should just cap the output of
the "backtrace" command or whether it should be a global limit.  If it were just the
former I would have expected the limit check to be in the "backtrace" command
function.

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, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928