[rfc/rfa] Use ARM exception tables as GDB unwinder

[rfc/rfa] Use ARM exception tables as GDB unwinder

[Ulrich Weigand]

Hello,

as suggested by Dan Jacobowitz, this patch adds support for using the frame
unwinding instructions found in certain records of the ARM-specific exception
handling data (in .ARM.exidx and .ARM.extbl sections) for unwinding in GDB.

There are two main parts needed to implement this support: 

1. A new_objfile observer that checks every newly loaded objfile for the
presence of ARM exception handling records, and if found, records all frame
unwinding instructions present there.  This data is stored indexed by section
and offset, analogous to how mapping symbols are handled.

2. An unwind sniffer that checks whether the PC is covered by an exception
table entry with unwinding instructions.  If so, those instructions are
decoded and used to construct a prologue unwinder cache record.  This is
then used with the existing prologue unwinder's this_is and prev_register
routines to perform actual unwinding.

In addition, one minor change was necessary to the prologue unwinder:
the this_id routine no longer uses get_frame_func to retrieve the
function start address; instead, this value is stored in the cache.
This allows the exception unwinder to provide this information from
the exception table, instead of having to rely on symbol data.

Tested on armv7l-linux-gnueabi with no regressions.  The patch fixes
the following testsuite failures:
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO: BINprelinkNOdebugNOpieNO: core: core main bt
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO: BINprelinkNOdebugNOpieYES: core: core main bt
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO: BINprelinkNOdebugINpieNO: core: core main bt
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO: BINprelinkNOdebugINpieYES: core: core main bt
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO: BINprelinkNOdebugSEPpieNO: core: core main bt
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO: BINprelinkNOdebugSEPpieYES: core: core main bt
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugIN: BINprelinkNOdebugNOpieNO: core: core main bt
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugIN: BINprelinkNOdebugNOpieYES: core: core main bt
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugIN: BINprelinkNOdebugINpieNO: core: core main bt
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugIN: BINprelinkNOdebugINpieYES: core: core main bt
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugIN: BINprelinkNOdebugSEPpieNO: core: core main bt
FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugIN: BINprelinkNOdebugSEPpieYES: core: core main bt

On systems where no libc debug/symbol information is available, the
following additional testsuite failures are fixed as well:
FAIL: gdb.gdb/selftest.exp: backtrace through signal handler
FAIL: gdb.threads/attach-stopped.exp: threaded: attach2 to stopped bt
FAIL: gdb.threads/attachstop-mt.exp: attach0 to sleeping bt
FAIL: gdb.threads/attachstop-mt.exp: attach3 to stopped bt
FAIL: gdb.threads/attachstop-mt.exp: attach4 to stopped bt
FAIL: gdb.threads/pthreads.exp: check backtrace from thread 1
FAIL: gdb.threads/pthreads.exp: check backtrace from thread 2
FAIL: gdb.threads/pthreads.exp: apply backtrace command to all three threads


OK for mainline?

Bye,
Ulrich


ChangeLog:

	* arm-tdep.c: Include "observer.h".
	(struct arm_prologue_cache): Add THIS_FUNC member.
	(arm_make_prologue_cache): Set it.
	(arm_prologue_this_id): Use it instead of calling get_frame_func.
	(arm_exidx_data_key): New static variable.
	(struct arm_exidx_entry, arm_exidx_entry_s): New data types.
	(struct arm_exidx_data): Likewise.
	(arm_exidx_data_free ): New function.
	(arm_compare_exidx_entries): Likewise.
	(section_covers_vma): Likewise.
	(arm_exidx_new_objfile): Likewise.
	(arm_find_exidx_entry): Likewise.
	(arm_exidx_fill_cache): Likewise.
	(arm_exidx_unwind_sniffer): Likewise.
	(arm_exidx_unwind): New global variable.
	(arm_gdbarch_init): Append unwinder arm_exidx_unwind.
	(_initialize_arm_tdep): Attach arm_exidx_new_objfile to new_objfile
	observer.  Register arm_exidx_data_key as objfile data.

Index: gdb/arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.310
diff -u -p -r1.310 arm-tdep.c
--- gdb/arm-tdep.c	12 Oct 2010 08:46:15 -0000	1.310
+++ gdb/arm-tdep.c	18 Oct 2010 17:14:31 -0000
@@ -42,6 +42,7 @@
 #include "prologue-value.h"
 #include "target-descriptions.h"
 #include "user-regs.h"
+#include "observer.h"
 
 #include "arm-tdep.h"
 #include "gdb/sim-arm.h"
@@ -225,6 +226,9 @@ struct arm_prologue_cache
      to identify this frame.  */
   CORE_ADDR prev_sp;
 
+  /* Starting address of the current function.  */
+  CORE_ADDR this_func;
+
   /* The frame base for this frame is just prev_sp - frame size.
      FRAMESIZE is the distance from the frame pointer to the
      initial stack pointer.  */
@@ -1793,6 +1797,7 @@ arm_make_prologue_cache (struct frame_in
     return cache;
 
   cache->prev_sp = unwound_fp + cache->framesize;
+  cache->this_func = get_frame_func (this_frame);
 
   /* Calculate actual addresses of saved registers using offsets
      determined by arm_scan_prologue.  */
@@ -1813,7 +1818,7 @@ arm_prologue_this_id (struct frame_info 
 {
   struct arm_prologue_cache *cache;
   struct frame_id id;
-  CORE_ADDR pc, func;
+  CORE_ADDR pc;
 
   if (*this_cache == NULL)
     *this_cache = arm_make_prologue_cache (this_frame);
@@ -1828,8 +1833,7 @@ arm_prologue_this_id (struct frame_info 
   if (cache->prev_sp == 0)
     return;
 
-  func = get_frame_func (this_frame);
-  id = frame_id_build (cache->prev_sp, func);
+  id = frame_id_build (cache->prev_sp, cache->this_func);
   *this_id = id;
 }
 
@@ -1899,6 +1903,629 @@ struct frame_unwind arm_prologue_unwind 
   default_frame_sniffer
 };
 
+/* Maintain a list of ARM exception table entries per objfile, similar to the
+   list of mapping symbols.  We only cache entries for standard ARM-defined
+   personality routines; the cache will contains only the frame unwinding
+   instructions associated with the entry (not the descriptors).  */
+
+static const struct objfile_data *arm_exidx_data_key;
+
+struct arm_exidx_entry
+{
+  bfd_vma addr;
+  gdb_byte *entry;
+};
+typedef struct arm_exidx_entry arm_exidx_entry_s;
+DEF_VEC_O(arm_exidx_entry_s);
+
+struct arm_exidx_data
+{
+  VEC(arm_exidx_entry_s) **section_maps;
+};
+
+static void
+arm_exidx_data_free (struct objfile *objfile, void *arg)
+{
+  struct arm_exidx_data *data = arg;
+  unsigned int i;
+
+  for (i = 0; i < objfile->obfd->section_count; i++)
+    VEC_free (arm_exidx_entry_s, data->section_maps[i]);
+}
+
+static inline int
+arm_compare_exidx_entries (const struct arm_exidx_entry *lhs,
+			   const struct arm_exidx_entry *rhs)
+{
+  return lhs->addr < rhs->addr;
+}
+
+static bfd_boolean
+section_covers_vma (bfd *abfd, asection *section, void *ptr)
+{
+  bfd_vma vma = *(bfd_vma *) ptr;
+  return ((section->flags & SEC_ALLOC) != 0
+          && section->vma <= vma
+          && vma < section->vma + section->size);
+}
+
+/* Parse contents of exception table and exception index sections
+   of OBJFILE, and fill in the exception table entry cache.
+
+   For each entry that refers to a standard ARM-defined personality
+   routine, extract the frame unwinding instructions (from either
+   the index or the table section).  The unwinding instructions
+   are normalized by:
+    - extracting them from the rest of the table data
+    - converting to host endianness
+    - appending the implicit 0xb0 ("Finish") code
+
+   The extracted and normalized instructions are stored for later
+   retrieval by the arm_find_exidx_entry routine.  */
+ 
+static void
+arm_exidx_new_objfile (struct objfile *objfile)
+{
+  struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
+  struct arm_exidx_data *data;
+  asection *exidx, *extab;
+  bfd_vma exidx_vma = 0, extab_vma = 0;
+  bfd_size_type exidx_size = 0, extab_size = 0;
+  gdb_byte *exidx_data = NULL, *extab_data = NULL;
+  LONGEST i;
+
+  /* If we've already touched this file, do nothing.  */
+  if (!objfile || objfile_data (objfile, arm_exidx_data_key) != NULL)
+    return;
+
+  /* Read contents of exception table and index.  */
+  exidx = bfd_get_section_by_name (objfile->obfd, ".ARM.exidx");
+  if (exidx)
+    {
+      exidx_vma = bfd_section_vma (objfile->obfd, exidx);
+      exidx_size = bfd_get_section_size (exidx);
+      exidx_data = xmalloc (exidx_size);
+      make_cleanup (xfree, exidx_data);
+
+      if (!bfd_get_section_contents (objfile->obfd, exidx,
+				     exidx_data, 0, exidx_size))
+	{
+	  do_cleanups (cleanups);
+	  return;
+	}
+    }
+
+  extab = bfd_get_section_by_name (objfile->obfd, ".ARM.extab");
+  if (extab)
+    {
+      extab_vma = bfd_section_vma (objfile->obfd, extab);
+      extab_size = bfd_get_section_size (extab);
+      extab_data = xmalloc (extab_size);
+      make_cleanup (xfree, extab_data);
+
+      if (!bfd_get_section_contents (objfile->obfd, extab,
+				     extab_data, 0, extab_size))
+	{
+	  do_cleanups (cleanups);
+	  return;
+	}
+    }
+
+  /* Allocate exception table data structure.  */
+  data = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct arm_exidx_data);
+  set_objfile_data (objfile, arm_exidx_data_key, data);
+  data->section_maps = OBSTACK_CALLOC (&objfile->objfile_obstack,
+				       objfile->obfd->section_count,
+				       VEC(arm_exidx_entry_s) *);
+
+  /* Fill in exception table.  */
+  for (i = 0; i < exidx_size / 8; i++)
+    {
+      struct arm_exidx_entry new_exidx_entry;
+      bfd_vma idx = bfd_h_get_32 (objfile->obfd, exidx_data + i * 8);
+      bfd_vma val = bfd_h_get_32 (objfile->obfd, exidx_data + i * 8 + 4);
+      asection *sect;
+      gdb_byte *p, *entry = NULL;
+
+      /* Extract address of start of function.  */
+      idx = ((idx & 0x7fffffff) ^ 0x40000000) - 0x40000000;
+      idx += exidx_vma + i * 8;
+
+      /* Find section containing function and compute section offset.  */
+      sect = bfd_sections_find_if (objfile->obfd, section_covers_vma, &idx);
+      if (sect == NULL)
+	continue;
+      idx -= bfd_get_section_vma (objfile->obfd, sect);
+
+      /* Determine address of exception table entry.  */
+      if (val == 1)
+	{
+	  /* EXIDX_CANTUNWIND -- no exception table entry present.  */
+	}
+      else if ((val & 0xff000000) == 0x80000000)
+	{
+	  /* Exception table entry embedded in .ARM.exidx
+	     -- must be short form.  */
+	  p = entry = obstack_alloc (&objfile->objfile_obstack, 4);
+	  *p++ = (gdb_byte) ((val >> 16) & 0xff);
+	  *p++ = (gdb_byte) ((val >> 8) & 0xff);
+	  *p++ = (gdb_byte) (val & 0xff);
+	  *p++ = 0xb0; /* implied "Finish" to terminate the list */
+	}
+      else if (!(val & 0x80000000))
+	{
+	  /* Exception table entry in .ARM.extab.  */
+	  val = ((val & 0x7fffffff) ^ 0x40000000) - 0x40000000;
+	  val += exidx_vma + i * 8 + 4;
+
+	  if (val >= extab_vma && val + 4 <= extab_vma + extab_size)
+	    {
+	      bfd_vma data = bfd_h_get_32 (objfile->obfd,
+					   extab_data + val - extab_vma);
+
+	      if ((data & 0xff000000) == 0x80000000)
+		{
+		  /* Short form.  */
+		  p = entry = obstack_alloc (&objfile->objfile_obstack, 4);
+		  *p++ = (gdb_byte) ((data >> 16) & 0xff);
+		  *p++ = (gdb_byte) ((data >> 8) & 0xff);
+		  *p++ = (gdb_byte) (data & 0xff);
+		  *p++ = 0xb0; /* implied "Finish" to terminate the list */
+		}
+	      else if ((data & 0xff000000) == 0x81000000
+		       || (data & 0xff000000) == 0x82000000)
+		{
+		  /* Long form.  */
+		  int n_words = ((data >> 16) & 0xff) + 1;
+		  if (val + 4 * n_words <= extab_vma + extab_size)
+		    {
+		      p = entry = obstack_alloc (&objfile->objfile_obstack,
+						 4 * n_words);
+		      *p++ = (gdb_byte) ((data >> 8) & 0xff);
+		      *p++ = (gdb_byte) (data & 0xff);
+
+		      while (--n_words)
+			{
+			  val += 4;
+			  data = bfd_h_get_32 (objfile->obfd,
+					       extab_data + val - extab_vma);
+
+			  *p++ = (gdb_byte) ((data >> 24) & 0xff);
+			  *p++ = (gdb_byte) ((data >> 16) & 0xff);
+			  *p++ = (gdb_byte) ((data >> 8) & 0xff);
+			  *p++ = (gdb_byte) (data & 0xff);
+			}
+
+		      *p++ = 0xb0; /* implied "Finish" to terminate the list */
+		      *p++ = 0xb0; /* one more for 4-byte alignment */
+		    }
+		}
+	    }
+	}
+
+      /* Push entry onto vector.  They are guaranteed to always
+	 appear in order of increasing addresses.  */
+      new_exidx_entry.addr = idx;
+      new_exidx_entry.entry = entry;
+      VEC_safe_push (arm_exidx_entry_s, data->section_maps[sect->index],
+		     &new_exidx_entry);
+    }
+
+  do_cleanups (cleanups);
+}
+
+/* Search for the exception table entry covering MEMADDR.  If one is found,
+   return a pointer to its data.  Otherwise, return 0.  If START is non-NULL,
+   set *START to the start of the function covered by this entry.  */
+
+static gdb_byte *
+arm_find_exidx_entry (CORE_ADDR memaddr, CORE_ADDR *start)
+{
+  struct obj_section *sec;
+
+  sec = find_pc_section (memaddr);
+  if (sec != NULL)
+    {
+      struct arm_exidx_data *data;
+      VEC(arm_exidx_entry_s) *map;
+      struct arm_exidx_entry map_key = { memaddr - obj_section_addr (sec), 0 };
+      unsigned int idx;
+
+      data = objfile_data (sec->objfile, arm_exidx_data_key);
+      if (data != NULL)
+	{
+	  map = data->section_maps[sec->the_bfd_section->index];
+	  if (!VEC_empty (arm_exidx_entry_s, map))
+	    {
+	      struct arm_exidx_entry *map_sym;
+
+	      idx = VEC_lower_bound (arm_exidx_entry_s, map, &map_key,
+				     arm_compare_exidx_entries);
+
+	      /* VEC_lower_bound finds the earliest ordered insertion
+		 point.  If the following symbol starts at this exact
+		 address, we use that; otherwise, the preceding
+		 exception table entry covers this address.  */
+	      if (idx < VEC_length (arm_exidx_entry_s, map))
+		{
+		  map_sym = VEC_index (arm_exidx_entry_s, map, idx);
+		  if (map_sym->addr == map_key.addr)
+		    {
+		      if (start)
+			*start = map_sym->addr + obj_section_addr (sec);
+		      return map_sym->entry;
+		    }
+		}
+
+	      if (idx > 0)
+		{
+		  map_sym = VEC_index (arm_exidx_entry_s, map, idx - 1);
+		  if (start)
+		    *start = map_sym->addr + obj_section_addr (sec);
+		  return map_sym->entry;
+		}
+	    }
+	}
+    }
+
+  return NULL;
+}
+
+/* Given the current frame THIS_FRAME, and its associated frame unwinding
+   instruction list from the ARM exception table entry ENTRY, allocate and
+   return a prologue cache structure describing how to unwind this frame.
+
+   Return NULL if the unwinding instruction list contains a "spare",
+   "reserved" or "refuse to unwind" instruction as defined in section
+   "9.3 Frame unwinding instructions" of the "Exception Handling ABI
+   for the ARM Architecture" document.  */
+
+static struct arm_prologue_cache *
+arm_exidx_fill_cache (struct frame_info *this_frame, gdb_byte *entry)
+{
+  CORE_ADDR vsp;
+  int vsp_valid = 0;
+
+  struct arm_prologue_cache *cache;
+  cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
+  cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
+
+  for (;;)
+    {
+      gdb_byte insn;
+
+      /* Whenever we reload SP, we actually have to retrieve its
+	 actual value in the current frame.  */
+      if (!vsp_valid)
+	{
+	  if (trad_frame_realreg_p (cache->saved_regs, ARM_SP_REGNUM))
+	    {
+	      int reg = cache->saved_regs[ARM_SP_REGNUM].realreg;
+	      vsp = get_frame_register_unsigned (this_frame, reg);
+	    }
+	  else
+	    {
+	      CORE_ADDR addr = cache->saved_regs[ARM_SP_REGNUM].addr;
+	      vsp = get_frame_memory_unsigned (this_frame, addr, 4);
+	    }
+
+	  vsp_valid = 1;
+	}
+
+      /* Decode next unwind instruction.  */
+      insn = *entry++;
+
+      if ((insn & 0xc0) == 0)
+	{
+	  int offset = insn & 0x3f;
+	  vsp += (offset << 2) + 4;
+	}
+      else if ((insn & 0xc0) == 0x40)
+	{
+	  int offset = insn & 0x3f;
+	  vsp -= (offset << 2) + 4;
+	}
+      else if ((insn & 0xf0) == 0x80)
+	{
+	  int mask = ((insn & 0xf) << 8) | *entry++;
+	  int i;
+
+	  /* The special case of an all-zero mask identifies
+	     "Refuse to unwind".  We return NULL to fall back
+	     to the prologue analyzer.  */
+	  if (mask == 0)
+	    return NULL;
+
+	  /* Pop registers r4..r15 under mask.  */
+	  for (i = 0; i < 12; i++)
+	    if (mask & (1 << i))
+	      {
+	        cache->saved_regs[4 + i].addr = vsp;
+		vsp += 4;
+	      }
+
+	  /* Special-case popping SP -- we need to reload vsp.  */
+	  if (mask & (1 << (ARM_SP_REGNUM - 4)))
+	    vsp_valid = 0;
+	}
+      else if ((insn & 0xf0) == 0x90)
+	{
+	  int reg = insn & 0xf;
+
+	  /* Reserved cases.  */
+	  if (reg == ARM_SP_REGNUM || reg == ARM_PC_REGNUM)
+	    return NULL;
+
+	  /* Set SP from another register and mark VSP for reload.  */
+	  cache->saved_regs[ARM_SP_REGNUM] = cache->saved_regs[reg];
+	  vsp_valid = 0;
+	}
+      else if ((insn & 0xf0) == 0xa0)
+	{
+	  int count = insn & 0x7;
+	  int pop_lr = (insn & 0x8) != 0;
+	  int i;
+
+	  /* Pop r4..r[4+count].  */
+	  for (i = 0; i <= count; i++)
+	    {
+	      cache->saved_regs[4 + i].addr = vsp;
+	      vsp += 4;
+	    }
+
+	  /* If indicated by flag, pop LR as well.  */
+	  if (pop_lr)
+	    {
+	      cache->saved_regs[ARM_LR_REGNUM].addr = vsp;
+	      vsp += 4;
+	    }
+	}
+      else if (insn == 0xb0)
+	{
+	  /* We could only have updated PC by popping into it; if so, it
+	     will show up as address.  Otherwise, copy LR into PC.  */
+	  if (!trad_frame_addr_p (cache->saved_regs, ARM_PC_REGNUM))
+	    cache->saved_regs[ARM_PC_REGNUM]
+	      = cache->saved_regs[ARM_LR_REGNUM];
+
+	  /* We're done.  */
+	  break;
+	}
+      else if (insn == 0xb1)
+	{
+	  int mask = *entry++;
+	  int i;
+
+	  /* All-zero mask and mask >= 16 is "spare".  */
+	  if (mask == 0 || mask >= 16)
+	    return NULL;
+
+	  /* Pop r0..r3 under mask.  */
+	  for (i = 0; i < 4; i++)
+	    if (mask & (1 << i))
+	      {
+		cache->saved_regs[i].addr = vsp;
+		vsp += 4;
+	      }
+	}
+      else if (insn == 0xb2)
+	{
+	  ULONGEST offset = 0;
+	  unsigned shift = 0;
+
+	  do
+	    {
+	      offset |= (*entry & 0x7f) << shift;
+	      shift += 7;
+	    }
+	  while (*entry++ & 0x80);
+
+	  vsp += 0x204 + (offset << 2);
+	}
+      else if (insn == 0xb3)
+	{
+	  int start = *entry >> 4;
+	  int count = (*entry++) & 0xf;
+	  int i;
+
+	  /* Only registers D0..D15 are valid here.  */
+	  if (start + count >= 16)
+	    return NULL;
+
+	  /* Pop VFP double-precision registers D[start]..D[start+count].  */
+	  for (i = 0; i <= count; i++)
+	    {
+	      cache->saved_regs[ARM_D0_REGNUM + start + i].addr = vsp;
+	      vsp += 8;
+	    }
+
+	  /* Add an extra 4 bytes for FSTMFDX-style stack.  */
+	  vsp += 4;
+	}
+      else if ((insn & 0xf8) == 0xb8)
+	{
+	  int count = insn & 0x7;
+	  int i;
+
+	  /* Pop VFP double-precision registers D[8]..D[8+count].  */
+	  for (i = 0; i <= count; i++)
+	    {
+	      cache->saved_regs[ARM_D0_REGNUM + 8 + i].addr = vsp;
+	      vsp += 8;
+	    }
+
+	  /* Add an extra 4 bytes for FSTMFDX-style stack.  */
+	  vsp += 4;
+	}
+      else if (insn == 0xc6)
+	{
+	  int start = *entry >> 4;
+	  int count = (*entry++) & 0xf;
+	  int i;
+
+	  /* Only registers WR0..WR15 are valid.  */
+	  if (start + count >= 16)
+	    return NULL;
+
+	  /* Pop iwmmx registers WR[start]..WR[start+count].  */
+	  for (i = 0; i <= count; i++)
+	    {
+	      cache->saved_regs[ARM_WR0_REGNUM + start + i].addr = vsp;
+	      vsp += 8;
+	    }
+	}
+      else if (insn == 0xc7)
+	{
+	  int mask = *entry++;
+	  int i;
+
+	  /* All-zero mask and mask >= 16 is "spare".  */
+	  if (mask == 0 || mask >= 16)
+	    return NULL;
+
+	  /* Pop iwmmx general-purpose registers WCGR0..WCGR3 under mask.  */
+	  for (i = 0; i < 4; i++)
+	    if (mask & (1 << i))
+	      {
+		cache->saved_regs[ARM_WCGR0_REGNUM + i].addr = vsp;
+		vsp += 4;
+	      }
+	}
+      else if ((insn & 0xf8) == 0xc0)
+	{
+	  int count = insn & 0x7;
+	  int i;
+
+	  /* Pop iwmmx registers WR[10]..WR[10+count].  */
+	  for (i = 0; i <= count; i++)
+	    {
+	      cache->saved_regs[ARM_WR0_REGNUM + 10 + i].addr = vsp;
+	      vsp += 8;
+	    }
+	}
+      else if (insn == 0xc8)
+	{
+	  int start = *entry >> 4;
+	  int count = (*entry++) & 0xf;
+	  int i;
+
+	  /* Only registers D0..D31 are valid.  */
+	  if (start + count >= 16)
+	    return NULL;
+
+	  /* Pop VFP double-precision registers
+	     D[16+start]..D[16+start+count].  */
+	  for (i = 0; i <= count; i++)
+	    {
+	      cache->saved_regs[ARM_D0_REGNUM + 16 + start + i].addr = vsp;
+	      vsp += 8;
+	    }
+	}
+      else if (insn == 0xc9)
+	{
+	  int start = *entry >> 4;
+	  int count = (*entry++) & 0xf;
+	  int i;
+
+	  /* Pop VFP double-precision registers D[start]..D[start+count].  */
+	  for (i = 0; i <= count; i++)
+	    {
+	      cache->saved_regs[ARM_D0_REGNUM + start + i].addr = vsp;
+	      vsp += 8;
+	    }
+	}
+      else if ((insn & 0xf8) == 0xd0)
+	{
+	  int count = insn & 0x7;
+	  int i;
+
+	  /* Pop VFP double-precision registers D[8]..D[8+count].  */
+	  for (i = 0; i <= count; i++)
+	    {
+	      cache->saved_regs[ARM_D0_REGNUM + 8 + i].addr = vsp;
+	      vsp += 8;
+	    }
+	}
+      else
+	{
+	  /* Everything else is "spare".  */
+	  return NULL;
+	}
+    }
+
+  /* If we restore SP from a register, assume this was the frame register.
+     Otherwise just fall back to SP as frame register.  */
+  if (trad_frame_realreg_p (cache->saved_regs, ARM_SP_REGNUM))
+    cache->framereg = cache->saved_regs[ARM_SP_REGNUM].realreg;
+  else
+    cache->framereg = ARM_SP_REGNUM;
+
+  /* Determine offset to previous frame.  */
+  cache->framesize
+    = vsp - get_frame_register_unsigned (this_frame, cache->framereg);
+
+  /* We already got the previous SP.  */
+  cache->prev_sp = vsp;
+
+  return cache;
+}
+
+/* Unwinding via ARM exception table entries.  Note that the sniffer
+   already computes a filled-in prologue cache, which is then used
+   with the same arm_prologue_this_id and arm_prologue_prev_register
+   routines also used for prologue-parsing based unwinding.  */
+
+static int
+arm_exidx_unwind_sniffer (const struct frame_unwind *self,
+			  struct frame_info *this_frame,
+			  void **this_prologue_cache)
+{
+  CORE_ADDR addr_in_block, func_exidx, func_symtab;
+  struct arm_prologue_cache *cache;
+  gdb_byte *entry;
+
+  /* See if we have an ARM exception table entry covering this address.  */
+  addr_in_block = get_frame_address_in_block (this_frame);
+  entry = arm_find_exidx_entry (addr_in_block, &func_exidx);
+  if (!entry)
+    return 0;
+
+  /* The ARM exception index does not mark the *end* of the function
+     covered by the entry, and some functions will not have any entry.
+     Therefore, the entry we found above may not actually be correct
+     for this PC.  As a sanity check, also try to determine the function
+     covering PC via the symbol table.  If this finds a function start
+     address *closer* to PC than the one found via the exception table,
+     ignore the exception record, and fall back to prologue parsing.
+
+     (Note that if we don't find a function via the symbol table, the
+     exception table may still be wrong.  But in this case, prologue
+     parsing wouldn't work anyway, so we use the exception table and
+     hope for the best.)  */
+  if (find_pc_partial_function (addr_in_block, NULL, &func_symtab, NULL)
+      && func_symtab > func_exidx)
+    return 0;
+
+  /* Decode the list of unwinding instructions into a prologue cache.
+     Note that this may fail due to e.g. a "refuse to unwind" code.  */
+  cache = arm_exidx_fill_cache (this_frame, entry);
+  if (!cache)
+    return 0;
+
+  /* Fill in the function start, and remember the cache.  */
+  cache->this_func = func_exidx;
+  *this_prologue_cache = cache;
+  return 1;
+}
+
+struct frame_unwind arm_exidx_unwind = {
+  NORMAL_FRAME,
+  arm_prologue_this_id,
+  arm_prologue_prev_register,
+  NULL,
+  arm_exidx_unwind_sniffer
+};
+
 static struct arm_prologue_cache *
 arm_make_stub_cache (struct frame_info *this_frame)
 {
@@ -7488,6 +8115,7 @@ arm_gdbarch_init (struct gdbarch_info in
   /* Add some default predicates.  */
   frame_unwind_append_unwinder (gdbarch, &arm_stub_unwind);
   dwarf2_append_unwinders (gdbarch);
+  frame_unwind_append_unwinder (gdbarch, &arm_exidx_unwind);
   frame_unwind_append_unwinder (gdbarch, &arm_prologue_unwind);
 
   /* Now we have tuned the configuration, set a few final things,
@@ -7590,6 +8218,11 @@ _initialize_arm_tdep (void)
   arm_objfile_data_key
     = register_objfile_data_with_cleanup (NULL, arm_objfile_data_free);
 
+  /* Add ourselves to objfile event chain.  */
+  observer_attach_new_objfile (arm_exidx_new_objfile);
+  arm_exidx_data_key
+    = register_objfile_data_with_cleanup (NULL, arm_exidx_data_free);
+
   /* Register an ELF OS ABI sniffer for ARM binaries.  */
   gdbarch_register_osabi_sniffer (bfd_arch_arm,
 				  bfd_target_elf_flavour,
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Matthew Gretton-Dann]

Ulrich,

Firstly an apology - I'm not going to be able to look at this in detail
for a while, and Richard Earnshaw is away as well for a couple of weeks.

However, I do have one quick question:

On Wed, 2010-10-20 at 02:00 +0200, Ulrich Weigand wrote:
> In addition, one minor change was necessary to the prologue unwinder:
> the this_id routine no longer uses get_frame_func to retrieve the
> function start address; instead, this value is stored in the cache.
> This allows the exception unwinder to provide this information from
> the exception table, instead of having to rely on symbol data.

One of the 'optimisations' that can be applied to the exception unwind
table is that if two contiguous functions have the same unwind tables
they can be merged.

So for instance:

    int foo (int x, int y) { return x + y + 1; }
    int bar (int x, int y) { return x + y + 2; }

Both functions have the same unwind table entry, and so the linker may
merge the entries.  So when actually in 'bar' a naive lookup of the
function you are in using the start address of the exception table entry
would incorrectly say you are in 'foo'.

The libjava developers came across this earlier in the year when ld
started applying this optimisation (see:
http://sourceware.org/ml/binutils/2010-04/msg00193.html).

Can the change you detail above handle this?

Thanks,

Matt

-- 
Matthew Gretton-Dann
Principal Engineer - PDSW Tools
ARM Ltd

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Daniel Jacobowitz]

Thanks for doing this!

On Wed, Oct 20, 2010 at 02:00:56AM +0200, Ulrich Weigand wrote:
> In addition, one minor change was necessary to the prologue unwinder:
> the this_id routine no longer uses get_frame_func to retrieve the
> function start address; instead, this value is stored in the cache.
> This allows the exception unwinder to provide this information from
> the exception table, instead of having to rely on symbol data.

Like Matthew, I have some concern about this; unwind regions are not
1:1 with functions.  Not only will the linker combine identical
regions, but hand-written code (or, hypothetically,
-fnon-call-exceptions) can have multiple regions in a function.
But we can probably ignore the latter case; the former can be handled
by using whichever is larger, the symbol or exception region address.

> +  /* See if we have an ARM exception table entry covering this address.  */
> +  addr_in_block = get_frame_address_in_block (this_frame);
> +  entry = arm_find_exidx_entry (addr_in_block, &func_exidx);
> +  if (!entry)
> +    return 0;

I assume you're deliberately treating can't-unwind and no-information
the same.  We use can't-unwind to mark _start, but it also shows up in
exception handling in some cases.

> +  /* The ARM exception index does not mark the *end* of the function
> +     covered by the entry, and some functions will not have any entry.
> +     Therefore, the entry we found above may not actually be correct
> +     for this PC.  As a sanity check, also try to determine the function
> +     covering PC via the symbol table.  If this finds a function start
> +     address *closer* to PC than the one found via the exception table,
> +     ignore the exception record, and fall back to prologue parsing.
> +
> +     (Note that if we don't find a function via the symbol table, the
> +     exception table may still be wrong.  But in this case, prologue
> +     parsing wouldn't work anyway, so we use the exception table and
> +     hope for the best.)  */
> +  if (find_pc_partial_function (addr_in_block, NULL, &func_symtab, NULL)
> +      && func_symtab > func_exidx)
> +    return 0;

This doesn't go well with the discussion above about combined
exception regions... did you encounter this problem in practice?  The
linker used to have this problem, but hasn't for a while (circa
2009-05-05).

The actual unwinder looks OK to me.  If you want to handle C++ code
with exceptions (or C with cleanups), you'll need to check the name of
the referenced personality routine; it is __gxx_personality_v0 (et
cetera).  Readelf has a sample of that.  Code which doesn't throw or
catch uses the standard personality routines, as you've implemented.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Ulrich Weigand]

Matthew Gretton-Dann wrote:

> Firstly an apology - I'm not going to be able to look at this in detail
> for a while, and Richard Earnshaw is away as well for a couple of weeks.

No problem, thanks for looking at this patch!

> One of the 'optimisations' that can be applied to the exception unwind
> table is that if two contiguous functions have the same unwind tables
> they can be merged.
[snip]
> Can the change you detail above handle this?

That depends on whether there is symbol information for those functions.

If there *is* a symbol for the second function, my patch would detect
that the symbol is closer than the unwind start address, in which case
it would refuse to use unwind data, and fall back to prologue parsing.

If there is no symbol, the patch would use the function start address
from the unwind information as part of the frame ID, which isn't really
correct.  On the other hand, since there is no symbol, we don't *know*
the real start address either ...   Using the address from unwind data
seems as good as anything else that we could possibly do here.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Ulrich Weigand]

Dan Jacobowitz wrote:

> On Wed, Oct 20, 2010 at 02:00:56AM +0200, Ulrich Weigand wrote:
> > In addition, one minor change was necessary to the prologue unwinder:
> > the this_id routine no longer uses get_frame_func to retrieve the
> > function start address; instead, this value is stored in the cache.
> > This allows the exception unwinder to provide this information from
> > the exception table, instead of having to rely on symbol data.
> 
> Like Matthew, I have some concern about this; unwind regions are not
> 1:1 with functions.  Not only will the linker combine identical
> regions, but hand-written code (or, hypothetically,
> -fnon-call-exceptions) can have multiple regions in a function.
> But we can probably ignore the latter case; the former can be handled
> by using whichever is larger, the symbol or exception region address.

This is in effect what happens, because if the symbol is larger than
the exception region address, I'm refusing to use the unwind record
and fall back to prologue parsing (which will use the symbol).

> > +  /* See if we have an ARM exception table entry covering this address.  */
> > +  addr_in_block = get_frame_address_in_block (this_frame);
> > +  entry = arm_find_exidx_entry (addr_in_block, &func_exidx);
> > +  if (!entry)
> > +    return 0;
> 
> I assume you're deliberately treating can't-unwind and no-information
> the same.  We use can't-unwind to mark _start, but it also shows up in
> exception handling in some cases.

Hmm, I'm seeing many functions marked as can't-unwind in the Ubuntu
copy of glibc, so I was assuming this just means that for some reason
the unwind data couldn't be generated reliably enough to allow run-time
unwinding.  However, for debugging purposes we still want to attempt to
show a backtrace, so I'm falling back to prologue parsing ...

> > +  /* The ARM exception index does not mark the *end* of the function
> > +     covered by the entry, and some functions will not have any entry.
> > +     Therefore, the entry we found above may not actually be correct
> > +     for this PC.  As a sanity check, also try to determine the function
> > +     covering PC via the symbol table.  If this finds a function start
> > +     address *closer* to PC than the one found via the exception table,
> > +     ignore the exception record, and fall back to prologue parsing.
> > +
> > +     (Note that if we don't find a function via the symbol table, the
> > +     exception table may still be wrong.  But in this case, prologue
> > +     parsing wouldn't work anyway, so we use the exception table and
> > +     hope for the best.)  */
> > +  if (find_pc_partial_function (addr_in_block, NULL, &func_symtab, NULL)
> > +      && func_symtab > func_exidx)
> > +    return 0;
> 
> This doesn't go well with the discussion above about combined
> exception regions... did you encounter this problem in practice?  The
> linker used to have this problem, but hasn't for a while (circa
> 2009-05-05).

Yes, I've definitely seen this in practice.  In the Ubuntu glibc I have:

Unwind table index '.ARM.exidx' at offset 0xe8448 contains 596 entries:

0x15470: 0x800b3fac
  Compact model 0
  0x0b      vsp = vsp + 48
  0x3f      vsp = vsp + 256
  0xac      pop {r4, r5, r6, r7, r8r14}

0x157ec: 0x80aab0b0
  Compact model 0
  0xaa      pop {r4, r5, r6r14}
  0xb0      finish
  0xb0      finish

0x1584c: 0x8003a8b0
  Compact model 0
  0x03      vsp = vsp + 16
  0xa8      pop {r4r14}
  0xb0      finish

0x15890: 0x808408b0
  Compact model 0
  0x84 0x08 pop {r7, r14}
  0xb0      finish

0x25a50: 0x808400b0
  Compact model 0
  0x84 0x00 pop {r14}
  0xb0      finish

0x25a90: 0x80a8b0b0
  Compact model 0
  0xa8      pop {r4r14}
  0xb0      finish
  0xb0      finish

Note the large gap between 0x15890 and 0x25a50, which contains many
functions, including "raise", for which the "pop {r7, r14}" instruction
certainly isn't correct.  This caused test suite failures before I
added the check above.

I understood the ARM standard to say that this was expected, because
for C or assembly routines, presence of an unwind entry is optional.
Is this not the case?

> The actual unwinder looks OK to me.  If you want to handle C++ code
> with exceptions (or C with cleanups), you'll need to check the name of
> the referenced personality routine; it is __gxx_personality_v0 (et
> cetera).  Readelf has a sample of that.  Code which doesn't throw or
> catch uses the standard personality routines, as you've implemented.

Ah, I didn't see the readelf code (because I was looking at an old
checkout, I would appear).  I can add those personality routines as well
(however, these are nowhere used in the glibc I'm looking at).

Thanks for the review!

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Daniel Jacobowitz]

On Thu, Oct 21, 2010 at 05:50:51PM +0200, Ulrich Weigand wrote:
> Hmm, I'm seeing many functions marked as can't-unwind in the Ubuntu
> copy of glibc, so I was assuming this just means that for some reason
> the unwind data couldn't be generated reliably enough to allow run-time
> unwinding.  However, for debugging purposes we still want to attempt to
> show a backtrace, so I'm falling back to prologue parsing ...

Oh, right - can't unwind can be generated by the compiler, or by the
linker.  It should cover all non-fexceptions code with can't-unwind
markers.

> Note the large gap between 0x15890 and 0x25a50, which contains many
> functions, including "raise", for which the "pop {r7, r14}" instruction
> certainly isn't correct.  This caused test suite failures before I
> added the check above.
> 
> I understood the ARM standard to say that this was expected, because
> for C or assembly routines, presence of an unwind entry is optional.
> Is this not the case?

No, this is not the case.  The linker is supposed to fix it up:

/* Scan .ARM.exidx tables, and create a list describing edits which should be
   made to those tables, such that:

     1. Regions without unwind data are marked with EXIDX_CANTUNWIND entries.
     2. Duplicate entries are merged together (EXIDX_CANTUNWIND, or unwind
        codes which have been inlined into the index).

   If MERGE_EXIDX_ENTRIES is false, duplicate entries are not merged.

   The edits are applied when the tables are written
   (in elf32_arm_write_section).
*/

If it's not doing that, we should figure out why - it can lead to
crashes in libgcc, if the unwinder is invoked, rather than the correct
failure to unwind.

I think 2.19 didn't do this but 2.20 did.

> Ah, I didn't see the readelf code (because I was looking at an old
> checkout, I would appear).  I can add those personality routines as well
> (however, these are nowhere used in the glibc I'm looking at).

Correct, they're only used with C cleanups or C++ exception throw/catch.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Ulrich Weigand]

Dan Jacobowitz wrote:
> On Thu, Oct 21, 2010 at 05:50:51PM +0200, Ulrich Weigand wrote:
> > I understood the ARM standard to say that this was expected, because
> > for C or assembly routines, presence of an unwind entry is optional.
> > Is this not the case?
> 
> No, this is not the case.  The linker is supposed to fix it up:
[snip]
> If it's not doing that, we should figure out why - it can lead to
> crashes in libgcc, if the unwinder is invoked, rather than the correct
> failure to unwind.
> 
> I think 2.19 didn't do this but 2.20 did.

Hmm, the Ubuntu glibc image I'm using was built with 2.20.51.20100908-0ubuntu1
which should be recent enough ...

I'm re-running a glibc build from source to try and debug what the linker
is doing here.

> > Ah, I didn't see the readelf code (because I was looking at an old
> > checkout, I would appear).  I can add those personality routines as well
> > (however, these are nowhere used in the glibc I'm looking at).
> 
> Correct, they're only used with C cleanups or C++ exception throw/catch.

One issue with this just occurred to me: comparing personality routines
by *name* relies on symbol information being present.  Since we're doing
the whole ARM unwinder support primarily to better cope with the case
where symbol information is absent, this may be counter-productive ...

Is there some other way to recognize those particular unwinders?


Another, related topic: Running the GDB testsuite on a system without
debug/symbol info, I'm still seeing failures in unwinding from interrupted
system calls.  This is because the assembler code to do the syscall
clobbers r7 without generating appropriate unwind records, and thus
unwinding fails somewhere higher up the stack.

Now, this is exactly the problem you fixed by moving the actual syscall
into a separate routine __libc_do_syscall.  However, this routine is
only called from C code built for -mthumb.  C code built for -marm,
as well as code originally in (ARM) assembler, will continue to use
inline sequences clobbering r7.

Note that even in a glibc built completely for Thumb, there still will
be some instances of the latter, e.g. the automatically generated
system call stubs (like the one for "kill") as well as certain
routines provided fully in assembler (like "vfork").  Unfortunately,
if these are called from Thumb code higher up the stack, the fact
that those routines clobber r7 (even as they themselves are ARM),
still breaks unwinding ...

Any thoughts how to fix this?  Should those calls also use an out-of-line
syscall helper?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Daniel Jacobowitz]

On Thu, Oct 21, 2010 at 08:26:09PM +0200, Ulrich Weigand wrote:
> One issue with this just occurred to me: comparing personality routines
> by *name* relies on symbol information being present.  Since we're doing
> the whole ARM unwinder support primarily to better cope with the case
> where symbol information is absent, this may be counter-productive ...
> 
> Is there some other way to recognize those particular unwinders?

Yes, it's true that this requires names.  Fortunately, in the usual
case they are in the dynamic symbol table.  So, I think you'll have
the names - or do we not see dynamic symbols here?

> Another, related topic: Running the GDB testsuite on a system without
> debug/symbol info, I'm still seeing failures in unwinding from interrupted
> system calls.  This is because the assembler code to do the syscall
> clobbers r7 without generating appropriate unwind records, and thus
> unwinding fails somewhere higher up the stack.
> 
> Now, this is exactly the problem you fixed by moving the actual syscall
> into a separate routine __libc_do_syscall.  However, this routine is
> only called from C code built for -mthumb.  C code built for -marm,
> as well as code originally in (ARM) assembler, will continue to use
> inline sequences clobbering r7.

This should work... the problem with r7 was that it's the hard frame
pointer for Thumb (even Thumb-2 - although it shouldn't be, most
likely, it's a wasteful choice).  So we couldn't mark it as
clobbered.  Does GCC not mark r7 as saved on the stack for the ARM
code?  It should know perfectly well that r7 is not unchanged.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Ulrich Weigand]

Dan Jacobowitz wrote:
> On Thu, Oct 21, 2010 at 08:26:09PM +0200, Ulrich Weigand wrote:
> > One issue with this just occurred to me: comparing personality routines
> > by *name* relies on symbol information being present.  Since we're doing
> > the whole ARM unwinder support primarily to better cope with the case
> > where symbol information is absent, this may be counter-productive ...
> > 
> > Is there some other way to recognize those particular unwinders?
> 
> Yes, it's true that this requires names.  Fortunately, in the usual
> case they are in the dynamic symbol table.  So, I think you'll have
> the names - or do we not see dynamic symbols here?

Hmm, I was confused by readelf apparently being unable to resolve
them in the case of libstdc++:

Unwind table index '.ARM.exidx' at offset 0x95768 contains 1433 entries:

0x44704: 0x1 [cantunwind]

0x4478c: 0x80aab0b0
  Compact model 0
  0xaa      pop {r4, r5, r6r14}
  0xb0      finish
  0xb0      finish

0x44804: 0x1 [cantunwind]

0x44938: @0x8fec0
  Personality routine: 0x43264

0x44990: @0x8fedc
  Personality routine: 0x43264

0x44b00: 0x1 [cantunwind]


But in fact 0x43264 is the address of the PLT entry pointing to this
jump slot:
000a4480  000d7516 R_ARM_JUMP_SLOT   00084e25   __gxx_personality_v0

So we should certainly be able to resolve that in GDB.


> > Another, related topic: Running the GDB testsuite on a system without
> > debug/symbol info, I'm still seeing failures in unwinding from interrupted
> > system calls.  This is because the assembler code to do the syscall
> > clobbers r7 without generating appropriate unwind records, and thus
> > unwinding fails somewhere higher up the stack.
> > 
> > Now, this is exactly the problem you fixed by moving the actual syscall
> > into a separate routine __libc_do_syscall.  However, this routine is
> > only called from C code built for -mthumb.  C code built for -marm,
> > as well as code originally in (ARM) assembler, will continue to use
> > inline sequences clobbering r7.
> 
> This should work... the problem with r7 was that it's the hard frame
> pointer for Thumb (even Thumb-2 - although it shouldn't be, most
> likely, it's a wasteful choice).  So we couldn't mark it as
> clobbered.  Does GCC not mark r7 as saved on the stack for the ARM
> code?  It should know perfectly well that r7 is not unchanged.

Right, for C code there should be no problem.  The problems I've been
seeing all come from *assembler* source files using the DO_CALL macro.
This gets CFI correct, but doesn't create any unwind records ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Daniel Jacobowitz]

On Thu, Oct 21, 2010 at 10:29:32PM +0200, Ulrich Weigand wrote:
> Right, for C code there should be no problem.  The problems I've been
> seeing all come from *assembler* source files using the DO_CALL macro.
> This gets CFI correct, but doesn't create any unwind records ...

Oh.... yes, we could move that out of line for ARM too.  I don't like
the solution much though, because it lengthens all syscall backtraces
unnecessarily.

We're getting into the point where backtraces using the ARM unwinder
are not useful for GDB because they're flow-insensitive.  But if all
the callers of DO_CALL don't contain any other calls, we have the
option to record the ip = r7 save as if the syscall is the only
possible 'call site'.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Ulrich Weigand]

Dan Jacobowitz wrote:
> No, this is not the case.  The linker is supposed to fix it up:
> 
> /* Scan .ARM.exidx tables, and create a list describing edits which should be
>    made to those tables, such that:
> 
>      1. Regions without unwind data are marked with EXIDX_CANTUNWIND entries.
>      2. Duplicate entries are merged together (EXIDX_CANTUNWIND, or unwind
>         codes which have been inlined into the index).
> 
>    If MERGE_EXIDX_ENTRIES is false, duplicate entries are not merged.
> 
>    The edits are applied when the tables are written
>    (in elf32_arm_write_section).
> */
> 
> If it's not doing that, we should figure out why - it can lead to
> crashes in libgcc, if the unwinder is invoked, rather than the correct
> failure to unwind.
> 
> I think 2.19 didn't do this but 2.20 did.

Well, the function elf32_arm_fix_exidx_coverage you refer to above is
never called for a relocatable link ...

Unfortunately, the glibc link process works like this (abbreviated):
 - build all the objects (some with exidx, some without)
 - collect objects into an archive libc_pic.a
 - do a relocatable link:  -o libc_pic.os -r --whole-archive libc_pic.a
 - build shared object: -o libc.so -shared ... libc_pic.os ... 

The relocatable link step concatenates the text sections of objects
with exidx and those without, but does not call any special fixup
routine, so no EXIDX_CANTUNWIND entries are inserted.

The shared object final link step *does* run the fixup routine, but
since it now only sees a single big text section, it doesn't fix up
anything ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Daniel Jacobowitz]

On Fri, Oct 22, 2010 at 12:26:05AM +0200, Ulrich Weigand wrote:
> Well, the function elf32_arm_fix_exidx_coverage you refer to above is
> never called for a relocatable link ...

Boo.  I consider this a linker bug.  I'll file an internal issue about
it; thanks for the analysis.

For now, I suppose GDB gets to cope.  I'd like to remove that
workaround someday, but we have to fix the linker first.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Ulrich Weigand]

Dan Jacobowitz wrote:
> On Thu, Oct 21, 2010 at 10:29:32PM +0200, Ulrich Weigand wrote:
> > Right, for C code there should be no problem.  The problems I've been
> > seeing all come from *assembler* source files using the DO_CALL macro.
> > This gets CFI correct, but doesn't create any unwind records ...
> 
> Oh.... yes, we could move that out of line for ARM too.  I don't like
> the solution much though, because it lengthens all syscall backtraces
> unnecessarily.
> 
> We're getting into the point where backtraces using the ARM unwinder
> are not useful for GDB because they're flow-insensitive.  But if all
> the callers of DO_CALL don't contain any other calls, we have the
> option to record the ip = r7 save as if the syscall is the only
> possible 'call site'.

Hmm, looking at DO_CALL a bit more I think this can actually be handled
inline just fine:

#define DO_CALL(syscall_name, args)             \
    DOARGS_##args;                              \
    mov ip, r7;                                 \
    cfi_register (r7, ip);                      \
    ldr r7, =SYS_ify (syscall_name);            \
    swi 0x0;                                    \
    mov r7, ip;                                 \
    cfi_restore (r7);                           \
    UNDOARGS_##args

Note the calls to DOARGS_... / UNDOARGS_..., which actually save and
restore registers on the stack anyway (just not r7).  Why not simply
have (a variant) of [UN]DOARGS_... save r7 to the stack, and then
provide both correct CFI and ARM unwind records for it?  [ Saving
to IP would then be no longer necessary. ]

Am I missing something here?


Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Daniel Jacobowitz]

On Wed, Dec 01, 2010 at 05:45:31PM +0100, Ulrich Weigand wrote:
> Hmm, looking at DO_CALL a bit more I think this can actually be handled
> inline just fine:
> 
> #define DO_CALL(syscall_name, args)             \
>     DOARGS_##args;                              \
>     mov ip, r7;                                 \
>     cfi_register (r7, ip);                      \
>     ldr r7, =SYS_ify (syscall_name);            \
>     swi 0x0;                                    \
>     mov r7, ip;                                 \
>     cfi_restore (r7);                           \
>     UNDOARGS_##args
> 
> Note the calls to DOARGS_... / UNDOARGS_..., which actually save and
> restore registers on the stack anyway (just not r7).  Why not simply
> have (a variant) of [UN]DOARGS_... save r7 to the stack, and then
> provide both correct CFI and ARM unwind records for it?  [ Saving
> to IP would then be no longer necessary. ]
> 
> Am I missing something here?

I don't think you are missing anything, except a couple of cycles.

We save r7 in ip because it ought to be faster loading and storing it
on the stack.  But we already take the position that the system call
overhead is substantial compared to DO_CALL... so the extra two memory
ops do not seem like a huge loss to me, especially if we can save on
code size.

I haven't looked at this code in a while, but isn't
nptl/sysdep-cancel.h:PSEUDO broken for non-cancellable syscalls with
many arguments?  It calls DOARGS and then DO_CALL, but DO_CALL does
DOARGS again.  For 0-4 arguments it doesn't matter; I don't know if
any of the 5/6/7 argument calls have a used nocancel variant.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Andreas Schwab]

Daniel Jacobowitz <dan@codesourcery.com> writes:

> I haven't looked at this code in a while, but isn't
> nptl/sysdep-cancel.h:PSEUDO broken for non-cancellable syscalls with
> many arguments?  It calls DOARGS and then DO_CALL, but DO_CALL does
> DOARGS again.  For 0-4 arguments it doesn't matter; I don't know if
> any of the 5/6/7 argument calls have a used nocancel variant.

It calls DO_CALL with args == 0, so DOARGS/UNDOARGS do nothing.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Wed, Dec 01, 2010 at 05:45:31PM +0100, Ulrich Weigand wrote:
> > Hmm, looking at DO_CALL a bit more I think this can actually be handled
> > inline just fine:
> > 
> > #define DO_CALL(syscall_name, args)             \
> >     DOARGS_##args;                              \
> >     mov ip, r7;                                 \
> >     cfi_register (r7, ip);                      \
> >     ldr r7, =SYS_ify (syscall_name);            \
> >     swi 0x0;                                    \
> >     mov r7, ip;                                 \
> >     cfi_restore (r7);                           \
> >     UNDOARGS_##args
> > 
> > Note the calls to DOARGS_... / UNDOARGS_..., which actually save and
> > restore registers on the stack anyway (just not r7).  Why not simply
> > have (a variant) of [UN]DOARGS_... save r7 to the stack, and then
> > provide both correct CFI and ARM unwind records for it?  [ Saving
> > to IP would then be no longer necessary. ]
> > 
> > Am I missing something here?
> 
> I don't think you are missing anything, except a couple of cycles.
> 
> We save r7 in ip because it ought to be faster loading and storing it
> on the stack.  But we already take the position that the system call
> overhead is substantial compared to DO_CALL... so the extra two memory
> ops do not seem like a huge loss to me, especially if we can save on
> code size.

I finally got around to continue working on this.  The patch below
implements this approach.  However, this uncovered one rather fundamental
problem:  when implementing the vfork system call, the syscall stub
must *not* put anything on the stack or else things will break due to
the peculiar semantics of vfork:

The system will return to execute solely the child at first, until such
time as the child calls exec; and in the meantime, the child will share
the parent's address space.  Now if vfork pushed something on the stack,
the child will pop that value, and go on to overwrite that stack location
with other stuff.  Once the parent later gets to run as well, it will
restore a corrupted value into r7 ...

I'm not quite sure if there is a way around this ... Any suggestions
would be appreciated.

> I haven't looked at this code in a while, but isn't
> nptl/sysdep-cancel.h:PSEUDO broken for non-cancellable syscalls with
> many arguments?  It calls DOARGS and then DO_CALL, but DO_CALL does
> DOARGS again.  For 0-4 arguments it doesn't matter; I don't know if
> any of the 5/6/7 argument calls have a used nocancel variant.

As Andreas notes, DOARGS is called with a 0 argument.  However, I'm
wondering if there isn't a bug anyway:

  __##syscall_name##_nocancel:                                          \
    .cfi_sections .debug_frame;                                         \
    cfi_startproc;                                                      \
    DO_CALL (syscall_name, args);                                       \
    PSEUDO_RET;                                                         \
    cfi_endproc;                                                        \
  .size __##syscall_name##_nocancel,.-__##syscall_name##_nocancel;      \

Shouldn't there be a 
    cmn r0, $4096;
in between the DO_CALL and the PSEUDO_RET?

Also, some of the code in the linuxthreads/ directories seems to be broken,
but I'm wondering why those are still there in the first place; linuxthreads
support has been removed from glibc a long time ago, hasn't it?

Bye,
Ulrich

diff --git a/sysdeps/unix/sysv/linux/arm/eabi/nptl/sysdep-cancel.h b/sysdeps/unix/sysv/linux/arm/eabi/nptl/sysdep-cancel.h
index 458558b..7819578 100644
--- a/sysdeps/unix/sysv/linux/arm/eabi/nptl/sysdep-cancel.h
+++ b/sysdeps/unix/sysv/linux/arm/eabi/nptl/sysdep-cancel.h
@@ -44,29 +44,22 @@
   .size __##syscall_name##_nocancel,.-__##syscall_name##_nocancel;	\
   ENTRY (name);								\
     SINGLE_THREAD_P;							\
-    DOARGS_##args;							\
     bne .Lpseudo_cancel;						\
-    cfi_remember_state;							\
-    DO_CALL (syscall_name, 0);						\
-    UNDOARGS_##args;							\
+    DO_CALL (syscall_name, args);					\
     cmn r0, $4096;							\
     PSEUDO_RET;								\
-    cfi_restore_state;							\
   .Lpseudo_cancel:							\
-    .fnstart;								\
     DOCARGS_##args;	/* save syscall args etc. around CENABLE.  */	\
     CENABLE;								\
     mov ip, r0;		/* put mask in safe place.  */			\
     UNDOCARGS_##args;	/* restore syscall args.  */			\
     ldr r7, =SYS_ify (syscall_name);					\
     swi 0x0;		/* do the call.  */				\
-    .fnend;		/* Past here we can't easily unwind.  */	\
     mov r7, r0;		/* save syscall return value.  */		\
     mov r0, ip;		/* get mask back.  */				\
     CDISABLE;								\
     mov r0, r7;		/* retrieve return value.  */			\
     RESTORE_LR_##args;							\
-    UNDOARGS_##args;							\
     cmn r0, $4096
 
 /* DOARGS pushes four bytes on the stack for five arguments, eight bytes for
@@ -79,6 +72,7 @@
   .save {r7, lr}
 
 # define DOCARGS_0 \
+  .fnstart; \
   stmfd sp!, {r7, lr}; \
   cfi_adjust_cfa_offset (8); \
   cfi_rel_offset (r7, 0); \
@@ -89,9 +83,11 @@
   ldmfd sp!, {r7, lr}; \
   cfi_adjust_cfa_offset (-8); \
   cfi_restore (r7); \
-  cfi_restore (lr)
+  cfi_restore (lr); \
+  .fnend
 
 # define DOCARGS_1 \
+  .fnstart; \
   stmfd sp!, {r0, r1, r7, lr}; \
   cfi_adjust_cfa_offset (16); \
   cfi_rel_offset (r7, 8); \
@@ -106,6 +102,7 @@
   RESTORE_LR_0
 
 # define DOCARGS_2 \
+  .fnstart; \
   stmfd sp!, {r0, r1, r7, lr}; \
   cfi_adjust_cfa_offset (16); \
   cfi_rel_offset (r7, 8); \
@@ -120,6 +117,7 @@
   RESTORE_LR_0
 
 # define DOCARGS_3 \
+  .fnstart; \
   stmfd sp!, {r0, r1, r2, r3, r7, lr}; \
   cfi_adjust_cfa_offset (24); \
   cfi_rel_offset (r7, 16); \
@@ -134,6 +132,7 @@
   RESTORE_LR_0
 
 # define DOCARGS_4 \
+  .fnstart; \
   stmfd sp!, {r0, r1, r2, r3, r7, lr}; \
   cfi_adjust_cfa_offset (24); \
   cfi_rel_offset (r7, 16); \
@@ -147,47 +146,58 @@
 # define RESTORE_LR_4 \
   RESTORE_LR_0
 
-/* r4 is only stmfd'ed for correct stack alignment.  */
 # define DOCARGS_5 \
-  .save {r4}; \
-  stmfd sp!, {r0, r1, r2, r3, r4, r7, lr}; \
-  cfi_adjust_cfa_offset (28); \
-  cfi_rel_offset (r7, 20); \
-  cfi_rel_offset (lr, 24); \
-  .save {r7, lr}; \
-  .pad #20
+  .fnstart; \
+  stmfd sp!, {r0, r1, r2, r3, r4, r5, r7, lr}; \
+  cfi_adjust_cfa_offset (32); \
+  cfi_rel_offset (r4, 16); \
+  cfi_rel_offset (r5, 20); \
+  cfi_rel_offset (r7, 24); \
+  cfi_rel_offset (lr, 28); \
+  .save {r4, r5, r7, lr}; \
+  .pad #16; \
+  ldr r4, [sp, $32]
 # define UNDOCARGS_5 \
   ldmfd sp!, {r0, r1, r2, r3}; \
   cfi_adjust_cfa_offset (-16); \
   .fnend; \
   .fnstart; \
-  .save {r4}; \
-  .save {r7, lr}; \
-  .pad #4
+  .save {r4, r5, r7, lr};
 # define RESTORE_LR_5 \
-  ldmfd sp!, {r4, r7, lr}; \
-  cfi_adjust_cfa_offset (-12); \
-  /* r4 will be marked as restored later.  */ \
+  ldmfd sp!, {r4, r5, r7, lr}; \
+  cfi_adjust_cfa_offset (-16); \
+  cfi_restore (r4); \
+  cfi_restore (r5); \
   cfi_restore (r7); \
-  cfi_restore (lr)
+  cfi_restore (lr); \
+  .fnend
 
 # define DOCARGS_6 \
-  .save {r4, r5}; \
-  stmfd sp!, {r0, r1, r2, r3, r7, lr}; \
-  cfi_adjust_cfa_offset (24); \
+  .fnstart; \
+  mov ip, sp; \
+  stmfd sp!, {r0, r1, r2, r3, r4, r5, r7, lr}; \
+  cfi_adjust_cfa_offset (32); \
+  cfi_rel_offset (r4, 16); \
+  cfi_rel_offset (r5, 20); \
   cfi_rel_offset (r7, 16); \
   cfi_rel_offset (lr, 20); \
-  .save {r7, lr}; \
-  .pad #16
+  .save {r4, r5, r7, lr}; \
+  .pad #16; \
+  ldmia ip, {r4, r5}
 # define UNDOCARGS_6 \
   ldmfd sp!, {r0, r1, r2, r3}; \
   cfi_adjust_cfa_offset (-16); \
   .fnend; \
   .fnstart; \
-  .save {r4, r5}; \
-  .save {r7, lr}
+  .save {r4, r5, r7, lr};
 # define RESTORE_LR_6 \
-  RESTORE_LR_0
+  ldmfd sp!, {r4, r5, r7, lr}; \
+  cfi_adjust_cfa_offset (-16); \
+  cfi_restore (r4); \
+  cfi_restore (r5); \
+  cfi_restore (r7); \
+  cfi_restore (lr); \
+  .fnend
 
 # ifdef IS_IN_libpthread
 #  define CENABLE	bl PLTJMP(__pthread_enable_asynccancel)
diff --git a/sysdeps/unix/sysv/linux/arm/eabi/sysdep.h b/sysdeps/unix/sysv/linux/arm/eabi/sysdep.h
index b7815ba..70bbfa6 100644
--- a/sysdeps/unix/sysv/linux/arm/eabi/sysdep.h
+++ b/sysdeps/unix/sysv/linux/arm/eabi/sysdep.h
@@ -106,12 +106,95 @@
 #undef	DO_CALL
 #define DO_CALL(syscall_name, args)		\
     DOARGS_##args;				\
-    mov ip, r7;					\
-    cfi_register (r7, ip);			\
     ldr r7, =SYS_ify (syscall_name);		\
     swi 0x0;					\
-    mov r7, ip;					\
-    cfi_restore (r7);				\
     UNDOARGS_##args
 
+#undef  DOARGS_0
+#define DOARGS_0 \
+  .fnstart; \
+  str r7, [sp, $-4]!; \
+  cfi_adjust_cfa_offset (4); \
+  cfi_rel_offset (r7, 0); \
+  .save { r7 }
+#undef  DOARGS_1
+#define DOARGS_1 DOARGS_0
+#undef  DOARGS_2
+#define DOARGS_2 DOARGS_0
+#undef  DOARGS_3
+#define DOARGS_3 DOARGS_0
+#undef  DOARGS_4
+#define DOARGS_4 DOARGS_0
+#undef  DOARGS_5
+#define DOARGS_5 \
+  .fnstart; \
+  stmfd sp!, {r4, r7}; \
+  cfi_adjust_cfa_offset (8); \
+  cfi_rel_offset (r4, 0); \
+  cfi_rel_offset (r7, 4); \
+  .save { r4, r7 }; \
+  ldr r4, [sp, $8]
+#undef  DOARGS_6
+#define DOARGS_6 \
+  .fnstart; \
+  mov ip, sp; \
+  stmfd sp!, {r4, r5, r7}; \
+  cfi_adjust_cfa_offset (12); \
+  cfi_rel_offset (r4, 0); \
+  cfi_rel_offset (r5, 4); \
+  cfi_rel_offset (r7, 8); \
+  .save { r4, r5, r7 }; \
+  ldmia ip, {r4, r5}
+#undef  DOARGS_7
+#define DOARGS_7 \
+  .fnstart; \
+  mov ip, sp; \
+  stmfd sp!, {r4, r5, r6, r7}; \
+  cfi_adjust_cfa_offset (16); \
+  cfi_rel_offset (r4, 0); \
+  cfi_rel_offset (r5, 4); \
+  cfi_rel_offset (r6, 8); \
+  cfi_rel_offset (r7, 12); \
+  .save { r4, r5, r6, r7 }; \
+  ldmia ip, {r4, r5, r6}
+
+#undef  UNDOARGS_0
+#define UNDOARGS_0 \
+  ldr r7, [sp], $4; \
+  cfi_adjust_cfa_offset (-4); \
+  cfi_restore (r7); \
+  .fnend
+#undef  UNDOARGS_1
+#define UNDOARGS_1 UNDOARGS_0
+#undef  UNDOARGS_2
+#define UNDOARGS_2 UNDOARGS_0
+#undef  UNDOARGS_3
+#define UNDOARGS_3 UNDOARGS_0
+#undef  UNDOARGS_4
+#define UNDOARGS_4 UNDOARGS_0
+#undef  UNDOARGS_5
+#define UNDOARGS_5 \
+  ldmfd sp!, {r4, r7}; \
+  cfi_adjust_cfa_offset (-8); \
+  cfi_restore (r4); \
+  cfi_restore (r7); \
+  .fnend
+#undef  UNDOARGS_6
+#define UNDOARGS_6 \
+  ldmfd sp!, {r4, r5, r7}; \
+  cfi_adjust_cfa_offset (-12); \
+  cfi_restore (r4); \
+  cfi_restore (r5); \
+  cfi_restore (r7); \
+  .fnend
+#undef  UNDOARGS_7
+#define UNDOARGS_7 \
+  ldmfd sp!, {r4, r5, r6, r7}; \
+  cfi_adjust_cfa_offset (-16); \
+  cfi_restore (r4); \
+  cfi_restore (r5); \
+  cfi_restore (r6); \
+  cfi_restore (r7); \
+  .fnend
+
 #endif /* _LINUX_ARM_EABI_SYSDEP_H */

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Daniel Jacobowitz]

On Wed, Mar 09, 2011 at 07:43:04PM +0100, Ulrich Weigand wrote:
> Daniel Jacobowitz wrote:
> > On Wed, Dec 01, 2010 at 05:45:31PM +0100, Ulrich Weigand wrote:
> > > Hmm, looking at DO_CALL a bit more I think this can actually be handled
> > > inline just fine:
> > > 
> > > #define DO_CALL(syscall_name, args)             \
> > >     DOARGS_##args;                              \
> > >     mov ip, r7;                                 \
> > >     cfi_register (r7, ip);                      \
> > >     ldr r7, =SYS_ify (syscall_name);            \
> > >     swi 0x0;                                    \
> > >     mov r7, ip;                                 \
> > >     cfi_restore (r7);                           \
> > >     UNDOARGS_##args
> > > 
> > > Note the calls to DOARGS_... / UNDOARGS_..., which actually save and
> > > restore registers on the stack anyway (just not r7).  Why not simply
> > > have (a variant) of [UN]DOARGS_... save r7 to the stack, and then
> > > provide both correct CFI and ARM unwind records for it?  [ Saving
> > > to IP would then be no longer necessary. ]
> > > 
> > > Am I missing something here?
> > 
> > I don't think you are missing anything, except a couple of cycles.
> > 
> > We save r7 in ip because it ought to be faster loading and storing it
> > on the stack.  But we already take the position that the system call
> > overhead is substantial compared to DO_CALL... so the extra two memory
> > ops do not seem like a huge loss to me, especially if we can save on
> > code size.
> 
> I finally got around to continue working on this.  The patch below
> implements this approach.  However, this uncovered one rather fundamental
> problem:  when implementing the vfork system call, the syscall stub
> must *not* put anything on the stack or else things will break due to
> the peculiar semantics of vfork:
> 
> The system will return to execute solely the child at first, until such
> time as the child calls exec; and in the meantime, the child will share
> the parent's address space.  Now if vfork pushed something on the stack,
> the child will pop that value, and go on to overwrite that stack location
> with other stuff.  Once the parent later gets to run as well, it will
> restore a corrupted value into r7 ...
> 
> I'm not quite sure if there is a way around this ... Any suggestions
> would be appreciated.

Hmm, I wonder if that's why I did it this way before.

Can we use a different technique to implement vfork than for other
system calls?  I don't see a reason they all need to be the same.

> As Andreas notes, DOARGS is called with a 0 argument.  However, I'm
> wondering if there isn't a bug anyway:
> 
>   __##syscall_name##_nocancel:                                          \
>     .cfi_sections .debug_frame;                                         \
>     cfi_startproc;                                                      \
>     DO_CALL (syscall_name, args);                                       \
>     PSEUDO_RET;                                                         \
>     cfi_endproc;                                                        \
>   .size __##syscall_name##_nocancel,.-__##syscall_name##_nocancel;      \
> 
> Shouldn't there be a 
>     cmn r0, $4096;
> in between the DO_CALL and the PSEUDO_RET?

It does look that way.

> Also, some of the code in the linuxthreads/ directories seems to be broken,
> but I'm wondering why those are still there in the first place; linuxthreads
> support has been removed from glibc a long time ago, hasn't it?

Yes.  Those files should just be removed.

-- 
Daniel Jacobowitz
Mentor Graphics

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Wed, Mar 09, 2011 at 07:43:04PM +0100, Ulrich Weigand wrote:
> > I finally got around to continue working on this.  The patch below
> > implements this approach.  However, this uncovered one rather fundamental
> > problem:  when implementing the vfork system call, the syscall stub
> > must *not* put anything on the stack or else things will break due to
> > the peculiar semantics of vfork:
> > 
> > The system will return to execute solely the child at first, until such
> > time as the child calls exec; and in the meantime, the child will share
> > the parent's address space.  Now if vfork pushed something on the stack,
> > the child will pop that value, and go on to overwrite that stack location
> > with other stuff.  Once the parent later gets to run as well, it will
> > restore a corrupted value into r7 ...
> > 
> > I'm not quite sure if there is a way around this ... Any suggestions
> > would be appreciated.
> 
> Hmm, I wonder if that's why I did it this way before.
> 
> Can we use a different technique to implement vfork than for other
> system calls?  I don't see a reason they all need to be the same.

After some back and forth, I've been able to come up with a solution
that works well enough for all GDB tests to pass.  It is not completely
perfect since unwinding (on the parent side) out of an interrupted vfork
call works only while the child has not yet clobbered the stack, but I
don't see how to do anything better ...

Patch is appended below.  Does this look reasonable to you?

> > As Andreas notes, DOARGS is called with a 0 argument.  However, I'm
> > wondering if there isn't a bug anyway:
> > 
> >   __##syscall_name##_nocancel:                                          \
> >     .cfi_sections .debug_frame;                                         \
> >     cfi_startproc;                                                      \
> >     DO_CALL (syscall_name, args);                                       \
> >     PSEUDO_RET;                                                         \
> >     cfi_endproc;                                                        \
> >   .size __##syscall_name##_nocancel,.-__##syscall_name##_nocancel;      \
> > 
> > Shouldn't there be a 
> >     cmn r0, $4096;
> > in between the DO_CALL and the PSEUDO_RET?
> 
> It does look that way.
> 
> > Also, some of the code in the linuxthreads/ directories seems to be broken,
> > but I'm wondering why those are still there in the first place; linuxthreads
> > support has been removed from glibc a long time ago, hasn't it?
> 
> Yes.  Those files should just be removed.

OK, thanks.  I'll prepare patches for those two issues.

Bye,
Ulrich


diff --git a/sysdeps/unix/sysv/linux/arm/eabi/nptl/sysdep-cancel.h b/sysdeps/unix/sysv/linux/arm/eabi/nptl/sysdep-cancel.h
index 458558b..f4a8af4 100644
--- a/sysdeps/unix/sysv/linux/arm/eabi/nptl/sysdep-cancel.h
+++ b/sysdeps/unix/sysv/linux/arm/eabi/nptl/sysdep-cancel.h
@@ -47,20 +47,20 @@
     DOARGS_##args;							\
     bne .Lpseudo_cancel;						\
     cfi_remember_state;							\
-    DO_CALL (syscall_name, 0);						\
+    ldr r7, =SYS_ify (syscall_name);					\
+    swi 0x0;								\
     UNDOARGS_##args;							\
     cmn r0, $4096;							\
     PSEUDO_RET;								\
     cfi_restore_state;							\
   .Lpseudo_cancel:							\
-    .fnstart;								\
+    .fnstart;		/* matched by the .fnend in UNDOARGS below.  */	\
     DOCARGS_##args;	/* save syscall args etc. around CENABLE.  */	\
     CENABLE;								\
     mov ip, r0;		/* put mask in safe place.  */			\
     UNDOCARGS_##args;	/* restore syscall args.  */			\
     ldr r7, =SYS_ify (syscall_name);					\
     swi 0x0;		/* do the call.  */				\
-    .fnend;		/* Past here we can't easily unwind.  */	\
     mov r7, r0;		/* save syscall return value.  */		\
     mov r0, ip;		/* get mask back.  */				\
     CDISABLE;								\
@@ -69,34 +69,34 @@
     UNDOARGS_##args;							\
     cmn r0, $4096
 
-/* DOARGS pushes four bytes on the stack for five arguments, eight bytes for
-   six arguments, and nothing for fewer.  In order to preserve doubleword
+/* DOARGS pushes eight bytes on the stack for five arguments, twelve bytes for
+   six arguments, and four bytes for fewer.  In order to preserve doubleword
    alignment, sometimes we must save an extra register.  */
 
 # define RESTART_UNWIND \
   .fnend; \
   .fnstart; \
-  .save {r7, lr}
+  .save {r7}; \
+  .save {lr}
 
 # define DOCARGS_0 \
-  stmfd sp!, {r7, lr}; \
-  cfi_adjust_cfa_offset (8); \
-  cfi_rel_offset (r7, 0); \
-  cfi_rel_offset (lr, 4); \
-  .save {r7, lr}
+  .save {r7}; \
+  str lr, [sp, #-4]!; \
+  cfi_adjust_cfa_offset (4); \
+  cfi_rel_offset (lr, 0); \
+  .save {lr}
 # define UNDOCARGS_0
 # define RESTORE_LR_0 \
-  ldmfd sp!, {r7, lr}; \
-  cfi_adjust_cfa_offset (-8); \
-  cfi_restore (r7); \
+  ldr lr, [sp], #4; \
+  cfi_adjust_cfa_offset (-4); \
   cfi_restore (lr)
 
 # define DOCARGS_1 \
-  stmfd sp!, {r0, r1, r7, lr}; \
-  cfi_adjust_cfa_offset (16); \
-  cfi_rel_offset (r7, 8); \
-  cfi_rel_offset (lr, 12); \
-  .save {r7, lr}; \
+  .save {r7}; \
+  stmfd sp!, {r0, r1, lr}; \
+  cfi_adjust_cfa_offset (12); \
+  cfi_rel_offset (lr, 8); \
+  .save {lr}; \
   .pad #8
 # define UNDOCARGS_1 \
   ldr r0, [sp], #8; \
@@ -106,11 +106,11 @@
   RESTORE_LR_0
 
 # define DOCARGS_2 \
-  stmfd sp!, {r0, r1, r7, lr}; \
-  cfi_adjust_cfa_offset (16); \
-  cfi_rel_offset (r7, 8); \
-  cfi_rel_offset (lr, 12); \
-  .save {r7, lr}; \
+  .save {r7}; \
+  stmfd sp!, {r0, r1, lr}; \
+  cfi_adjust_cfa_offset (12); \
+  cfi_rel_offset (lr, 8); \
+  .save {lr}; \
   .pad #8
 # define UNDOCARGS_2 \
   ldmfd sp!, {r0, r1}; \
@@ -120,11 +120,11 @@
   RESTORE_LR_0
 
 # define DOCARGS_3 \
-  stmfd sp!, {r0, r1, r2, r3, r7, lr}; \
-  cfi_adjust_cfa_offset (24); \
-  cfi_rel_offset (r7, 16); \
-  cfi_rel_offset (lr, 20); \
-  .save {r7, lr}; \
+  .save {r7}; \
+  stmfd sp!, {r0, r1, r2, r3, lr}; \
+  cfi_adjust_cfa_offset (20); \
+  cfi_rel_offset (lr, 16); \
+  .save {lr}; \
   .pad #16
 # define UNDOCARGS_3 \
   ldmfd sp!, {r0, r1, r2, r3}; \
@@ -134,11 +134,11 @@
   RESTORE_LR_0
 
 # define DOCARGS_4 \
-  stmfd sp!, {r0, r1, r2, r3, r7, lr}; \
-  cfi_adjust_cfa_offset (24); \
-  cfi_rel_offset (r7, 16); \
-  cfi_rel_offset (lr, 20); \
-  .save {r7, lr}; \
+  .save {r7}; \
+  stmfd sp!, {r0, r1, r2, r3, lr}; \
+  cfi_adjust_cfa_offset (20); \
+  cfi_rel_offset (lr, 16); \
+  .save {lr}; \
   .pad #16
 # define UNDOCARGS_4 \
   ldmfd sp!, {r0, r1, r2, r3}; \
@@ -149,43 +149,40 @@
 
 /* r4 is only stmfd'ed for correct stack alignment.  */
 # define DOCARGS_5 \
-  .save {r4}; \
-  stmfd sp!, {r0, r1, r2, r3, r4, r7, lr}; \
-  cfi_adjust_cfa_offset (28); \
-  cfi_rel_offset (r7, 20); \
-  cfi_rel_offset (lr, 24); \
-  .save {r7, lr}; \
+  .save {r4, r7}; \
+  stmfd sp!, {r0, r1, r2, r3, r4, lr}; \
+  cfi_adjust_cfa_offset (24); \
+  cfi_rel_offset (lr, 20); \
+  .save {lr}; \
   .pad #20
 # define UNDOCARGS_5 \
   ldmfd sp!, {r0, r1, r2, r3}; \
   cfi_adjust_cfa_offset (-16); \
   .fnend; \
   .fnstart; \
-  .save {r4}; \
-  .save {r7, lr}; \
+  .save {r4, r7}; \
+  .save {lr}; \
   .pad #4
 # define RESTORE_LR_5 \
-  ldmfd sp!, {r4, r7, lr}; \
-  cfi_adjust_cfa_offset (-12); \
+  ldmfd sp!, {r4, lr}; \
+  cfi_adjust_cfa_offset (-8); \
   /* r4 will be marked as restored later.  */ \
-  cfi_restore (r7); \
   cfi_restore (lr)
 
 # define DOCARGS_6 \
-  .save {r4, r5}; \
-  stmfd sp!, {r0, r1, r2, r3, r7, lr}; \
-  cfi_adjust_cfa_offset (24); \
-  cfi_rel_offset (r7, 16); \
-  cfi_rel_offset (lr, 20); \
-  .save {r7, lr}; \
+  .save {r4, r5, r7}; \
+  stmfd sp!, {r0, r1, r2, r3, lr}; \
+  cfi_adjust_cfa_offset (20); \
+  cfi_rel_offset (lr, 16); \
+  .save {lr}; \
   .pad #16
 # define UNDOCARGS_6 \
   ldmfd sp!, {r0, r1, r2, r3}; \
   cfi_adjust_cfa_offset (-16); \
   .fnend; \
   .fnstart; \
-  .save {r4, r5}; \
-  .save {r7, lr}
+  .save {r4, r5, r7}; \
+  .save {lr};
 # define RESTORE_LR_6 \
   RESTORE_LR_0
 
diff --git a/sysdeps/unix/sysv/linux/arm/eabi/sysdep.h b/sysdeps/unix/sysv/linux/arm/eabi/sysdep.h
index b7815ba..a80621e 100644
--- a/sysdeps/unix/sysv/linux/arm/eabi/sysdep.h
+++ b/sysdeps/unix/sysv/linux/arm/eabi/sysdep.h
@@ -106,12 +106,95 @@
 #undef	DO_CALL
 #define DO_CALL(syscall_name, args)		\
     DOARGS_##args;				\
-    mov ip, r7;					\
-    cfi_register (r7, ip);			\
     ldr r7, =SYS_ify (syscall_name);		\
     swi 0x0;					\
-    mov r7, ip;					\
-    cfi_restore (r7);				\
     UNDOARGS_##args
 
+#undef  DOARGS_0
+#define DOARGS_0 \
+  .fnstart; \
+  str r7, [sp, #-4]!; \
+  cfi_adjust_cfa_offset (4); \
+  cfi_rel_offset (r7, 0); \
+  .save { r7 }
+#undef  DOARGS_1
+#define DOARGS_1 DOARGS_0
+#undef  DOARGS_2
+#define DOARGS_2 DOARGS_0
+#undef  DOARGS_3
+#define DOARGS_3 DOARGS_0
+#undef  DOARGS_4
+#define DOARGS_4 DOARGS_0
+#undef  DOARGS_5
+#define DOARGS_5 \
+  .fnstart; \
+  stmfd sp!, {r4, r7}; \
+  cfi_adjust_cfa_offset (8); \
+  cfi_rel_offset (r4, 0); \
+  cfi_rel_offset (r7, 4); \
+  .save { r4, r7 }; \
+  ldr r4, [sp, #8]
+#undef  DOARGS_6
+#define DOARGS_6 \
+  .fnstart; \
+  mov ip, sp; \
+  stmfd sp!, {r4, r5, r7}; \
+  cfi_adjust_cfa_offset (12); \
+  cfi_rel_offset (r4, 0); \
+  cfi_rel_offset (r5, 4); \
+  cfi_rel_offset (r7, 8); \
+  .save { r4, r5, r7 }; \
+  ldmia ip, {r4, r5}
+#undef  DOARGS_7
+#define DOARGS_7 \
+  .fnstart; \
+  mov ip, sp; \
+  stmfd sp!, {r4, r5, r6, r7}; \
+  cfi_adjust_cfa_offset (16); \
+  cfi_rel_offset (r4, 0); \
+  cfi_rel_offset (r5, 4); \
+  cfi_rel_offset (r6, 8); \
+  cfi_rel_offset (r7, 12); \
+  .save { r4, r5, r6, r7 }; \
+  ldmia ip, {r4, r5, r6}
+
+#undef  UNDOARGS_0
+#define UNDOARGS_0 \
+  ldr r7, [sp], #4; \
+  cfi_adjust_cfa_offset (-4); \
+  cfi_restore (r7); \
+  .fnend
+#undef  UNDOARGS_1
+#define UNDOARGS_1 UNDOARGS_0
+#undef  UNDOARGS_2
+#define UNDOARGS_2 UNDOARGS_0
+#undef  UNDOARGS_3
+#define UNDOARGS_3 UNDOARGS_0
+#undef  UNDOARGS_4
+#define UNDOARGS_4 UNDOARGS_0
+#undef  UNDOARGS_5
+#define UNDOARGS_5 \
+  ldmfd sp!, {r4, r7}; \
+  cfi_adjust_cfa_offset (-8); \
+  cfi_restore (r4); \
+  cfi_restore (r7); \
+  .fnend
+#undef  UNDOARGS_6
+#define UNDOARGS_6 \
+  ldmfd sp!, {r4, r5, r7}; \
+  cfi_adjust_cfa_offset (-12); \
+  cfi_restore (r4); \
+  cfi_restore (r5); \
+  cfi_restore (r7); \
+  .fnend
+#undef  UNDOARGS_7
+#define UNDOARGS_7 \
+  ldmfd sp!, {r4, r5, r6, r7}; \
+  cfi_adjust_cfa_offset (-16); \
+  cfi_restore (r4); \
+  cfi_restore (r5); \
+  cfi_restore (r6); \
+  cfi_restore (r7); \
+  .fnend
+
 #endif /* _LINUX_ARM_EABI_SYSDEP_H */
diff --git a/sysdeps/unix/sysv/linux/arm/vfork.S b/sysdeps/unix/sysv/linux/arm/vfork.S
index a020658..e63690e 100644
--- a/sysdeps/unix/sysv/linux/arm/vfork.S
+++ b/sysdeps/unix/sysv/linux/arm/vfork.S
@@ -33,7 +33,28 @@ ENTRY (__vfork)
 #ifdef SAVE_PID
 	SAVE_PID
 #endif
-	DO_CALL (vfork, 0)
+#ifdef __ARM_EABI__
+	/* The DO_CALL macro saves r7 on the stack, to enable generation
+	   of ARM unwind info.  Since the stack is initially shared between
+	   parent and child of vfork, that saved value could be corrupted.
+	   To avoid this problem, we save r7 into ip as well, and restore
+	   from there.  */
+	mov	ip, r7
+	cfi_register (r7, ip)
+	.fnstart
+	str r7, [sp, #-4]!
+	cfi_adjust_cfa_offset (4)
+	.save { r7 }
+	ldr	r7, =SYS_ify (vfork)
+	swi	0x0
+	.fnend
+	add	sp, sp, #4
+	cfi_adjust_cfa_offset (-4)
+	mov	r7, ip
+	cfi_restore (r7);
+#else
+	swi	SYS_ify(vfork)
+#endif
 #ifdef RESTORE_PID
 	RESTORE_PID
 #endif


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Daniel Jacobowitz]

On Sat, Mar 19, 2011 at 12:01:04AM +0100, Ulrich Weigand wrote:
> After some back and forth, I've been able to come up with a solution
> that works well enough for all GDB tests to pass.  It is not completely
> perfect since unwinding (on the parent side) out of an interrupted vfork
> call works only while the child has not yet clobbered the stack, but I
> don't see how to do anything better ...
> 
> Patch is appended below.  Does this look reasonable to you?

Yes, this does.

-- 
Daniel Jacobowitz
Mentor Graphics

Re: [rfc/rfa] Use ARM exception tables as GDB unwinder

[Ulrich Weigand]

Daniel Jacobowitz wrote:
> On Sat, Mar 19, 2011 at 12:01:04AM +0100, Ulrich Weigand wrote:
> > After some back and forth, I've been able to come up with a solution
> > that works well enough for all GDB tests to pass.  It is not completely
> > perfect since unwinding (on the parent side) out of an interrupted vfork
> > call works only while the child has not yet clobbered the stack, but I
> > don't see how to do anything better ...
> > 
> > Patch is appended below.  Does this look reasonable to you?
> 
> Yes, this does.

OK, thanks for the review!

I've now submitted the patches to the libc-ports mailing list ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com