Visa Hankala <[email protected]> wrote: > On Sat, Mar 02, 2019 at 01:40:50PM -0500, Ted Unangst wrote: > > Visa Hankala wrote: > > > On Sat, Mar 02, 2019 at 04:37:04PM +0100, Sebastien Marie wrote: > > > > Thread 1 (thread 469200): > > > > #0 sched_yield () at -:3 > > > > #1 0x00000a8c0609d9c5 in _libc__spinlock (lock=Variable "lock" is not > > > > available.) at /usr/src/lib/libc/thread/rthread.c:50 > > > > #2 0x00000a8c060702be in _thread_flockfile (fp=0x7f7ffffcfaa8) at > > > > /usr/src/lib/libc/thread/rthread_file.c:180 > > > > #3 0x00000a8c0609e11a in _libc_fflush (fp=0x7f7ffffcfaa8) at > > > > /usr/src/lib/libc/stdio/fflush.c:46 > > > > #4 0x00000a8c0606c89a in _libc_vdprintf (fd=Variable "fd" is not > > > > available.) at /usr/src/lib/libc/stdio/vdprintf.c:72 > > > > #5 0x00000a8c060a7f63 in _libc__rthread_debug (level=Variable "level" > > > > is not available.) at /usr/src/lib/libc/thread/rthread_debug.c:23 > > > > #6 0x00000a8c060410c5 in _rthread_mutex_timedlock (mutexp=Variable > > > > "mutexp" is not available.) at > > > > /usr/src/lib/libc/thread/rthread_mutex.c:163 > > > > #7 0x00000a8c060bc482 in malloc (size=56) at > > > > /usr/src/lib/libc/stdlib/malloc.c:1253 > > > > #8 0x00000a8c060703e4 in _thread_flockfile (fp=0x7f7ffffd02b8) at > > > > /usr/src/lib/libc/thread/rthread_file.c:156 > > > > #9 0x00000a8c0609e11a in _libc_fflush (fp=0x7f7ffffd02b8) at > > > > /usr/src/lib/libc/stdio/fflush.c:46 > > > > #10 0x00000a8c0606c89a in _libc_vdprintf (fd=Variable "fd" is not > > > > available.) at /usr/src/lib/libc/stdio/vdprintf.c:72 > > > > #11 0x00000a8c060a7f63 in _libc__rthread_debug (level=Variable "level" > > > > is not available.) at /usr/src/lib/libc/thread/rthread_debug.c:23 > > > > #12 0x00000a8c3c2792b6 in _rthread_reaper () at > > > > /data/openbsd/src/lib/librthread/rthread.c:260 > > > > #13 0x00000a8c3c279229 in pthread_join (thread=Variable "thread" is not > > > > available.) at /data/openbsd/src/lib/librthread/rthread.c:319 > > > > #14 0x00000a8993d1a705 in main (argc=1, argv=0x7f7ffffd0ab8) at > > > > test.c:86 > > > > > > This does not look good. The thread is recursing with hash_lock of > > > rthread_file.c. Apparently triggered by the debug output routine. > > > > > > > Yeah, I think thread_debug needs to use snprintf and write the message > > itself. > > Does the below look good? > > Index: thread/rthread_debug.c > =================================================================== > RCS file: src/lib/libc/thread/rthread_debug.c,v > retrieving revision 1.3 > diff -u -p -r1.3 rthread_debug.c > --- thread/rthread_debug.c 5 Sep 2017 02:40:54 -0000 1.3 > +++ thread/rthread_debug.c 3 Mar 2019 06:31:26 -0000 > @@ -5,6 +5,7 @@ > #include <pthread.h> > #include <stdarg.h> > #include <stdio.h> > +#include <string.h> > #include <unistd.h> > > #include "rthread.h" > @@ -18,10 +19,12 @@ void > _rthread_debug(int level, const char *fmt, ...) > { > if (_rthread_debug_level >= level) { > + char buf[256]; > va_list ap; > va_start(ap, fmt); > - vdprintf(STDERR_FILENO, fmt, ap); > + vsnprintf(buf, sizeof(buf), fmt, ap); > va_end(ap); > + write(STDERR_FILENO, buf, strlen(buf)); > } > } > DEF_STRONG(_rthread_debug); >
We previously decided that the dprintf family is as safe as snprintf+write, and we are preferring dprintf in various places, such as signal-safe. Can you explain why it not safe here? What is different?
