[Patch, libfortran] Thread safety and simplification of error printing

[Janne Blomqvist]

On Sun, May 8, 2011 at 14:40, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
> On Sat, May 7, 2011 at 19:35, Janne Blomqvist <blomqvist.janne@gmail.com> wrote:
>> Hi,
>>
>> the error printing functionality (in io/unix.c) st_printf and
>> st_vprintf are not thread-safe as they use a static buffer. However,
>> since these routines are used when something has gone wrong, we
>> shouldn't use malloc() to allocate thread-specific buffers or anything
>> fancy like that. The way the buffer is used is that it is filled with
>> vs(n)printf() and then the buffer is written using write(); the
>> simplest way to fix this is to just print directly using vfprintf(),
>> which is what this patch does. Also, the patch uses
>> flockfile/funlockfile in a few places to increase the likelihood of
>> multiple related messages to end up together on the output.
>>
>> As stderr is unbuffered and stdout is buffered, this patch also gets
>> rid of the functionality to choose where to send the error output, as
>> we don't want to deal with buffering in the error handling path. IMHO
>> this is no loss, as all targets/shells/batch schedulers/ etc. where
>> this might be relevant, offer functionality to redirect stderr to
>> wherever stdout goes. So there is no need for a gfortran-specific
>> mechanism for this.
>
> While this patch makes error printing thread-safe, it's no longer
> async-signal-safe as the stderr lock might lead to a deadlock. So I'm
> retracting this patch and thinking some more about this problem.

So here is an updated patch. Compared to the status quo the main
differences are that it uses a stack allocated buffer to do the
formatting instead of a static one. This prevents conflicts from
multiple threads and in contrast to the approach in the previous patch
should also be async-signal-safe (or at least, less likely to go
belly-up when called from within a signal handler than using
vfprintf() directly). It also introduces a new function to write a
string to standard error without any formatting (estr_write), which is
simpler and less likely to cause problems than the general purpose
vs(n)printf() used before. This can be used in lieu of st_printf in
the majority of cases where no formatting is done.

Regtested on x86_64-unknown-linux-gnu, Ok for trunk?

frontend ChangeLog:

2011-05-08  Janne Blomqvist  <jb@gcc.gnu.org>

	* gfortran.texi: Remove GFORTRAN_USE_STDERR documentation.


library ChangeLog:

2011-05-08  Janne Blomqvist  <jb@gcc.gnu.org>

	* io/unix.c (st_vprintf,st_printf): Move to runtime/error.c.
	* libgfortran.h (struct options_t): Remove use_stderr field.
	(st_vprintf,st_printf): Move prototypes.
	(estr_write): New prototype.
	* runtime/error.c (sys_exit): Use estr_write instead of st_printf.
	(estr_write): New function.
	(st_vprintf): Move from io/unix.c, use stack allocated buffer,
	always output to stderr.
	(st_printf): Move from io/unix.c.
	(show_locus): Use a local variable instead of static.
	(os_error): Use estr_write instead of st_printf.
	(runtime_error): Likewise.
	(runtime_error_at): Likewise.
	(runtime_warning_at): Likewise.
	(internal_error): Likewise.
	(generate_error): Likewise.
	(generate_warning): Likewise.
	(notify_std): Likewise.
	* runtime/pause.c (do_pause): Likewise.
	(pause_string): Likewise.
	* runtime/stop.c (stop_string): Likewise.
	(error_stop_string): Likewise.
	* config/fpu_aix.h (set_fpu): Likewise.
	* config/fpu_generic.h (set_fpu): Likewise.
	* config/fpu_glibc.h (set_fpu): Likewise.
	* config/fpu-sysv.h (set_fpu): Likewise.
	* runtime/backtrace.c (dump_glibc_backtrace): Likewise.
	(show_backtrace): Likewise.
	* runtime/environ.c (print_spaces): Likewise.
	(show_string): Likewise.
	(show_variables): Likewise.
	(variable_table[]): Remove GFORTRAN_USE_STDERR entry.


-- 
Janne Blomqvist
-------------- next part --------------
A non-text attachment was scrubbed...
Name: st_printf2.diff
Type: text/x-patch
Size: 29683 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20110508/bdfbb05c/attachment.bin>