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

["Metzger, Markus T" <markus.t.metzger@intel.com>]

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