From patchwork Sun May 15 11:12:58 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Janne Blomqvist X-Patchwork-Id: 95613 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 039ECB6EF7 for ; Sun, 15 May 2011 21:13:23 +1000 (EST) Received: (qmail 24539 invoked by alias); 15 May 2011 11:13:18 -0000 Received: (qmail 24522 invoked by uid 22791); 15 May 2011 11:13:15 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, RFC_ABUSE_POST, TW_BG, TW_PW, TW_RG, TW_TP, TW_WU X-Spam-Check-By: sourceware.org Received: from mail-pz0-f47.google.com (HELO mail-pz0-f47.google.com) (209.85.210.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 15 May 2011 11:12:59 +0000 Received: by pzk36 with SMTP id 36so1984334pzk.20 for ; Sun, 15 May 2011 04:12:58 -0700 (PDT) MIME-Version: 1.0 Received: by 10.68.15.170 with SMTP id y10mr5442134pbc.364.1305457978485; Sun, 15 May 2011 04:12:58 -0700 (PDT) Received: by 10.68.54.105 with HTTP; Sun, 15 May 2011 04:12:58 -0700 (PDT) In-Reply-To: References: Date: Sun, 15 May 2011 14:12:58 +0300 Message-ID: Subject: Re: [Patch, libfortran] PR 48931 Async-signal-safety of backtrace signal handler From: Janne Blomqvist To: GCC Patches , Fortran List Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Hi, so, here is take 3 (sigh). Compared to take 2, it no longer uses stdio, since opening a stdio FILE stream probably malloc()'s a buffer, which is not async-signal-safe. Regtested on x86_64-unknown-linux-gnu, Ok for trunk? 2011-05-15 Janne Blomqvist PR libfortran/48931 * configure.ac: Check for backtrace_symbols_fd instead of backtrace_symbols, check for readlink. * config.h.in: Regenerated. * configure: Regenerated. * runtime/backtrace.c (local_strcasestr): Remove. (bt_header): New function. (dump_glibc_backtrace): Remove. (fd_gets): New function. (show_backtrace): Rework to use backtrace_symbols_fd and pipes, reformat output. * runtime/main.c (store_exe_path): Try to check /proc/self/exe first. diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac index cf38fb0..74cfe44 100644 --- a/libgfortran/configure.ac +++ b/libgfortran/configure.ac @@ -264,10 +264,10 @@ AC_CHECK_FUNCS(sleep time ttyname signal alarm clock access fork execl) AC_CHECK_FUNCS(wait setmode execvp pipe dup2 close fdopen strcasestr getrlimit) AC_CHECK_FUNCS(gettimeofday stat fstat lstat getpwuid vsnprintf dup getcwd) AC_CHECK_FUNCS(localtime_r gmtime_r strerror_r getpwuid_r ttyname_r) -AC_CHECK_FUNCS(clock_gettime strftime) +AC_CHECK_FUNCS(clock_gettime strftime readlink) # Check for glibc backtrace functions -AC_CHECK_FUNCS(backtrace backtrace_symbols) +AC_CHECK_FUNCS(backtrace backtrace_symbols_fd) # Check libc for getgid, getpid, getuid AC_CHECK_LIB([c],[getgid],[AC_DEFINE([HAVE_GETGID],[1],[libc includes getgid])]) diff --git a/libgfortran/runtime/backtrace.c b/libgfortran/runtime/backtrace.c index 10917d3..04246a9 100644 --- a/libgfortran/runtime/backtrace.c +++ b/libgfortran/runtime/backtrace.c @@ -54,59 +54,57 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #define CAN_FORK (defined(HAVE_FORK) && defined(HAVE_EXECVP) \ && defined(HAVE_WAIT)) #define GLIBC_BACKTRACE (defined(HAVE_BACKTRACE) \ - && defined(HAVE_BACKTRACE_SYMBOLS)) + && defined(HAVE_BACKTRACE_SYMBOLS_FD)) #define CAN_PIPE (CAN_FORK && defined(HAVE_PIPE) \ && defined(HAVE_DUP2) && defined(HAVE_FDOPEN) \ && defined(HAVE_CLOSE)) -#if GLIBC_BACKTRACE && CAN_PIPE -static char * -local_strcasestr (const char *s1, const char *s2) -{ -#ifdef HAVE_STRCASESTR - return strcasestr (s1, s2); -#else +/* GDB style #NUM index for each stack frame. */ - const char *p = s1; - const size_t len = strlen (s2); - const char u = *s2, v = isupper((int) *s2) ? tolower((int) *s2) - : (islower((int) *s2) ? toupper((int) *s2) - : *s2); - - while (1) - { - while (*p != u && *p != v && *p) - p++; - if (*p == 0) - return NULL; - if (strncasecmp (p, s2, len) == 0) - return (char *)p; - } -#endif +static void +bt_header (int num) +{ + st_printf (" #%d ", num); } -#endif -#if GLIBC_BACKTRACE -static void -dump_glibc_backtrace (int depth, char *str[]) -{ - int i; +/* fgets()-like function that reads a line from a fd, without + needing to malloc() a buffer, and does not use locks, hence should + be async-signal-safe. */ - for (i = 0; i < depth; i++) +static char * +fd_gets (char *s, int size, int fd) +{ + for (int i = 0; i < size; i++) { - estr_write (" + "); - estr_write (str[i]); - estr_write ("\n"); + char c; + ssize_t nread = read (fd, &c, 1); + if (nread == 1) + { + s[i] = c; + if (c == '\n') + { + if (i + 1 < size) + s[i+1] = '\0'; + else + s[i] = '\0'; + break; + } + } + else + { + s[i] = '\0'; + break; + } } - - free (str); + return s; } -#endif + /* show_backtrace displays the backtrace, currently obtained by means of the glibc backtrace* functions. */ + void show_backtrace (void) { @@ -116,176 +114,184 @@ show_backtrace (void) #define BUFSIZE 1024 void *trace[DEPTH]; - char **str; int depth; depth = backtrace (trace, DEPTH); if (depth <= 0) return; - str = backtrace_symbols (trace, depth); - #if CAN_PIPE -#ifndef STDIN_FILENO -#define STDIN_FILENO 0 -#endif - -#ifndef STDOUT_FILENO -#define STDOUT_FILENO 1 -#endif - -#ifndef STDERR_FILENO -#define STDERR_FILENO 2 -#endif - /* We attempt to extract file and line information from addr2line. */ do { /* Local variables. */ - int f[2], pid, line, i; - FILE *output; - char addr_buf[DEPTH][GFC_XTOA_BUF_SIZE], func[BUFSIZE], file[BUFSIZE]; + int f[2], pid, line, bt[2], inp[2]; + char addr_buf[GFC_XTOA_BUF_SIZE], func[BUFSIZE], file[BUFSIZE]; char *p, *end; - const char *addr[DEPTH]; - /* Write the list of addresses in hexadecimal format. */ - for (i = 0; i < depth; i++) - addr[i] = gfc_xtoa ((GFC_UINTEGER_LARGEST) (intptr_t) trace[i], addr_buf[i], - sizeof (addr_buf[i])); + char *exe_path = full_exe_path (); + if (exe_path == NULL) + exe_path = (char *) "a.out"; /* Don't output an error message if something goes wrong, we'll simply fall back to the pstack and glibc backtraces. */ if (pipe (f) != 0) break; + if (pipe (inp) != 0) + break; if ((pid = fork ()) == -1) break; if (pid == 0) { /* Child process. */ -#define NUM_FIXEDARGS 5 - char *arg[DEPTH+NUM_FIXEDARGS+1]; +#define NUM_FIXEDARGS 7 + char *arg[NUM_FIXEDARGS]; close (f[0]); - close (STDIN_FILENO); + + close (inp[1]); + if (dup2 (inp[0], STDIN_FILENO) == -1) + _exit (1); + close (inp[0]); + close (STDERR_FILENO); if (dup2 (f[1], STDOUT_FILENO) == -1) - _exit (0); + _exit (1); close (f[1]); arg[0] = (char *) "addr2line"; arg[1] = (char *) "-e"; - arg[2] = full_exe_path (); + arg[2] = exe_path; arg[3] = (char *) "-f"; arg[4] = (char *) "-s"; - for (i = 0; i < depth; i++) - arg[NUM_FIXEDARGS+i] = (char *) addr[i]; - arg[NUM_FIXEDARGS+depth] = NULL; + arg[5] = (char *) "-C"; + arg[6] = NULL; execvp (arg[0], arg); - _exit (0); + _exit (1); #undef NUM_FIXEDARGS } /* Father process. */ close (f[1]); - wait (NULL); - output = fdopen (f[0], "r"); - i = -1; + close (inp[0]); + if (pipe (bt) != 0) + break; + backtrace_symbols_fd (trace, depth, bt[1]); + close (bt[1]); - if (fgets (func, sizeof(func), output)) + estr_write ("\nBacktrace for this error:\n"); + for (int j = 0; j < depth; j++) { - estr_write ("\nBacktrace for this error:\n"); - - do + const char *addr = gfc_xtoa + ((GFC_UINTEGER_LARGEST) (intptr_t) trace[j], + addr_buf, sizeof (addr_buf)); + + write (inp[1], addr, strlen (addr)); + write (inp[1], "\n", 1); + + if (! fd_gets (func, sizeof(func), f[0])) + goto fallback; + if (! fd_gets (file, sizeof(file), f[0])) + goto fallback; + + for (p = func; *p != '\n' && *p != '\r'; p++) + ; + *p = '\0'; + + /* If we only have the address, use the glibc backtrace. */ + if (func[0] == '?' && func[1] == '?' && file[0] == '?' + && file[1] == '?') { - if (! fgets (file, sizeof(file), output)) - goto fallback; - - i++; - - for (p = func; *p != '\n' && *p != '\r'; p++) - ; - - *p = '\0'; - - /* Try to recognize the internal libgfortran functions. */ - if (strncasecmp (func, "*_gfortran", 10) == 0 - || strncasecmp (func, "_gfortran", 9) == 0 - || strcmp (func, "main") == 0 || strcmp (func, "_start") == 0 - || strcmp (func, "_gfortrani_backtrace_handler") == 0) - continue; - - if (local_strcasestr (str[i], "libgfortran.so") != NULL - || local_strcasestr (str[i], "libgfortran.dylib") != NULL - || local_strcasestr (str[i], "libgfortran.a") != NULL) - continue; - - /* If we only have the address, use the glibc backtrace. */ - if (func[0] == '?' && func[1] == '?' && file[0] == '?' - && file[1] == '?') + bt_header (j); + while (1) { - estr_write (" + "); - estr_write (str[i]); - estr_write ("\n"); - continue; + char bc; + ssize_t nread = read (bt[0], &bc, 1); + if (nread != 1 || bc == '\n') + break; + write (STDERR_FILENO, &bc, 1); } - - /* Extract the line number. */ - for (end = NULL, p = file; *p; p++) - if (*p == ':') - end = p; - if (end != NULL) - { - *end = '\0'; - line = atoi (++end); - } - else - line = -1; - - if (strcmp (func, "MAIN__") == 0) - estr_write (" + in the main program\n"); - else + estr_write ("\n"); + continue; + } + else + { + /* Forward to the next entry in the backtrace. */ + while (1) { - estr_write (" + function "); - estr_write (func); - estr_write (" (0x"); - estr_write (addr[i]); - estr_write (")\n"); + char bc; + ssize_t nread = read (bt[0], &bc, 1); + if (nread != 1 || bc == '\n') + break; } + } - if (line <= 0 && strcmp (file, "??") == 0) - continue; + /* _start is a setup routine that calls main(), and main() is + the frontend routine that calls some setup stuff and then + calls MAIN__, so at this point we should stop. */ + if (strcmp (func, "_start") == 0 || strcmp (func, "main") == 0) + break; + + /* Extract the line number. */ + for (end = NULL, p = file; *p; p++) + if (*p == ':') + end = p; + if (end != NULL) + { + *end = '\0'; + line = atoi (++end); + } + else + line = -1; + + bt_header (j); + estr_write (exe_path); + estr_write ("[0x"); + estr_write (addr); + estr_write ("] in "); + estr_write (func); + + if (line <= 0 && strcmp (file, "??") == 0) + { + estr_write ("\n"); + continue; + } - if (line <= 0) - { - estr_write (" from file "); - estr_write (file); - estr_write ("\n"); - } - else - st_printf (" at line %d of file %s\n", line, file); + if (line <= 0) + { + estr_write (" at "); + estr_write (file); + estr_write ("\n"); } - while (fgets (func, sizeof(func), output)); + else + st_printf (" at %s:%d\n", file, line); - free (str); - return; + } /* Loop over each hex address. */ + close (inp[1]); + close (bt[0]); + wait (NULL); + return; fallback: - estr_write ("** Something went wrong while running addr2line. **\n" - "** Falling back to a simpler backtrace scheme. **\n"); - } - } + estr_write ("** Something went wrong while running addr2line. **\n" + "** Falling back to a simpler backtrace scheme. **\n"); + } while (0); #undef DEPTH #undef BUFSIZE -#endif -#endif +#endif /* CAN_PIPE */ + + /* Fallback to the glibc backtrace. */ + estr_write ("\nBacktrace for this error:\n"); + backtrace_symbols_fd (trace, depth, STDERR_FILENO); + return; -#if CAN_FORK && defined(HAVE_GETPPID) +#elif defined(CAN_FORK) && defined(HAVE_GETPPID) /* Try to call pstack. */ do { @@ -312,15 +318,9 @@ fallback: execvp (arg[0], arg); #undef NUM_ARGS - /* pstack didn't work, so we fall back to dumping the glibc - backtrace if we can. */ -#if GLIBC_BACKTRACE - dump_glibc_backtrace (depth, str); -#else + /* pstack didn't work. */ estr_write (" unable to produce a backtrace, sorry!\n"); -#endif - - _exit (0); + _exit (1); } /* Father process. */ @@ -328,13 +328,7 @@ fallback: return; } while(0); -#endif - -#if GLIBC_BACKTRACE - /* Fallback to the glibc backtrace. */ - estr_write ("\nBacktrace for this error:\n"); - dump_glibc_backtrace (depth, str); - return; -#endif +#else estr_write ("\nBacktrace not yet available on this platform, sorry!\n"); +#endif } diff --git a/libgfortran/runtime/main.c b/libgfortran/runtime/main.c index f5d4721..54d9e09 100644 --- a/libgfortran/runtime/main.c +++ b/libgfortran/runtime/main.c @@ -92,6 +92,19 @@ store_exe_path (const char * argv0) if (please_free_exe_path_when_done) free ((char *) exe_path); + /* Reading the /proc/self/exe symlink is Linux-specific(?), but if + it works it gives the correct answer. */ +#ifdef HAVE_READLINK + int len; + if ((len = readlink ("/proc/self/exe", buf, sizeof (buf) - 1)) != -1) + { + buf[len] = '\0'; + exe_path = strdup (buf); + please_free_exe_path_when_done = 1; + return; + } +#endif + /* On the simulator argv is not set. */ if (argv0 == NULL || argv0[0] == '/') { @@ -107,7 +120,9 @@ store_exe_path (const char * argv0) cwd = ""; #endif - /* exe_path will be cwd + "/" + argv[0] + "\0" */ + /* exe_path will be cwd + "/" + argv[0] + "\0". This will not work + if the executable is not in the cwd, but at this point we're out + of better ideas. */ size_t pathlen = strlen (cwd) + 1 + strlen (argv0) + 1; path = malloc (pathlen); snprintf (path, pathlen, "%s%c%s", cwd, DIR_SEPARATOR, argv0);