From patchwork Fri Jan 21 17:24:56 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Janne Blomqvist X-Patchwork-Id: 79871 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 81BD9B7157 for ; Sat, 22 Jan 2011 04:25:19 +1100 (EST) Received: (qmail 2731 invoked by alias); 21 Jan 2011 17:25:17 -0000 Received: (qmail 2709 invoked by uid 22791); 21 Jan 2011 17:25:15 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, TW_CP, TW_PW, TW_TP, TW_WU X-Spam-Check-By: sourceware.org Received: from mail-iy0-f175.google.com (HELO mail-iy0-f175.google.com) (209.85.210.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 21 Jan 2011 17:25:00 +0000 Received: by iyj18 with SMTP id 18so1972200iyj.20 for ; Fri, 21 Jan 2011 09:24:58 -0800 (PST) MIME-Version: 1.0 Received: by 10.231.199.76 with SMTP id er12mr1068696ibb.72.1295630696923; Fri, 21 Jan 2011 09:24:56 -0800 (PST) Received: by 10.231.160.68 with HTTP; Fri, 21 Jan 2011 09:24:56 -0800 (PST) Date: Fri, 21 Jan 2011 19:24:56 +0200 Message-ID: Subject: [Patch, libfortran] PR46267 strerror() might not be thread-safe, take 2 From: Janne Blomqvist To: Fortran List , GCC Patches 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, POSIX does not require strerror() to be thread-safe (although some implementations are), using it is thus a potential error in libgfortran. The attached patch fixes this in the case that strerror_r() is available; thanks to Jakub for the hint about __builtin_classify_type. Compared to the previous version of the patch at http://gcc.gnu.org/ml/gcc-patches/2011-01/msg01400.html this version does not use TLS, instead the API of gf_strerror() is now similar to GNU strerror_r() and callers have been updated to supply a buffer (that might, or might not, be used). Regtested on x86_64-unknown-linux-gnu, Ok for trunk? diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac index 7b28f12..4f137e4 100644 --- a/libgfortran/configure.ac +++ b/libgfortran/configure.ac @@ -249,7 +249,7 @@ AC_CHECK_FUNCS(chdir strerror getlogin gethostname kill link symlink perror) AC_CHECK_FUNCS(sleep time ttyname signal alarm ctime 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) +AC_CHECK_FUNCS(localtime_r gmtime_r strerror_r) # Check for glibc backtrace functions AC_CHECK_FUNCS(backtrace backtrace_symbols) diff --git a/libgfortran/intrinsics/gerror.c b/libgfortran/intrinsics/gerror.c index ccb5c3e..6feadc9 100644 --- a/libgfortran/intrinsics/gerror.c +++ b/libgfortran/intrinsics/gerror.c @@ -43,16 +43,17 @@ PREFIX(gerror) (char * msg, gfc_charlen_type msg_len) int p_len; char *p; - memset (msg, ' ', msg_len); /* Blank the string. */ - - p = strerror (errno); - if (p == NULL) - return; - + p = gf_strerror (errno, msg, msg_len); p_len = strlen (p); - if (msg_len < p_len) - memcpy (msg, p, msg_len); - else - memcpy (msg, p, p_len); + /* The returned pointer p might or might not be the same as the msg + argument. */ + if (p != msg) + { + if (msg_len < p_len) + p_len = msg_len; + memcpy (msg, p, p_len); + } + if (msg_len > p_len) + memset (&msg[p_len], ' ', msg_len - p_len); } #endif diff --git a/libgfortran/io/unix.c b/libgfortran/io/unix.c index fa64e20..950b7a2 100644 --- a/libgfortran/io/unix.c +++ b/libgfortran/io/unix.c @@ -256,16 +256,6 @@ flush_if_preconnected (stream * s) } -/* get_oserror()-- Get the most recent operating system error. For - * unix, this is errno. */ - -const char * -get_oserror (void) -{ - return strerror (errno); -} - - /******************************************************************** Raw I/O functions (read, write, seek, tell, truncate, close). diff --git a/libgfortran/libgfortran.h b/libgfortran/libgfortran.h index ac86492..c9d3f37 100644 --- a/libgfortran/libgfortran.h +++ b/libgfortran/libgfortran.h @@ -1,5 +1,6 @@ /* Common declarations for all of libgfortran. - Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010 + Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, + 2011 Free Software Foundation, Inc. Contributed by Paul Brook , and Andy Vaught @@ -738,9 +739,6 @@ extern void internal_error (st_parameter_common *, const char *) __attribute__ ((noreturn)); internal_proto(internal_error); -extern const char *get_oserror (void); -internal_proto(get_oserror); - extern const char *translate_error (int); internal_proto(translate_error); @@ -756,6 +754,9 @@ internal_proto(notify_std); extern notification notification_std(int); internal_proto(notification_std); +extern char *gf_strerror (int, char *, size_t); +internal_proto(gf_strerror); + /* fpu.c */ extern void set_fpu (void); diff --git a/libgfortran/runtime/error.c b/libgfortran/runtime/error.c index 1baf9d3..754a031 100644 --- a/libgfortran/runtime/error.c +++ b/libgfortran/runtime/error.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2002, 2003, 2005, 2006, 2007, 2009, 2010 +/* Copyright (C) 2002, 2003, 2005, 2006, 2007, 2009, 2010, 2011 Free Software Foundation, Inc. Contributed by Andy Vaught @@ -141,6 +141,38 @@ gfc_xtoa (GFC_UINTEGER_LARGEST n, char *buffer, size_t len) return p; } + +/* Hopefully thread-safe wrapper for a strerror() style function. */ + +char * +gf_strerror (int errnum, +#ifdef HAVE_STRERROR_R + char * buf, size_t buflen) +{ + /* TODO: How to prevent the compiler warning due to strerror_r of + the untaken branch having the wrong return type? */ + if (__builtin_classify_type (strerror_r (0, buf, 0)) == 5) + { + /* GNU strerror_r() */ + return strerror_r (errnum, buf, buflen); + } + else + { + /* POSIX strerror_r () */ + strerror_r (errnum, buf, buflen); + return buf; + } +#else + char * buf __attribute__((unused)), + size_t buflen __attribute__((unused))) +{ + /* strerror () is not necessarily thread-safe, but should at least + be available everywhere. */ + return strerror (errnum); +#endif +} + + /* show_locus()-- Print a line number and filename describing where * something went wrong */ @@ -192,6 +224,8 @@ recursion_check (void) } +#define STRERR_MAXSZ 256 + /* os_error()-- Operating system error. We get a message from the * operating system, show it and leave. Some operating system errors * are caught and processed by the library. If not, we come here. */ @@ -199,8 +233,10 @@ recursion_check (void) void os_error (const char *message) { + char errmsg[STRERR_MAXSZ]; recursion_check (); - st_printf ("Operating system error: %s\n%s\n", get_oserror (), message); + st_printf ("Operating system error: %s\n%s\n", + gf_strerror (errno, errmsg, STRERR_MAXSZ), message); sys_exit (1); } iexport(os_error); @@ -389,6 +425,7 @@ translate_error (int code) void generate_error (st_parameter_common *cmp, int family, const char *message) { + char errmsg[STRERR_MAXSZ]; /* If there was a previous error, don't mask it with another error message, EOF or EOR condition. */ @@ -402,7 +439,8 @@ generate_error (st_parameter_common *cmp, int family, const char *message) if (message == NULL) message = - (family == LIBERROR_OS) ? get_oserror () : translate_error (family); + (family == LIBERROR_OS) ? gf_strerror (errno, errmsg, STRERR_MAXSZ) : + translate_error (family); if (cmp->flags & IOPARM_HAS_IOMSG) cf_strcpy (cmp->iomsg, cmp->iomsg_len, message);