[Libunwind-devel] [patch] Reduce Linux stack use.

[Paul Pluzhnikov]

Greetings,

Currently, libunwind allocates several PATH_MAX entries on stack, while
trying to find a binary via /proc/.../maps.

However stack space may be at premium (especially when sigaltstack is used),
and PATH_MAX on Linux is 4096, while SIGSTKSZ is only 8192 on x86.

Attached patch eliminates multiple PATH_MAX stack allocations, and simplifies
code in maps_next, at the cost of being unable to do anything if we can't
mmap one page. It appears to me that under such low-memory conditions,
libunwind will fail shortly elsewhere anyway.

This patch also disables more of debug_frame-handling code when
CONFIG_DEBUG_FRAME is undefined.

Tested on Linux/x86_64 with and without CONFIG_DEBUG_FRAME, no regressions.

Thanks,
--
Paul Pluzhnikov

commit f7ff774240ee84a37687910a0b5b4ec70745d6ef
Author: Paul Pluzhnikov <address@hidden>
Date:   Mon Oct 5 16:39:35 2009 -0700

    Reduce stack use, and make more of debug_frame conditional on
    CONFIG_DEBUG_FRAME.

diff --git a/src/dwarf/Gfind_proc_info-lsb.c b/src/dwarf/Gfind_proc_info-lsb.c
index 01d91f6..9484289 100644
--- a/src/dwarf/Gfind_proc_info-lsb.c
+++ b/src/dwarf/Gfind_proc_info-lsb.c
@@ -221,14 +221,6 @@ load_debug_frame (const char *file, char **buf, size_t 
*bufsize, int is_local)
 
   return 0;
 }
-#else
-static int
-load_debug_frame (const char *file, char **buf, size_t *bufsize, int is_local)
-{
-  return 1;
-}
-#endif /* CONFIG_DEBUG_FRAME */
-
 
 /* Locate the binary which originated the contents of address ADDR. Return
    the name of the binary in *name (space is allocated by the caller)
@@ -244,10 +236,16 @@ find_binary_for_address (unw_word_t ip, char *name, 
size_t name_size)
   unsigned long segbase, mapoff, hi;
 
   maps_init (&mi, pid);
-  while (maps_next (&mi, &segbase, &hi, &mapoff, name, name_size))
+  while (maps_next (&mi, &segbase, &hi, &mapoff))
     if (ip >= segbase && ip < hi)
       {
-       found = 1;
+       size_t len = strlen (mi.path);
+
+       if (len + 1 <= name_size)
+         {
+           memcpy (name, mi.path, len + 1);
+           found = 1;
+         }
        break;
       }
   maps_close (&mi);
@@ -400,6 +398,8 @@ debug_frame_tab_compare (const void *a, const void *b)
     return 0;
 }
 
+#endif /* CONFIG_DEBUG_FRAME */
+
 /* ptr is a pointer to a callback_data structure and, on entry,
    member ip contains the instruction-pointer we're looking
    for.  */
@@ -575,6 +575,8 @@ callback (struct dl_phdr_info *info, size_t size, void *ptr)
        }
     }
 
+#ifdef CONFIG_DEBUG_FRAME
+
   Debug (15, "Trying to find .debug_frame\n");
   di = &cb_data->di_debug;
   fdesc = locate_debug_info (unw_local_addr_space, info, ip, info->dlpi_name);
@@ -707,6 +709,7 @@ callback (struct dl_phdr_info *info, size_t size, void *ptr)
             (long) di->u.ti.segbase, (long) di->u.ti.table_len,
             (long) di->gp, (long) di->u.ti.table_data);
     }
+#endif  /* CONFIG_DEBUG_FRAME */
 
   return found;
 }
diff --git a/src/os-linux.c b/src/os-linux.c
index ae37d8f..210edd0 100644
--- a/src/os-linux.c
+++ b/src/os-linux.c
@@ -36,23 +36,28 @@ tdep_get_elf_image (struct elf_image *ei, pid_t pid, 
unw_word_t ip,
                    unsigned long *segbase, unsigned long *mapoff)
 {
   struct map_iterator mi;
-  char path[PATH_MAX];
-  int found = 0;
+  int found = 0, rc;
   unsigned long hi;
 
-  maps_init (&mi, pid);
-  while (maps_next (&mi, segbase, &hi, mapoff, path, sizeof (path)))
+  if (maps_init (&mi, pid) < 0)
+    return -1;
+
+  while (maps_next (&mi, segbase, &hi, mapoff))
     if (ip >= *segbase && ip < hi)
       {
        found = 1;
        break;
       }
-  maps_close (&mi);
 
   if (!found)
-    return -1;
+    {
+      maps_close (&mi);
+      return -1;
+    }
 
-  return elf_map_image (ei, path);
+  rc = elf_map_image (ei, mi.path);
+  maps_close (&mi);
+  return rc;
 }
 
 #endif /* UNW_REMOTE_ONLY */
diff --git a/src/os-linux.h b/src/os-linux.h
index b2c9a8b..c444922 100644
--- a/src/os-linux.h
+++ b/src/os-linux.h
@@ -34,6 +34,7 @@ struct map_iterator
     size_t buf_size;
     char *buf;
     char *buf_end;
+    char *path;
   };
 
 static inline char *
@@ -61,33 +62,33 @@ ltoa (char *buf, long val)
   return buf + len;
 }
 
-static inline void
+static inline int
 maps_init (struct map_iterator *mi, pid_t pid)
 {
-  char path[PATH_MAX], *cp;
+  char path[sizeof ("/proc/01234567879/maps")], *cp;
 
   memcpy (path, "/proc/", 6);
   cp = ltoa (path + 6, pid);
+  assert (cp + 6 < path + sizeof (path));
   memcpy (cp, "/maps", 6);
 
   mi->fd = open (path, O_RDONLY);
-  mi->offset = 0;
-  mi->buf_size = 0;
-
-  cp = NULL;
   if (mi->fd >= 0)
     {
-      /* Try to allocate a page-sized buffer.  If that fails, we'll
-        fall back on reading one line at a time.  */
+      /* Try to allocate a page-sized buffer.  */
       mi->buf_size = getpagesize ();
       cp = mmap (0, mi->buf_size, PROT_READ | PROT_WRITE,
                 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
       if (cp == MAP_FAILED)
-       cp = NULL;
+       return -1;
       else
-       cp += mi->buf_size;
+       {
+         mi->offset = 0;
+         mi->buf = mi->buf_end = cp + mi->buf_size;
+         return 0;
+       }
     }
-  mi->buf = mi->buf_end = cp;
+  return -1;
 }
 
 static inline char *
@@ -184,7 +185,7 @@ scan_string (char *cp, char *valp, size_t buf_size)
 
   while (*cp != ' ' && *cp != '\t' && *cp != '\0')
     {
-      if (i < buf_size - 1)
+      if (valp != NULL && i < buf_size - 1)
        valp[i++] = *cp;
       ++cp;
     }
@@ -196,10 +197,9 @@ scan_string (char *cp, char *valp, size_t buf_size)
 
 static inline int
 maps_next (struct map_iterator *mi,
-          unsigned long *low, unsigned long *high, unsigned long *offset,
-          char *path, size_t path_size)
+          unsigned long *low, unsigned long *high, unsigned long *offset)
 {
-  char line[256 + PATH_MAX], perm[16], dash, colon, *cp;
+  char perm[16], dash, colon, *cp;
   unsigned long major, minor, inum;
   size_t to_read = 256;        /* most lines fit in 256 characters easy */
   ssize_t i, nread;
@@ -209,80 +209,52 @@ maps_next (struct map_iterator *mi,
 
   while (1)
     {
-      if (mi->buf)
-       {
-         ssize_t bytes_left = mi->buf_end - mi->buf;
-         char *eol = NULL;
+      ssize_t bytes_left = mi->buf_end - mi->buf;
+      char *eol = NULL;
 
-         for (i = 0; i < bytes_left; ++i)
-           {
-             if (mi->buf[i] == '\n')
-               {
-                 eol = mi->buf + i;
-                 break;
-               }
-             else if (mi->buf[i] == '\0')
-               break;
-           }
-         if (!eol)
+      for (i = 0; i < bytes_left; ++i)
+       {
+         if (mi->buf[i] == '\n')
            {
-             /* copy down the remaining bytes, if any */
-             if (bytes_left > 0)
-               memmove (mi->buf_end - mi->buf_size, mi->buf, bytes_left);
-
-             mi->buf = mi->buf_end - mi->buf_size;
-             nread = read (mi->fd, mi->buf + bytes_left,
-                           mi->buf_size - bytes_left);
-             if (nread <= 0)
-               return 0;
-             else if ((size_t) (nread + bytes_left) < mi->buf_size)
-               {
-                 /* Move contents to the end of the buffer so we
-                    maintain the invariant that all bytes between
-                    mi->buf and mi->buf_end are valid.  */
-                 memmove (mi->buf_end - nread - bytes_left, mi->buf,
-                         nread + bytes_left);
-                 mi->buf = mi->buf_end - nread - bytes_left;
-               }
-
-             eol = mi->buf + bytes_left + nread - 1;
-
-             for (i = bytes_left; i < bytes_left + nread; ++i)
-               if (mi->buf[i] == '\n')
-                 {
-                   eol = mi->buf + i;
-                   break;
-                 }
+             eol = mi->buf + i;
+             break;
            }
-         cp = mi->buf;
-         mi->buf = eol + 1;
-         *eol = '\0';
+         else if (mi->buf[i] == '\0')
+           break;
        }
-      else
+      if (!eol)
        {
-         /* maps_init() wasn't able to allocate a buffer; do it the
-            slow way.  */
-         lseek (mi->fd, mi->offset, SEEK_SET);
-
-         if ((nread = read (mi->fd, line, to_read)) <= 0)
+         /* copy down the remaining bytes, if any */
+         if (bytes_left > 0)
+           memmove (mi->buf_end - mi->buf_size, mi->buf, bytes_left);
+
+         mi->buf = mi->buf_end - mi->buf_size;
+         nread = read (mi->fd, mi->buf + bytes_left,
+                       mi->buf_size - bytes_left);
+         if (nread <= 0)
            return 0;
-         for (i = 0; i < nread && line[i] != '\n'; ++i)
-           /* skip */;
-         if (i < nread)
-           {
-             line[i] = '\0';
-             mi->offset += i + 1;
-           }
-         else
+         else if ((size_t) (nread + bytes_left) < mi->buf_size)
            {
-             if (to_read < sizeof (line))
-               to_read = sizeof (line) - 1;
-             else
-               mi->offset += nread;    /* not supposed to happen... */
-             continue; /* duh, no newline found */
+             /* Move contents to the end of the buffer so we
+                maintain the invariant that all bytes between
+                mi->buf and mi->buf_end are valid.  */
+             memmove (mi->buf_end - nread - bytes_left, mi->buf,
+                      nread + bytes_left);
+             mi->buf = mi->buf_end - nread - bytes_left;
            }
-         cp = line;
+
+         eol = mi->buf + bytes_left + nread - 1;
+
+         for (i = bytes_left; i < bytes_left + nread; ++i)
+           if (mi->buf[i] == '\n')
+             {
+               eol = mi->buf + i;
+               break;
+             }
        }
+      cp = mi->buf;
+      mi->buf = eol + 1;
+      *eol = '\0';
 
       /* scan: "LOW-HIGH PERM OFFSET MAJOR:MINOR INUM PATH" */
       cp = scan_hex (cp, low);
@@ -294,7 +266,10 @@ maps_next (struct map_iterator *mi,
       cp = scan_char (cp, &colon);
       cp = scan_hex (cp, &minor);
       cp = scan_dec (cp, &inum);
-      cp = scan_string (cp, path, path_size);
+      cp = mi->path = skip_whitespace (cp);
+      if (!cp)
+       continue;
+      cp = scan_string (cp, NULL, 0);
       if (!cp || dash != '-' || colon != ':')
        continue;       /* skip line with unknown or bad format */
       return 1;
diff --git a/tests/Gtest-bt.c b/tests/Gtest-bt.c
index 8211f73..9b07ec4 100644
--- a/tests/Gtest-bt.c
+++ b/tests/Gtest-bt.c
@@ -38,12 +38,6 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE 
SOFTWARE.  */
 #include <unistd.h>
 #include <libunwind.h>
 
-#if UNW_TARGET_X86 || UNW_TARGET_X86_64 || UNW_TARGET_ARM
-# define STACK_SIZE    (128*1024)      /* On x86/-64 and ARM, SIGSTKSZ is too 
small */
-#else
-# define STACK_SIZE    SIGSTKSZ
-#endif
-
 #define panic(args...)                         \
        { fprintf (stderr, args); exit (-1); }
 
@@ -219,10 +213,10 @@ main (int argc, char **argv)
 
   if (verbose)
     printf ("\nBacktrace across signal handler on alternate stack:\n");
-  stk.ss_sp = malloc (STACK_SIZE);
+  stk.ss_sp = malloc (SIGSTKSZ);
   if (!stk.ss_sp)
     panic ("failed to allocate SIGSTKSZ (%u) bytes\n", SIGSTKSZ);
-  stk.ss_size = STACK_SIZE;
+  stk.ss_size = SIGSTKSZ;
   stk.ss_flags = 0;
   if (sigaltstack (&stk, NULL) < 0)
     panic ("sigaltstack: %s\n", strerror (errno));