[patch 04/12] entryval: Virtual tail call frames

[patch 04/12] entryval: Virtual tail call frames

[Jan Kratochvil]

Hi,

the testcase:
#0  d (i=<optimized out>) at ./gdb.arch/amd64-entry-value.cc:33
#1  0x00000000004003ce in main () at ./gdb.arch/amd64-entry-value.cc:100
->
#0  d (i=71) at ./gdb.arch/amd64-entry-value.cc:33
#1  0x0000000000400527 in c (i=7) at ./gdb.arch/amd64-entry-value.cc:39
#2  0x0000000000400545 in b (i=5) at ./gdb.arch/amd64-entry-value.cc:51
#3  0x00000000004003ee in main () at ./gdb.arch/amd64-entry-value.cc:100

you can see that by finding the virtual tail call frames we can match
callers-callees better and therefore even better recover the parameter values.

In some cases there can be ambiguity:
#0  d (i=<optimized out>) at ./gdb.arch/amd64-entry-value.cc:33
#1  0x0000000000400555 in amb_z (i=<optimized out>) at ./gdb.arch/amd64-entry-value.cc:57
#2  0x0000000000400565 in amb_y (i=<optimized out>) at ./gdb.arch/amd64-entry-value.cc:63
#3  0x0000000000400575 in amb_x (i=<optimized out>) at ./gdb.arch/amd64-entry-value.cc:69
#4  0x0000000000400595 in amb_b (i=101) at ./gdb.arch/amd64-entry-value.cc:84
#5  0x00000000004005a5 in amb_a (i=100) at ./gdb.arch/amd64-entry-value.cc:90
#6  0x00000000004003f8 in main () at ./gdb.arch/amd64-entry-value.cc:101

Between frame #3 and frame #4 is function `amb' but one cannot reliably
reconstruct its exact PC.  The code tries to determine only a sequence of
unambiguous bottom calees and top callers and anything in between is dropped.
Currently GDB knows that between frame #3 and frame #4 it is uncertain.
Former patch tried to display some "???" there.  It had a real frame_info,
later I removed it, I think such "???" marker should never have its real
frame_info inside GDB.  Still the backtracing function could display some
uncertainty indicator for the user; this is currently not implemented.
GDB already behaves such way without trying to recover any frames info.


Thanks,
Jan


gdb/
2011-07-18  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Recognize virtual tail call frames.
	* Makefile.in (SFILES): Add dwarf2-frame-tailcall.c.
	(COMMON_OBS): Add dwarf2-frame-tailcall.o.
	* dwarf2-frame-tailcall.c: New file.
	* dwarf2-frame-tailcall.h: New file.
	* dwarf2-frame.c: Include dwarf2-frame-tailcall.h.
	(struct dwarf2_frame_cache): New field tailcall_cache.
	(dwarf2_frame_cache): Call dwarf2_tailcall_sniffer_first.
	(dwarf2_frame_prev_register): Call
	dwarf2_tailcall_frame_unwind.prev_register when appropriate.
	(dwarf2_frame_dealloc_cache): New function.
	(dwarf2_frame_sniffer): Preinitialize cache by dwarf2_frame_cache.
	(dwarf2_frame_unwind): Install dwarf2_frame_dealloc_cache.
	(dwarf2_signal_frame_unwind): Do not install dwarf2_frame_dealloc_cache.
	(dwarf2_append_unwinders): Add dwarf2_tailcall_frame_unwind.
	(dwarf2_frame_cfa): Support also dwarf2_tailcall_frame_unwind.
	* dwarf2loc.c: Include gdbcmd.h.
	(tailcall_debug, show_tailcall_debug, func_addr_to_tail_call_list)
	(tailcall_dump, call_sitep, VEC (call_sitep), chain_candidate)
	(free_call_sitep_vecp, call_site_find_chain_1, call_site_find_chain)
	(_initialize_dwarf2loc): New.
	* dwarf2loc.h (struct call_site_chain): New.
	(call_site_find_chain): New declaration.
	* frame.c (get_frame_address_in_block): Support also TAILCALL_FRAME.
	* frame.h (enum frame_type): New entry TAILCALL_FRAME.
	* stack.c (frame_info): Support also TAILCALL_FRAME.

gdb/testsuite/
2011-07-18  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Recognize virtual tail call frames.
	* gdb.arch/amd64-entry-value.cc (c, a, b, amb_z, amb_y, amb_x, amb)
	(amb_b, amb_a): New.
	(main): Call a and b.
	* gdb.arch/amd64-entry-value.exp (tailcall: breakhere, tailcall: bt)
	(tailcall: p i, ambiguous: breakhere, ambiguous: bt): New tests.

--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -694,6 +694,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	cp-name-parser.y \
 	dbxread.c demangle.c dictionary.c disasm.c doublest.c dummy-frame.c \
 	dwarf2expr.c dwarf2loc.c dwarf2read.c dwarf2-frame.c \
+	dwarf2-frame-tailcall.c \
 	elfread.c environ.c eval.c event-loop.c event-top.c \
 	exceptions.c expprint.c \
 	f-exp.y f-lang.c f-typeprint.c f-valprint.c filesystem.c \
@@ -874,7 +875,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	bcache.o objfiles.o observer.o minsyms.o maint.o demangle.o \
 	dbxread.o coffread.o coff-pe-read.o \
 	dwarf2read.o mipsread.o stabsread.o corefile.o \
-	dwarf2expr.o dwarf2loc.o dwarf2-frame.o \
+	dwarf2expr.o dwarf2loc.o dwarf2-frame.o dwarf2-frame-tailcall.o \
 	ada-lang.o c-lang.o d-lang.o f-lang.o objc-lang.o \
 	ada-tasks.o \
 	ui-out.o cli-out.o \
--- /dev/null
+++ b/gdb/dwarf2-frame-tailcall.c
@@ -0,0 +1,415 @@
+/* Virtual tail call frames unwinder for GDB.
+
+   Copyright (C) 2010, 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+#include "defs.h"
+#include "gdb_assert.h"
+#include "frame.h"
+#include "dwarf2-frame-tailcall.h"
+#include "dwarf2loc.h"
+#include "frame-unwind.h"
+#include "block.h"
+#include "hashtab.h"
+#include "exceptions.h"
+#include "gdbtypes.h"
+
+
+/* Contains struct tailcall_cache indexed by next_bottom_frame.  */
+static htab_t cache_htab;
+
+/* Associated structure of the unwinder for call_site_chain.  */
+
+struct tailcall_cache
+{
+  /* It must be the first one of this struct.  It is the furthest callee.  */
+  struct frame_info *next_bottom_frame;
+
+  /* Reference count.  The whole chain of virtual tail call frames shares one
+     tailcall_cache.  */
+  int refc;
+
+  /* Associated found virtual taill call frames chain, it is never NULL.  */
+  struct call_site_chain *chain;
+
+  /* Cached pretended_chain_levels result.  */
+  int chain_levels;
+
+  /* Unwound PC from the top (caller) frame, as it is not contained
+     in CHAIN.  */
+
+  CORE_ADDR prev_pc;
+};
+
+/* hash_f for htab_create_alloc of cache_htab.  */
+
+static hashval_t
+cache_hash (const void *arg)
+{
+  const struct tailcall_cache *cache = arg;
+
+  return htab_hash_pointer (cache->next_bottom_frame);
+}
+
+/* eq_f for htab_create_alloc of cache_htab.  */
+
+static int
+cache_eq (const void *arg1, const void *arg2)
+{
+  const struct tailcall_cache *cache1 = arg1;
+  const struct tailcall_cache *cache2 = arg2;
+
+  return cache1->next_bottom_frame == cache2->next_bottom_frame;
+}
+
+/* Create new tailcall_cache for NEXT_BOTTOM_FRAME, NEXT_BOTTOM_FRAME must not
+   yet have been indexed by cache_htab.  Caller holds one reference of the new
+   tailcall_cache.  */
+
+static struct tailcall_cache *
+cache_new_ref1 (struct frame_info *next_bottom_frame)
+{
+  struct tailcall_cache *cache;
+  void **slot;
+
+  cache = xmalloc (sizeof (*cache));
+
+  cache->next_bottom_frame = next_bottom_frame;
+  cache->refc = 1;
+
+  slot = htab_find_slot (cache_htab, cache, INSERT);
+  gdb_assert (*slot == NULL);
+  *slot = cache;
+
+  return cache;
+}
+
+/* Create new reference to CACHE.  */
+
+static void
+cache_ref (struct tailcall_cache *cache)
+{
+  gdb_assert (cache->refc > 0);
+
+  cache->refc++;
+}
+
+/* Drop reference to CACHE, possibly fully freeing it and unregistering it from
+   cache_htab.  */
+
+static void
+cache_unref (struct tailcall_cache *cache)
+{
+  gdb_assert (cache->refc > 0);
+
+  if (!--cache->refc)
+    {
+      gdb_assert (htab_find_slot (cache_htab, cache, NO_INSERT) != NULL);
+      htab_remove_elt (cache_htab, cache);
+
+      xfree (cache->chain);
+      xfree (cache);
+    }
+}
+
+/* Return 1 if FI is a non-bottom (not the callee) tail call frame.  Otherwise
+   return 0.  */
+
+static int
+frame_is_tailcall (struct frame_info *fi)
+{
+  return frame_unwinder_is (fi, &dwarf2_tailcall_frame_unwind);
+}
+
+/* Try to find tailcall_cache in cache_htab if FI is a part of its virtual tail
+   call chain.  Otherwise return NULL.  No new reference is created.  */
+
+static struct tailcall_cache *
+cache_find (struct frame_info *fi)
+{
+  struct tailcall_cache *cache;
+  void **slot;
+
+  while (frame_is_tailcall (fi))
+    {
+      fi = get_next_frame (fi);
+      gdb_assert (fi != NULL);
+    }
+
+  slot = htab_find_slot (cache_htab, &fi, NO_INSERT);
+  if (slot == NULL)
+    return NULL;
+
+  cache = *slot;
+  gdb_assert (cache != NULL);
+  return cache;
+}
+
+/* Number of virtual frames between THIS_FRAME and CACHE->NEXT_BOTTOM_FRAME.
+   If THIS_FRAME is CACHE-> NEXT_BOTTOM_FRAME return -1.  */
+
+static int
+existing_next_levels (struct frame_info *this_frame,
+		      struct tailcall_cache *cache)
+{
+  int retval = (frame_relative_level (this_frame)
+		- frame_relative_level (cache->next_bottom_frame) - 1);
+
+  gdb_assert (retval >= -1);
+
+  return retval;
+}
+
+/* The number of virtual tail call frames in CHAIN.  With no virtual tail call
+   frames the function would return 0 (but CHAIN does not exist in such
+   case).  */
+
+static int
+pretended_chain_levels (struct call_site_chain *chain)
+{
+  int chain_levels;
+
+  gdb_assert (chain != NULL);
+
+  if (chain->callers == chain->length && chain->callees == chain->length)
+    return chain->length;
+
+  chain_levels = chain->callers + chain->callees;
+  gdb_assert (chain_levels < chain->length);
+
+  return chain_levels;
+}
+
+/* Implementation of frame_this_id_ftype.  THIS_CACHE must be already
+   initialized with tailcall_cache, THIS_FRAME must be a part of THIS_CACHE.
+
+   Specific virtual tail call frames are tracked by INLINE_DEPTH.  */
+
+static void
+tailcall_frame_this_id (struct frame_info *this_frame, void **this_cache,
+			struct frame_id *this_id)
+{
+  struct tailcall_cache *cache = *this_cache;
+  struct frame_info *next_frame;
+  CORE_ADDR this_frame_base;
+
+  /* Tail call does not make sense for a sentinel frame.  */
+  next_frame = get_next_frame (this_frame);
+  gdb_assert (next_frame != NULL);
+
+  /* SP does not change during tail calls.  */
+  this_frame_base = get_frame_base (next_frame);
+
+  (*this_id) = frame_id_build (this_frame_base, get_frame_pc (this_frame));
+  (*this_id).inline_depth = (cache->chain_levels
+			     - existing_next_levels (this_frame, cache));
+  gdb_assert ((*this_id).inline_depth > 0);
+}
+
+/* Find PC to be unwound from THIS_FRAME.  THIS_FRAME must be a part of
+   CACHE.  */
+
+static CORE_ADDR
+pretend_pc (struct frame_info *this_frame, struct tailcall_cache *cache)
+{
+  int next_levels = existing_next_levels (this_frame, cache);
+  struct call_site_chain *chain = cache->chain;
+  int caller_no;
+
+  gdb_assert (chain != NULL);
+
+  next_levels++;
+  gdb_assert (next_levels >= 0);
+
+  if (next_levels < chain->callees)
+    return chain->call_site[chain->length - next_levels - 1]->pc;
+  next_levels -= chain->callees;
+
+  /* Otherwise CHAIN->CALLEES are already covered by CHAIN->CALLERS.  */
+  if (chain->callees != chain->length)
+    {
+      if (next_levels < chain->callers)
+	return chain->call_site[chain->callers - next_levels - 1]->pc;
+      next_levels -= chain->callers;
+    }
+
+  gdb_assert (next_levels == 0);
+  return cache->prev_pc;
+}
+
+/* Implementation of frame_prev_register_ftype.  Register set of virtual tail
+   call frames is assumed to be the one of the top (caller) frame.  Only PC
+   value can be different for virtual tail call frames.  */
+
+static struct value *
+tailcall_frame_prev_register (struct frame_info *this_frame,
+			       void **this_cache, int regnum)
+{
+  struct gdbarch *this_gdbarch = get_frame_arch (this_frame);
+  struct tailcall_cache *cache = *this_cache;
+
+  if (regnum == gdbarch_pc_regnum (this_gdbarch))
+    return frame_unwind_got_constant (this_frame, regnum,
+				      pretend_pc (this_frame, cache));
+
+  return frame_unwind_got_register (this_frame, regnum, regnum);
+}
+
+/* Implementation of frame_sniffer_ftype.  It will never find a new chain, use
+   dwarf2_tailcall_sniffer_first for the bottom (callee) frame.  It will find
+   all the predecessing virtual tail call frames, it will return false when
+   there exist no more tail call frames in this chain.  */
+
+static int
+tailcall_frame_sniffer (const struct frame_unwind *self,
+			 struct frame_info *this_frame, void **this_cache)
+{
+  struct frame_info *next_frame;
+  int next_levels;
+  struct tailcall_cache *cache;
+
+  /* Inner tail call element does not make sense for a sentinel frame.  */
+  next_frame = get_next_frame (this_frame);
+  if (next_frame == NULL)
+    return 0;
+
+  cache = cache_find (next_frame);
+  if (cache == NULL)
+    return 0;
+
+  cache_ref (cache);
+
+  next_levels = existing_next_levels (this_frame, cache);
+
+  /* NEXT_LEVELS is -1 only in dwarf2_tailcall_sniffer_first.  */
+  gdb_assert (next_levels >= 0);
+  gdb_assert (next_levels <= cache->chain_levels);
+
+  if (next_levels == cache->chain_levels)
+    {
+      cache_unref (cache);
+      return 0;
+    }
+
+  *this_cache = cache;
+  return 1;
+}
+
+/* The initial "sniffer" whether THIS_FRAME is a bottom (callee) frame of a new
+   chain to create.  Keep TAILCALL_CACHEP NULL if it did not find any chain,
+   initialize it otherwise.  No tail call chain is created if there are no
+   unambiguous virtual tail call frames to report.  */
+
+void
+dwarf2_tailcall_sniffer_first (struct frame_info *this_frame,
+			       void **tailcall_cachep)
+{
+  CORE_ADDR prev_pc = 0;	/* GCC warning.  */
+  CORE_ADDR this_pc, pc;
+  struct gdbarch *prev_gdbarch;
+  struct call_site_chain *chain = NULL;
+  struct frame_info *fi;
+  struct tailcall_cache *cache;
+  int pc_regnum;
+  volatile struct gdb_exception except;
+
+  gdb_assert (*tailcall_cachep == NULL);
+
+  this_pc = get_frame_pc (this_frame);
+
+  /* Catch any unwinding errors.  */
+  TRY_CATCH (except, RETURN_MASK_ERROR)
+    {
+      prev_gdbarch = frame_unwind_arch (this_frame);
+      pc_regnum = gdbarch_pc_regnum (prev_gdbarch);
+      if (pc_regnum == -1)
+	break;
+
+      /* Simulate frame_unwind_pc without setting this_frame->prev_pc.p.  */
+      prev_pc = frame_unwind_register_unsigned (this_frame, pc_regnum);
+      if (call_site_for_pc (prev_pc) == NULL)
+	break;
+
+      /* call_site_find_chain can throw an exception.  */
+      chain = call_site_find_chain (prev_gdbarch, prev_pc, this_pc);
+    }
+  if (except.reason < 0)
+    return;
+
+  /* Ambiguous unwind or unambiguous unwind verified as matching.  */
+  if (chain == NULL || chain->length == 0)
+    {
+      xfree (chain);
+      return;
+    }
+
+  cache = cache_new_ref1 (this_frame);
+  *tailcall_cachep = cache;
+  cache->chain = chain;
+  cache->prev_pc = prev_pc;
+  cache->chain_levels = pretended_chain_levels (chain);
+  gdb_assert (cache->chain_levels > 0);
+}
+
+/* Implementation of frame_dealloc_cache_ftype.  It can be called even for the
+   bottom chain frame from dwarf2_frame_dealloc_cache which is not a real
+   TAILCALL_FRAME.  */
+
+static void
+tailcall_frame_dealloc_cache (struct frame_info *self, void *this_cache)
+{
+  struct tailcall_cache *cache = this_cache;
+
+  cache_unref (cache);
+}
+
+/* Implementation of frame_prev_arch_ftype.  We assume all the virtual tail
+   call frames have gdbarch of the bottom (callee) frame.  */
+
+static struct gdbarch *
+tailcall_frame_prev_arch (struct frame_info *this_frame,
+			  void **this_prologue_cache)
+{
+  struct tailcall_cache *cache = *this_prologue_cache;
+
+  return get_frame_arch (cache->next_bottom_frame);
+}
+
+/* Virtual tail call frame unwinder if dwarf2_tailcall_sniffer_first finds
+   a chain to create.  */
+
+const struct frame_unwind dwarf2_tailcall_frame_unwind =
+{
+  TAILCALL_FRAME,
+  default_frame_unwind_stop_reason,
+  tailcall_frame_this_id,
+  tailcall_frame_prev_register,
+  NULL,
+  tailcall_frame_sniffer,
+  tailcall_frame_dealloc_cache,
+  tailcall_frame_prev_arch
+};
+
+/* Provide a prototype to silence -Wmissing-prototypes.  */
+extern initialize_file_ftype _initialize_tailcall_frame;
+
+void
+_initialize_tailcall_frame (void)
+{
+  cache_htab = htab_create_alloc (50, cache_hash, cache_eq, NULL, xcalloc,
+				  xfree);
+}
--- /dev/null
+++ b/gdb/dwarf2-frame-tailcall.h
@@ -0,0 +1,33 @@
+/* Definitions for virtual tail call frames unwinder for GDB.
+
+   Copyright (C) 2010, 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+#ifndef DWARF2_FRAME_TAILCALL_H
+#define DWARF2_FRAME_TAILCALL_H 1
+
+struct frame_info;
+struct frame_unwind;
+
+/* The tail call frame unwinder.  */
+
+extern void dwarf2_tailcall_sniffer_first (struct frame_info *this_frame,
+					   void **tailcall_cachep);
+
+extern const struct frame_unwind dwarf2_tailcall_frame_unwind;
+
+#endif /* !DWARF2_FRAME_TAILCALL_H */
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -41,6 +41,7 @@
 #include "ax.h"
 #include "dwarf2loc.h"
 #include "exceptions.h"
+#include "dwarf2-frame-tailcall.h"
 
 struct comp_unit;
 
@@ -1034,6 +1035,13 @@ struct dwarf2_frame_cache
 
   /* The .text offset.  */
   CORE_ADDR text_offset;
+
+  /* If not NULL then this frame is the bottom frame of a TAILCALL_FRAME
+     sequence.  If NULL then it is a normal case with no TAILCALL_FRAME
+     involved.  Non-bottom frames of a virtual tail call frames chain use
+     dwarf2_tailcall_frame_unwind unwinder so this field does not apply for
+     them.  */
+  void *tailcall_cache;
 };
 
 static struct dwarf2_frame_cache *
@@ -1240,6 +1248,10 @@ incomplete CFI data; unspecified registers (e.g., %s) at %s"),
 
   do_cleanups (old_chain);
 
+  /* Try to find a virtual tail call frames chain with bottom (callee) frame
+     starting at THIS_FRAME.  */
+  dwarf2_tailcall_sniffer_first (this_frame, &cache->tailcall_cache);
+
   return cache;
 }
 
@@ -1285,6 +1297,15 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
   CORE_ADDR addr;
   int realnum;
 
+  /* Virtual tail call frames report different values only for PC.  Non-bottom
+     frames of a virtual tail call frames chain use
+     dwarf2_tailcall_frame_unwind unwinder so this code does not apply for
+     them.  */
+  if (cache->tailcall_cache && regnum == gdbarch_pc_regnum (gdbarch))
+    return dwarf2_tailcall_frame_unwind.prev_register (this_frame,
+						       &cache->tailcall_cache,
+						       regnum);
+
   switch (cache->reg[regnum].how)
     {
     case DWARF2_FRAME_REG_UNDEFINED:
@@ -1354,6 +1375,18 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
     }
 }
 
+/* Proxy for tailcall_frame_dealloc_cache for bottom frame of a virtual tail
+   call frames chain.  */
+
+static void
+dwarf2_frame_dealloc_cache (struct frame_info *self, void *this_cache)
+{
+  struct dwarf2_frame_cache *cache = dwarf2_frame_cache (self, &this_cache);
+
+  if (cache->tailcall_cache)
+    dwarf2_tailcall_frame_unwind.dealloc_cache (self, cache->tailcall_cache);
+}
+
 static int
 dwarf2_frame_sniffer (const struct frame_unwind *self,
 		      struct frame_info *this_frame, void **this_cache)
@@ -1380,7 +1413,14 @@ dwarf2_frame_sniffer (const struct frame_unwind *self,
 				      this_frame))
     return self->type == SIGTRAMP_FRAME;
 
-  return self->type != SIGTRAMP_FRAME;
+  if (self->type != NORMAL_FRAME)
+    return 0;
+
+  /* Preinitializa the cache so that TAILCALL_FRAME can find the record by
+     dwarf2_tailcall_sniffer_first.  */
+  dwarf2_frame_cache (this_frame, this_cache);
+
+  return 1;
 }
 
 static const struct frame_unwind dwarf2_frame_unwind =
@@ -1390,7 +1430,8 @@ static const struct frame_unwind dwarf2_frame_unwind =
   dwarf2_frame_this_id,
   dwarf2_frame_prev_register,
   NULL,
-  dwarf2_frame_sniffer
+  dwarf2_frame_sniffer,
+  dwarf2_frame_dealloc_cache
 };
 
 static const struct frame_unwind dwarf2_signal_frame_unwind =
@@ -1400,7 +1441,10 @@ static const struct frame_unwind dwarf2_signal_frame_unwind =
   dwarf2_frame_this_id,
   dwarf2_frame_prev_register,
   NULL,
-  dwarf2_frame_sniffer
+  dwarf2_frame_sniffer,
+
+  /* TAILCALL_CACHE can never be in such frame to need dealloc_cache.  */
+  NULL
 };
 
 /* Append the DWARF-2 frame unwinders to GDBARCH's list.  */
@@ -1408,6 +1452,10 @@ static const struct frame_unwind dwarf2_signal_frame_unwind =
 void
 dwarf2_append_unwinders (struct gdbarch *gdbarch)
 {
+  /* TAILCALL_FRAME must be first to find the record by
+     dwarf2_tailcall_sniffer_first.  */
+  frame_unwind_append_unwinder (gdbarch, &dwarf2_tailcall_frame_unwind);
+
   frame_unwind_append_unwinder (gdbarch, &dwarf2_frame_unwind);
   frame_unwind_append_unwinder (gdbarch, &dwarf2_signal_frame_unwind);
 }
@@ -1459,7 +1507,8 @@ dwarf2_frame_cfa (struct frame_info *this_frame)
   /* This restriction could be lifted if other unwinders are known to
      compute the frame base in a way compatible with the DWARF
      unwinder.  */
-  if (! frame_unwinder_is (this_frame, &dwarf2_frame_unwind))
+  if (!frame_unwinder_is (this_frame, &dwarf2_frame_unwind)
+      && !frame_unwinder_is (this_frame, &dwarf2_tailcall_frame_unwind))
     error (_("can't compute CFA for this frame"));
   return get_frame_base (this_frame);
 }
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -33,6 +33,7 @@
 #include "objfiles.h"
 #include "exceptions.h"
 #include "block.h"
+#include "gdbcmd.h"
 
 #include "dwarf2.h"
 #include "dwarf2expr.h"
@@ -298,6 +299,14 @@ dwarf_expr_get_base_type (struct dwarf_expr_context *ctx, size_t die_offset)
   return dwarf2_get_die_type (die_offset, debaton->per_cu);
 }
 
+static int tailcall_debug = 0;
+static void
+show_tailcall_debug (struct ui_file *file, int from_tty,
+		     struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Tail call frames debugging is %s.\n"), value);
+}
+
 /* Find DW_TAG_GNU_call_site's DW_AT_GNU_call_site_target address.
    CALLER_FRAME (for registers) can be NULL if it is not known.  This function
    always returns valid address or it throws NOT_FOUND_ERROR.  */
@@ -359,6 +368,335 @@ call_site_to_target_addr (struct call_site *call_site,
     }
 }
 
+/* Convert function entry point exact address ADDR to the function which is
+   compliant with TAIL_CALL_LIST_COMPLETE condition.  Throw NOT_FOUND_ERROR
+   otherwise.  */
+
+static struct symbol *
+func_addr_to_tail_call_list (struct gdbarch *gdbarch, CORE_ADDR addr)
+{
+  struct symbol *sym = find_pc_function (addr);
+  struct type *type;
+
+  if (sym == NULL || BLOCK_START (SYMBOL_BLOCK_VALUE (sym)) != addr)
+    throw_error (NOT_FOUND_ERROR,
+		 _("DW_TAG_GNU_call_site resolving failed to find function "
+		   "name for address %s"),
+		 paddress (gdbarch, addr));
+
+  type = SYMBOL_TYPE (sym);
+  gdb_assert (TYPE_CODE (type) == TYPE_CODE_FUNC);
+  gdb_assert (TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_FUNC);
+
+  return sym;
+}
+
+/* Print user readable form of CALL_SITE->PC to gdb_stdlog.  Used only for
+   TAILCALL_DEBUG.  */
+
+static void
+tailcall_dump (struct gdbarch *gdbarch, const struct call_site *call_site)
+{
+  CORE_ADDR addr = call_site->pc;
+  struct minimal_symbol *msym = lookup_minimal_symbol_by_pc (addr - 1);
+
+  fprintf_unfiltered (gdb_stdlog, " %s(%s)", paddress (gdbarch, addr),
+		      msym == NULL ? "???" : SYMBOL_PRINT_NAME (msym));
+
+}
+
+/* vec.h needs single word type name, typedef it.  */
+typedef struct call_site *call_sitep;
+
+/* Define VEC (call_sitep) functions.  */
+DEF_VEC_P (call_sitep);
+
+/* Intersect RESULTP with CHAIN to keep RESULTP unambiguous, keep in RESULTP
+   only top callers and bottom callees which are present in both.  GDBARCH is
+   used only for TAILCALL_DEBUG.  RESULTP is NULL after return if there are no
+   remaining possibilities to provide unambiguous non-trivial result.  RESULTP
+   should point to NULL on the first (initialization) call.  caller is
+   responsible for xfree of any RESULTP data.  */
+
+static void
+chain_candidate (struct gdbarch *gdbarch, struct call_site_chain **resultp,
+		 VEC (call_sitep) *chain)
+{
+  struct call_site_chain *result = *resultp;
+  long length = VEC_length (call_sitep, chain);
+  int callers, callees, idx;
+
+  if (result == NULL)
+    {
+      /* Create the initial chain containing all the passed PCs.  */
+
+      result = xmalloc (sizeof (*result) + sizeof (*result->call_site)
+					   * (length - 1));
+      result->length = length;
+      result->callers = result->callees = length;
+      memcpy (result->call_site, VEC_address (call_sitep, chain),
+	      sizeof (*result->call_site) * length);
+      *resultp = result;
+
+      if (tailcall_debug)
+	{
+	  fprintf_unfiltered (gdb_stdlog, "tailcall: initial:");
+	  for (idx = 0; idx < length; idx++)
+	    tailcall_dump (gdbarch, result->call_site[idx]);
+	  fputc_unfiltered ('\n', gdb_stdlog);
+	}
+
+      return;
+    }
+
+  if (tailcall_debug)
+    {
+      fprintf_unfiltered (gdb_stdlog, "tailcall: compare:");
+      for (idx = 0; idx < length; idx++)
+	tailcall_dump (gdbarch, VEC_index (call_sitep, chain, idx));
+      fputc_unfiltered ('\n', gdb_stdlog);
+    }
+
+  /* Intersect callers.  */
+
+  callers = min (result->callers, length);
+  for (idx = 0; idx < callers; idx++)
+    if (result->call_site[idx] != VEC_index (call_sitep, chain, idx))
+      {
+	result->callers = idx;
+	break;
+      }
+
+  /* Intersect callees.  */
+
+  callees = min (result->callees, length);
+  for (idx = 0; idx < callees; idx++)
+    if (result->call_site[result->length - 1 - idx]
+	!= VEC_index (call_sitep, chain, length - 1 - idx))
+      {
+	result->callees = idx;
+	break;
+      }
+
+  if (tailcall_debug)
+    {
+      fprintf_unfiltered (gdb_stdlog, "tailcall: reduced:");
+      for (idx = 0; idx < result->callers; idx++)
+	tailcall_dump (gdbarch, result->call_site[idx]);
+      fputs_unfiltered (" |", gdb_stdlog);
+      for (idx = 0; idx < result->callees; idx++)
+	tailcall_dump (gdbarch, result->call_site[result->length
+						  - result->callees + idx]);
+      fputc_unfiltered ('\n', gdb_stdlog);
+    }
+
+  if (result->callers == 0 && result->callees == 0)
+    {
+      /* There are no common callers or callees.  It could be also a direct
+	 call (which has length 0) with ambiguous possibility of an indirect
+	 call - CALLERS == CALLEES == 0 is valid during the first allocation
+	 but any subsequence processing of such entry means ambiguity.  */
+      xfree (result);
+      *resultp = NULL;
+      return;
+    }
+
+  /* See call_site_find_chain_1 why there is no way to reach the bottom callee
+     PC again.  In such case there must be two different code paths to reach
+     it, therefore some of the former determined intermediate PCs must differ
+     and the unambiguous chain gets shortened.  */
+  gdb_assert (result->callers + result->callees < result->length);
+}
+
+/* Cleanup helper to free VEC (call_sitep) **.  */
+
+static void
+free_call_sitep_vecp (void *arg)
+{
+  VEC (call_sitep) **vecp = arg;
+
+  if (*vecp)
+    {
+      VEC_free (call_sitep, *vecp);
+      *vecp = NULL;
+    }
+}
+
+/* Create and return call_site_chain for CALLER_PC and CALLEE_PC.  All the
+   assumed frames between them use GDBARCH.  Use depth first search so we can
+   keep single CHAIN of call_site's back to CALLER_PC.  Function recursion
+   would have needless GDB stack overhead.  Caller is responsible for xfree of
+   the returned result.  Any unreliability results in thrown
+   NOT_FOUND_ERROR.  */
+
+static struct call_site_chain *
+call_site_find_chain_1 (struct gdbarch *gdbarch, CORE_ADDR caller_pc,
+			CORE_ADDR callee_pc)
+{
+  struct func_type *func_specific;
+  struct obstack addr_obstack;
+  struct cleanup *back_to_retval, *back_to_workdata;
+  struct call_site_chain *retval = NULL;
+  struct call_site *call_site;
+
+  /* Mark CALL_SITEs so we do not visit the same ones twice.  */
+  htab_t addr_hash;
+
+  /* CHAIN contains only the intermediate CALL_SITEs.  Neither CALLER_PC's
+     call_site nor any possible call_site at CALLEE_PC's function is there.
+     Any CALL_SITE in CHAIN will be iterated to its siblings - via
+     TAIL_CALL_NEXT.  This is inappropriate for CALLER_PC's call_site.  */
+  VEC (call_sitep) *chain = NULL;
+
+  /* We are not interested in the specific PC inside the callee function.  */
+  callee_pc = get_pc_function_start (callee_pc);
+  if (callee_pc == 0)
+    throw_error (NOT_FOUND_ERROR, _("Unable to find function for PC %s"),
+		 paddress (gdbarch, callee_pc));
+
+  back_to_retval = make_cleanup (free_current_contents, &retval);
+
+  obstack_init (&addr_obstack);
+  back_to_workdata = make_cleanup_obstack_free (&addr_obstack);   
+  addr_hash = htab_create_alloc_ex (64, core_addr_hash, core_addr_eq, NULL,
+				    &addr_obstack, hashtab_obstack_allocate,
+				    NULL);
+  make_cleanup_htab_delete (addr_hash);
+
+  make_cleanup (free_call_sitep_vecp, &chain);
+
+  /* Do not push CALL_SITE to CHAIN.  Push there only the first tail call site at
+     the target's function.  All the possible tail call sites in the target's
+     function will get iterated as already pushed into CHAIN via their
+     TAIL_CALL_NEXT.  */
+  call_site = call_site_for_pc (caller_pc);
+
+  while (call_site)
+    {
+      CORE_ADDR target_func_addr;
+      struct call_site *target_call_site;
+
+      /* CALLER_FRAME with registers is not available for tail-call jumped
+	 frames.  */
+      target_func_addr = call_site_to_target_addr (call_site, NULL);
+
+      if (target_func_addr == callee_pc)
+	{
+	  chain_candidate (gdbarch, &retval, chain);
+	  if (retval == NULL)
+	    break;
+
+	  /* There is no way to reach CALLEE_PC again as we would prevent
+	     entering it twice as being already marked in ADDR_HASH.  */
+	  target_call_site = NULL;
+	}
+      else
+	{
+	  struct symbol *target_func;
+
+	  target_func = func_addr_to_tail_call_list (gdbarch, target_func_addr);
+	  target_call_site = TYPE_TAIL_CALL_LIST (SYMBOL_TYPE (target_func));
+	}
+
+      do
+	{
+	  /* Attempt to visit TARGET_CALL_SITE.  */
+
+	  if (target_call_site)
+	    {
+	      void **slot;
+
+	      slot = htab_find_slot (addr_hash, &target_call_site->pc, INSERT);
+	      if (*slot == NULL)
+		{
+		  /* Successfully entered TARGET_CALL_SITE.  */
+
+		  *slot = &target_call_site->pc;
+		  VEC_safe_push (call_sitep, chain, target_call_site);
+		  break;
+		}
+	    }
+
+	  /* Backtrack (without revisiting the originating call_site).  Try the
+	     callers's sibling; if there isn't any try the callers's callers's
+	     sibling etc.  */
+
+	  target_call_site = NULL;
+	  while (!VEC_empty (call_sitep, chain))
+	    {
+	      call_site = VEC_pop (call_sitep, chain);
+
+	      gdb_assert (htab_find_slot (addr_hash, &call_site->pc,
+					  NO_INSERT) != NULL);
+	      htab_remove_elt (addr_hash, &call_site->pc);
+
+	      target_call_site = call_site->tail_call_next;
+	      if (target_call_site)
+		break;
+	    }
+	}
+      while (target_call_site);
+
+      if (VEC_empty (call_sitep, chain))
+	call_site = NULL;
+      else
+	call_site = VEC_last (call_sitep, chain);
+    }
+
+  if (retval == NULL)
+    {
+      struct minimal_symbol *msym_caller, *msym_callee;
+      
+      msym_caller = lookup_minimal_symbol_by_pc (caller_pc);
+      msym_callee = lookup_minimal_symbol_by_pc (callee_pc);
+      throw_error (NOT_FOUND_ERROR,
+		   _("There are no unambiguously determinable intermediate "
+		     "callers or callees between caller function \"%s\" at %s "
+		     "and callee function \"%s\" at %s"),
+		   (msym_caller == NULL
+		    ? "???" : SYMBOL_PRINT_NAME (msym_caller)),
+		   paddress (gdbarch, caller_pc),
+		   (msym_callee == NULL
+		    ? "???" : SYMBOL_PRINT_NAME (msym_callee)),
+		   paddress (gdbarch, callee_pc));
+    }
+
+  do_cleanups (back_to_workdata);
+  discard_cleanups (back_to_retval);
+  return retval;
+}
+
+/* Create and return call_site_chain for CALLER_PC and CALLEE_PC.  All the
+   assumed frames between them use GDBARCH.  If valid call_site_chain cannot be
+   constructed return NULL.  Caller is responsible for xfree of the returned
+   result.  */
+
+struct call_site_chain *
+call_site_find_chain (struct gdbarch *gdbarch, CORE_ADDR caller_pc,
+		      CORE_ADDR callee_pc)
+{
+  volatile struct gdb_exception e;
+  struct call_site_chain *retval = NULL;
+
+  TRY_CATCH (e, RETURN_MASK_ERROR)
+    {
+      retval = call_site_find_chain_1 (gdbarch, caller_pc, callee_pc);
+    }
+  if (e.reason < 0)
+    {
+      if (e.error == NOT_FOUND_ERROR)
+	{
+	  if (info_verbose)
+	    exception_print (gdb_stdout, e);
+
+	  return NULL;
+	}
+      else
+	throw_exception (e);
+    }
+  return retval;
+}
+
 /* Fetch call_site_parameter from caller matching the parameters.  FRAME is for
    callee.  See DWARF_REG and FB_OFFSET description at struct
    dwarf_expr_context_funcs->push_dwarf_reg_entry_value.
@@ -3217,3 +3555,18 @@ const struct symbol_computed_ops dwarf2_loclist_funcs = {
   loclist_describe_location,
   loclist_tracepoint_var_ref
 };
+
+void
+_initialize_dwarf2loc (void)
+{
+  add_setshow_zinteger_cmd ("tailcall", class_maintenance,
+			    &tailcall_debug,
+			    _("Set tail call frames debugging."),
+			    _("Show tail call frames debugging."),
+			    _("When non-zero, the process of determining tail "
+			      "call frames will be printed.  You may also want "
+			      "to `set verbose 1' for more info."),
+			    NULL,
+			    show_tailcall_debug,
+			    &setdebuglist, &showdebuglist);
+}
--- a/gdb/dwarf2loc.h
+++ b/gdb/dwarf2loc.h
@@ -132,4 +132,23 @@ extern void dwarf2_compile_expr_to_ax (struct agent_expr *expr,
 				       const gdb_byte *op_end,
 				       struct dwarf2_per_cu_data *per_cu);
 
+/* Determined tail calls for constructing virtual tail call frames.  */
+
+struct call_site_chain
+  {
+    /* Initially CALLERS == CALLEES == LENGTH.  For partially ambiguous result
+       CALLERS + CALLEES < LENGTH.  */
+    int callers, callees, length;
+
+    /* Variably sized array with LENGTH elements.  Later [0..CALLERS-1] contain
+       top (GDB "prev") sites and [LENGTH-CALLEES..LENGTH-1] contain bottom
+       (GDB "next") sites.  One is interested primarily in the PC field.  */
+    struct call_site *call_site[1];
+  };
+
+struct call_site_stuff;
+extern struct call_site_chain *call_site_find_chain (struct gdbarch *gdbarch,
+						     CORE_ADDR caller_pc,
+						     CORE_ADDR callee_pc);
+
 #endif /* dwarf2loc.h */
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2031,8 +2031,10 @@ get_frame_address_in_block (struct frame_info *this_frame)
   while (get_frame_type (next_frame) == INLINE_FRAME)
     next_frame = next_frame->next;
 
-  if (get_frame_type (next_frame) == NORMAL_FRAME
+  if ((get_frame_type (next_frame) == NORMAL_FRAME
+       || get_frame_type (next_frame) == TAILCALL_FRAME)
       && (get_frame_type (this_frame) == NORMAL_FRAME
+	  || get_frame_type (this_frame) == TAILCALL_FRAME
 	  || get_frame_type (this_frame) == INLINE_FRAME))
     return pc - 1;
 
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -206,6 +206,8 @@ enum frame_type
   /* A frame representing an inlined function, associated with an
      upcoming (prev, outer, older) NORMAL_FRAME.  */
   INLINE_FRAME,
+  /* A virtual frame of a tail call - see dwarf2_tailcall_frame_unwind.  */
+  TAILCALL_FRAME,
   /* In a signal handler, various OSs handle this in various ways.
      The main thing is that the frame may be far from normal.  */
   SIGTRAMP_FRAME,
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1134,6 +1134,9 @@ frame_info (char *addr_exp, int from_tty)
 	printf_filtered (_(" Outermost frame: %s\n"),
 			 frame_stop_reason_string (reason));
     }
+  else if (get_frame_type (fi) == TAILCALL_FRAME)
+    printf_filtered (" tail call frame %d",
+		     frame_relative_level (get_prev_frame (fi)));
   else if (get_frame_type (fi) == INLINE_FRAME)
     printf_filtered (" inlined into frame %d",
 		     frame_relative_level (get_prev_frame (fi)));
--- a/gdb/testsuite/gdb.arch/amd64-entry-value.cc
+++ b/gdb/testsuite/gdb.arch/amd64-entry-value.cc
@@ -33,9 +33,71 @@ asm ("breakhere:");
   e (v);
 }
 
+static void __attribute__((noinline, noclone))
+c (int i)
+{
+  d (i * 10);
+}
+
+static void __attribute__((noinline, noclone))
+a (int i)
+{
+  c (i + 1);
+}
+
+static void __attribute__((noinline, noclone))
+b (int i)
+{
+  c (i + 2);
+}
+
+static void __attribute__((noinline, noclone))
+amb_z (int i)
+{
+  d (i + 7);
+}
+
+static void __attribute__((noinline, noclone))
+amb_y (int i)
+{
+  amb_z (i + 6);
+}
+
+static void __attribute__((noinline, noclone))
+amb_x (int i)
+{
+  amb_y (i + 5);
+}
+
+static void __attribute__((noinline, noclone))
+amb (int i)
+{
+  if (i < 0)
+    amb_x (i + 3);
+  else
+    amb_x (i + 4);
+}
+
+static void __attribute__((noinline, noclone))
+amb_b (int i)
+{
+  amb (i + 2);
+}
+
+static void __attribute__((noinline, noclone))
+amb_a (int i)
+{
+  amb_b (i + 1);
+}
+
 int
 main ()
 {
   d (30);
+  if (v)
+    a (1);
+  else
+    b (5);
+  amb_a (100);
   return 0;
 }
--- a/gdb/testsuite/gdb.arch/amd64-entry-value.exp
+++ b/gdb/testsuite/gdb.arch/amd64-entry-value.exp
@@ -47,3 +47,32 @@ gdb_continue_to_breakpoint "entry: breakhere"
 gdb_test "bt full" "^bt full\r\n#0 +d *\\(i=31\\) \[^\r\n\]*\r\nNo locals\\.\r\n#1 +0x\[0-9a-f\]+ in main .*" \
 	 "entry: bt full"
 gdb_test "p i" " = 31" "entry: p i"
+
+
+# Test virtual tail call frames.
+
+gdb_continue_to_breakpoint "tailcall: breakhere"
+
+# #0  d (i=71) at gdb.arch/amd64-entry-value.cc:33
+# #1  0x0000000000400527 in c (i=7) at gdb.arch/amd64-entry-value.cc:38
+# #2  0x0000000000400545 in b (i=5) at gdb.arch/amd64-entry-value.cc:50
+# #3  0x00000000004003ee in main () at gdb.arch/amd64-entry-value.cc:60
+gdb_test "bt" "^bt\r\n#0 +d *\\(i=71\\) \[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in c \\(i=7\\) \[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in b \\(i=5\\) \[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in main \[^\r\n\]*" \
+	 "tailcall: bt"
+gdb_test "p i" " = 71" "tailcall: p i"
+
+
+# Test partial-ambiguous virtual tail call frames chain.
+
+gdb_continue_to_breakpoint "ambiguous: breakhere"
+
+# #0  d (i=<optimized out>) at gdb.arch/amd64-entry-value.cc:33
+# #1  0x0000000000400555 in amb_z (i=<optimized out>) at gdb.arch/amd64-entry-value.cc:56
+# #2  0x0000000000400565 in amb_y (i=<optimized out>) at gdb.arch/amd64-entry-value.cc:62
+# #3  0x0000000000400575 in amb_x (i=<optimized out>) at gdb.arch/amd64-entry-value.cc:68
+# --- here is missing a frame for ambiguous PC in amb ().
+# #4  0x0000000000400595 in amb_b (i=101) at gdb.arch/amd64-entry-value.cc:83
+# #5  0x00000000004005a5 in amb_a (i=100) at gdb.arch/amd64-entry-value.cc:89
+# #6  0x00000000004003f8 in main () at gdb.arch/amd64-entry-value.cc:100
+gdb_test "bt" "^bt\r\n#0 +d \\(i=<optimized out>\\)\[^\r\n\]*\r\n#1 +0x\[0-9a-f\]+ in amb_z \\(i=<optimized out>\\)\[^\r\n\]*\r\n#2 +0x\[0-9a-f\]+ in amb_y \\(i=<optimized out>\\)\[^\r\n\]*\r\n#3 +0x\[0-9a-f\]+ in amb_x \\(i=<optimized out>\\)\[^\r\n\]*\r\n#4 +0x\[0-9a-f\]+ in amb_b \\(i=101\\)\[^\r\n\]*\r\n#5 +0x\[0-9a-f\]+ in amb_a \\(i=100\\)\[^\r\n\]*\r\n#6 +0x\[0-9a-f\]+ in main \\(\\)\[^\r\n\]*" \
+	 "ambiguous: bt"

Re: [patch 04/12] entryval: Virtual tail call frames

[Daniel Jacobowitz]

On Mon, Jul 18, 2011 at 4:16 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Between frame #3 and frame #4 is function `amb' but one cannot reliably
> reconstruct its exact PC.  The code tries to determine only a sequence of
> unambiguous bottom calees and top callers and anything in between is dropped.
> Currently GDB knows that between frame #3 and frame #4 it is uncertain.
> Former patch tried to display some "???" there.  It had a real frame_info,
> later I removed it, I think such "???" marker should never have its real
> frame_info inside GDB.  Still the backtracing function could display some
> uncertainty indicator for the user; this is currently not implemented.
> GDB already behaves such way without trying to recover any frames info.

For the record, based on my experiments with inlined function
unwinding, I agree with your conclusion here.  We shouldn't keep a
frame_info around unless it is a part of the default backtrace, and we
shouldn't display a frame that lacks accurate PC and source location
information.

I haven't looked at the patches, but I'm impressed with what you've
accomplished here!

-- 
Thanks,
Daniel

Re: [patch 04/12] entryval: Virtual tail call frames

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> +   Copyright (C) 2010, 2011 Free Software Foundation, Inc.

Diligent work!  Also I wanted to say that I think this series is
excellent, and I am really glad I picked the easier DWARF extensions ;-)

Jan> +/* Associated structure of the unwinder for call_site_chain.  */
Jan> +
Jan> +struct tailcall_cache
Jan> +{
Jan> +  /* It must be the first one of this struct.  It is the furthest callee.  */
Jan> +  struct frame_info *next_bottom_frame;

It was not obvious to me that the caching all works out correctly so
that it is ok to use a frame_info instead of a frame ID here.  I take
your word for it, of course, but a comment about how one would recognize
the correctness of this code would have been helpful.

Jan> +++ b/gdb/dwarf2-frame-tailcall.h

Should go in HFILES_NO_SRCDIR.

Jan> +/* Cleanup helper to free VEC (call_sitep) **.  */
Jan> +
Jan> +static void
Jan> +free_call_sitep_vecp (void *arg)

You can delete this function and just use 'VEC_cleanup (call_sitep)'.

Jan> +  add_setshow_zinteger_cmd ("tailcall", class_maintenance,
Jan> +			    &tailcall_debug,
Jan> +			    _("Set tail call frames debugging."),
Jan> +			    _("Show tail call frames debugging."),
Jan> +			    _("When non-zero, the process of determining tail "
Jan> +			      "call frames will be printed.  You may also want "
Jan> +			      "to `set verbose 1' for more info."),
Jan> +			    NULL,
Jan> +			    show_tailcall_debug,
Jan> +			    &setdebuglist, &showdebuglist);

Needs a doc patch.  (If it is in another patch in the series, ignore
this; I'm working through the patches in order.)

Jan> +  /* A virtual frame of a tail call - see dwarf2_tailcall_frame_unwind.  */
Jan> +  TAILCALL_FRAME,

This needs a Python API update and a corresponding doc patch.

Tom

Re: [patch 04/12] entryval: Virtual tail call frames

[Pedro Alves]

This (and gcc/dwarf's side of the) work is really impressive.  Kudos!

I'll shoot a couple of comments below on a couple of hunks
I noticed on a first skim of this patch.

Sorry for the quick comments.  This definitely deserves a thorougher read.

On Monday 18 July 2011 21:16:58, Jan Kratochvil wrote:

> +static void
> +tailcall_frame_this_id (struct frame_info *this_frame, void **this_cache,
> +                       struct frame_id *this_id)
> +{
> +  struct tailcall_cache *cache = *this_cache;
> +  struct frame_info *next_frame;
> +  CORE_ADDR this_frame_base;
> +
> +  /* Tail call does not make sense for a sentinel frame.  */
> +  next_frame = get_next_frame (this_frame);
> +  gdb_assert (next_frame != NULL);
> +
> +  /* SP does not change during tail calls.  */
> +  this_frame_base = get_frame_base (next_frame);

Should probably preserve frame_id->special_addr as well, for ia64.

> +
> +  (*this_id) = frame_id_build (this_frame_base, get_frame_pc (this_frame));
> +  (*this_id).inline_depth = (cache->chain_levels
> +                            - existing_next_levels (this_frame, cache));
> +  gdb_assert ((*this_id).inline_depth > 0);
> +}

> @@ -1285,6 +1297,15 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
>    CORE_ADDR addr;
>    int realnum;
>  
> +  /* Virtual tail call frames report different values only for PC.  Non-bottom
> +     frames of a virtual tail call frames chain use
> +     dwarf2_tailcall_frame_unwind unwinder so this code does not apply for
> +     them.  */
> +  if (cache->tailcall_cache && regnum == gdbarch_pc_regnum (gdbarch))
> +    return dwarf2_tailcall_frame_unwind.prev_register (this_frame,
> +                                                      &cache->tailcall_cache,
> +                                                      regnum);
> +

This looked strange.  Isn't it breaking some abstraction?
I would have expected dwarf2_tailcall_frame_unwind.prev_register
to be called _first_ (automatically by frame_prev_register) and then
have that defer to dwarf2_frame_prev_register when necessary.

-- 
Pedro Alves

Re: [patch 04/12] entryval: Virtual tail call frames

[Jan Kratochvil]

On Tue, 19 Jul 2011 16:43:10 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
...
> Jan> +/* Associated structure of the unwinder for call_site_chain.  */
> Jan> +
> Jan> +struct tailcall_cache
> Jan> +{
> Jan> +  /* It must be the first one of this struct.  It is the furthest callee.  */
> Jan> +  struct frame_info *next_bottom_frame;
> 
> It was not obvious to me that the caching all works out correctly so
> that it is ok to use a frame_info instead of a frame ID here.  I take
> your word for it, of course, but a comment about how one would recognize
> the correctness of this code would have been helpful.

There is dealloc_cache method used for any TAILCALL_FRAME and the reference
counting there is hopefully right so I do not see why there should be
a problem.

Once frame_info is created it can be ever destroyed only by
reinit_frame_cache, at that time everything gets deleted.  Maybe the code
could use frame_obstack_zalloc instead of xmalloc/xfree but that is only an
efficiency change.

Added there some comment.


> Jan> +++ b/gdb/dwarf2-frame-tailcall.h
> 
> Should go in HFILES_NO_SRCDIR.

done.


> Jan> +/* Cleanup helper to free VEC (call_sitep) **.  */
> Jan> +
> Jan> +static void
> Jan> +free_call_sitep_vecp (void *arg)
> 
> You can delete this function and just use 'VEC_cleanup (call_sitep)'.

done.


> Jan> +  add_setshow_zinteger_cmd ("tailcall", class_maintenance,
> Jan> +			    &tailcall_debug,
> Jan> +			    _("Set tail call frames debugging."),
> Jan> +			    _("Show tail call frames debugging."),
> Jan> +			    _("When non-zero, the process of determining tail "
> Jan> +			      "call frames will be printed.  You may also want "
> Jan> +			      "to `set verbose 1' for more info."),
> Jan> +			    NULL,
> Jan> +			    show_tailcall_debug,
> Jan> +			    &setdebuglist, &showdebuglist);
> 
> Needs a doc patch.

done.


> Jan> +  /* A virtual frame of a tail call - see dwarf2_tailcall_frame_unwind.  */
> Jan> +  TAILCALL_FRAME,
> 
> This needs a Python API update and a corresponding doc patch.

done.


It will be in a new patchset resubmit.


Thanks,
Jan

Re: [patch 04/12] entryval: Virtual tail call frames

[Jan Kratochvil]

On Tue, 19 Jul 2011 20:17:05 +0200, Pedro Alves wrote:
> > +static void
> > +tailcall_frame_this_id (struct frame_info *this_frame, void **this_cache,
> > +                       struct frame_id *this_id)
[...]
> Should probably preserve frame_id->special_addr as well, for ia64.
+
> > +  (*this_id) = frame_id_build (this_frame_base, get_frame_pc (this_frame));

Yes, done.


> > @@ -1285,6 +1297,15 @@ dwarf2_frame_prev_register (struct frame_info *this_frame, void **this_cache,
> >    CORE_ADDR addr;
> >    int realnum;
> >  
> > +  /* Virtual tail call frames report different values only for PC.  Non-bottom
> > +     frames of a virtual tail call frames chain use
> > +     dwarf2_tailcall_frame_unwind unwinder so this code does not apply for
> > +     them.  */
> > +  if (cache->tailcall_cache && regnum == gdbarch_pc_regnum (gdbarch))
> > +    return dwarf2_tailcall_frame_unwind.prev_register (this_frame,
> > +                                                      &cache->tailcall_cache,
> > +                                                      regnum);
> > +
> 
> This looked strange.  Isn't it breaking some abstraction?
> I would have expected dwarf2_tailcall_frame_unwind.prev_register
> to be called _first_ (automatically by frame_prev_register) and then
> have that defer to dwarf2_frame_prev_register when necessary.

This is now slightly changed.  Besides simulated PC there must be also
simulated SP (for the pushed return address in tail call frames).  So that
exception has been moved into dwarf2-frame-tailcall.c now.

Or do you question the current layout?  Without this patch:
bottom frame (NORMAL_FRAME)
top frame (NORMAL_FRAME)

with this patch for the same inferior state:
bottom frame (NORMAL_FRAME unwind with patched PC+SP into tail call frame #1)
tail call frame #1 (TAILCALL_FRAME)
tail call frame #2 (TAILCALL_FRAME)
top frame (NORMAL_FRAME)

It is questionable whether "bottom frame" should be NORMAL_FRAME or
TAILCALL_FRAME.  I find it more like NORMAL_FRAME as frame is about unwinding
registers and "bottom frame" does (almost) the normal register unwind.

"tail call frame #1+2" has register set of "top frame" which suggests this
frame types layout IMO.


It will be in a new patchset resubmit.


Thanks,
Jan