Re: First patch attempt for PATH_MAX fix in ruby1.9.1- 1.9.3~rc1-3

[Pino Toscano]

Hi,

Alle giovedì 13 ottobre 2011, Svante Signell ha scritto:
 Below is a patch attempt to solve the PATH_MAX issue on the latest
 ruby1.9.1.

I don't think this is correct: various functions in addr2line.c seem to 
assume the buffer is always the same, and they call themselves 
recursively (e.g. rb_dump_backtrace_with_lines (sets binary_filename) - 
fill_lines - follow_debuglink (sets binary_filename) - fill_lines - 
...). With your patch, it would also cause the binary_filename pointer 
to be stale over runs.

-- 
Pino Toscano


signature.asc
Description: This is a digitally signed message part.

First patch attempt for PATH_MAX fix in ruby1.9.1- 1.9.3~rc1-3

[Svante Signell]

Hi,

Below is a patch attempt to solve the PATH_MAX issue on the latest
ruby1.9.1.

When applied ruby builds with tests disabled. Please let me know if
there are remaining +-1 issues and if malloc/free (heap) should be
preferred before alloca (stack).

Thanks!
diff -u ruby1.9.1-1.9.3~rc1/addr2line.c ruby1.9.1-1.9.3~rc1.modified/addr2line.c
--- ruby1.9.1-1.9.3~rc1/addr2line.c	2011-07-08 14:23:10.0 +0200
+++ ruby1.9.1-1.9.3~rc1.modified/addr2line.c	2011-10-13 22:20:48.0 +0200
@@ -81,7 +81,7 @@
 } line_info_t;
 
 /* Avoid consuming stack as this module may be used from signal handler */
-static char binary_filename[PATH_MAX];
+static char *binary_filename = NULL;
 
 static unsigned long
 uleb128(char **p) {
@@ -407,6 +407,7 @@
by some linux distributions. See GDB's info for detail. */
 static const char global_debug_dir[] = /usr/lib/debug;
 char *p, *subdir;
+size_t len;
 
 p = strrchr(binary_filename, '/');
 if (!p) {
@@ -416,11 +417,12 @@
 
 subdir = (char *)alloca(strlen(binary_filename) + 1);
 strcpy(subdir, binary_filename);
+len = strlen(binary_filename) + strlen(subdir) + strlen(debuglink) + 1;
+binary_filename = (char*)alloca(len);
 strcpy(binary_filename, global_debug_dir);
-strncat(binary_filename, subdir,
-	PATH_MAX - strlen(binary_filename) - 1);
-strncat(binary_filename, debuglink,
-	PATH_MAX - strlen(binary_filename) - 1);
+strncat(binary_filename, subdir, strlen(subdir));
+strncat(binary_filename, debuglink, strlen(debuglink));
+binary_filename[len] = '\0';
 
 munmap(current_line-mapped, current_line-mapped_size);
 close(current_line-fd);
@@ -569,6 +571,7 @@
 	continue;
 	}
 
+	binary_filename = (char *)(alloca(len) + 1);
 	strncpy(binary_filename, path, len);
 	binary_filename[len] = '\0';