OSEC

Neohapsis is currently accepting applications for employment. For more information, please visit our website www.neohapsis.com or email hr@neohapsis.com
 
From: Marco S Hyman (marc_at_snafu.org)
Date: Wed Jan 22 2003 - 18:34:25 CST

  • Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

    A known weak spot in the pthread implementation has been floating point
    support. If multiple threads of the same application used floating
    point they'd trash each other.

    This patch fixes that problem for i386 *only*. It does not effect
    any other arch as the code to save/restore fp context doesn't exist
    for any other arch. That problem will be addressed in the future.
    The only down side I can see to this fix is that the fp context is
    saved every thread switch, which isn't the fastest thing in the world.

    A second problem was the "Fatal error '_pq_insert_tail: Already in
    priority queue' ..." problem. I sent an earlier patch to tech to
    help track that down. It worked, and I know have what appears to be
    a proper fix.

    So, -current thread users: please apply this patch and let me know if
    it makes things better (or worse) for you. Start from a clean -current
    source tree (back out any earlier patches) and then:

    If running -current (after 20 jan 03):

            cd /usr/src/lib/libpthread
            patch < /path/to/this/patch
            make obj && make depend && make
            sudo make install

    If running -current before 20 jan 03:

            cd /usr/src/lib/libc_r
            patch < /path/to/this/patch
            make obj && make depend && make
            sudo make install

    The patches will NOT apply cleanly to a -stable tree. You're welcome to
    try, but you'll need to do lots of clean-up by hand. However, do NOT
    mix/match an old libc and new libpthread. It will cause problems.
    Do not try to run the new libpthread with an old gcc -- it won't work.

    // marc

    Index: arch/i386/uthread_machdep.c
    ===================================================================
    RCS file: /cvs/src/lib/libpthread/arch/i386/uthread_machdep.c,v
    retrieving revision 1.2
    diff -u -p -r1.2 uthread_machdep.c
    --- arch/i386/uthread_machdep.c 13 Mar 2001 00:05:51 -0000 1.2
    +++ arch/i386/uthread_machdep.c 23 Jan 2003 00:12:01 -0000
    -36,11 +36,8 struct frame {
      * structure that can be later switched to.
      */
     void
    -_thread_machdep_init(statep, base, len, entry)
    - struct _machdep_state* statep;
    - void *base;
    - int len;
    - void (*entry)(void);
    +_thread_machdep_init(struct _machdep_state* statep, void *base, int len,
    + void (*entry)(void))
     {
             struct frame *f;
     
    -58,23 +55,33 _thread_machdep_init(statep, base, len,
             f->fr_eip = (int)entry;
     
             statep->esp = (int)f;
    +
    + _thread_machdep_save_float_state(statep);
     }
     
    +/*
    + * Floating point save restore copied from code in npx.c
    + * (without really understanding what it does).
    + */
    +#define fldcw(addr) __asm("fldcw %0" : : "m" (*addr))
    +#define fnsave(addr) __asm("fnsave %0" : "=m" (*addr))
    +#define frstor(addr) __asm("frstor %0" : : "m" (*addr))
    +#define fwait() __asm("fwait")
    +
     void
    -_thread_machdep_save_float_state(ms)
    - struct _machdep_state *ms;
    +_thread_machdep_save_float_state(struct _machdep_state *ms)
     {
    - char *fdata = (char *)&ms->fpreg;
    + struct save87 *addr = &ms->fpreg;
     
    - __asm__("fsave %0"::"m" (*fdata));
    + fnsave(addr);
    + fwait();
     }
     
     void
    -_thread_machdep_restore_float_state(ms)
    - struct _machdep_state *ms;
    +_thread_machdep_restore_float_state(struct _machdep_state *ms)
     {
    - char *fdata = (char *)&ms->fpreg;
    + struct save87 *addr = &ms->fpreg;
     
    - __asm__("frstor %0"::"m" (*fdata));
    + frstor(addr);
     }
     
    Index: arch/i386/uthread_machdep.h
    ===================================================================
    RCS file: /cvs/src/lib/libpthread/arch/i386/uthread_machdep.h,v
    retrieving revision 1.7
    diff -u -p -r1.7 uthread_machdep.h
    --- arch/i386/uthread_machdep.h 4 Oct 2000 05:55:34 -0000 1.7
    +++ arch/i386/uthread_machdep.h 23 Jan 2003 00:12:01 -0000
    -1,10 +1,10
     /* $OpenBSD: uthread_machdep.h,v 1.7 2000/10/04 05:55:34 d Exp $ */
     /* David Leonard, <dcsee.uq.edu.au>. Public domain. */
     
    -#include <machine/reg.h>
    +#include <machine/npx.h>
     
     struct _machdep_state {
             int esp;
    - struct fpreg fpreg;
    + struct save87 fpreg;
     };
     
    Index: uthread/pthread_private.h
    ===================================================================
    RCS file: /cvs/src/lib/libpthread/uthread/pthread_private.h,v
    retrieving revision 1.40
    diff -u -p -r1.40 pthread_private.h
    --- uthread/pthread_private.h 11 Dec 2002 23:21:19 -0000 1.40
    +++ uthread/pthread_private.h 23 Jan 2003 00:12:01 -0000
    -1073,7 +1073,7 struct pthread *_get_curthread(void);
     void _set_curthread(struct pthread *);
     int _thread_create(pthread_t *, const pthread_attr_t *,
                            void *(*start_routine)(void *), void *,pthread_t);
    -void _dispatch_signals(pthread_t, struct sigcontext *);
    +void _dispatch_signals(struct sigcontext *);
     void _thread_signal(pthread_t, int);
     int _mutex_cv_lock(pthread_mutex_t *);
     int _mutex_cv_unlock(pthread_mutex_t *);
    Index: uthread/uthread_kern.c
    ===================================================================
    RCS file: /cvs/src/lib/libpthread/uthread/uthread_kern.c,v
    retrieving revision 1.23
    diff -u -p -r1.23 uthread_kern.c
    --- uthread/uthread_kern.c 4 Nov 2002 21:28:49 -0000 1.23
    +++ uthread/uthread_kern.c 23 Jan 2003 00:12:01 -0000
    -115,11 +115,6 _thread_kern_sched(struct sigcontext * s
                     memcpy(&curthread->saved_sigcontext, scp,
                         sizeof(curthread->saved_sigcontext));
     
    - /*
    - * Save floating point state.
    - */
    - _thread_machdep_save_float_state(&curthread->_machdep);
    -
                     /* Flag the signal context as the last state saved: */
                     curthread->sig_saved = 1;
             } else
    -130,6 +125,9 _thread_kern_sched(struct sigcontext * s
             if ((curthread->flags & PTHREAD_FLAGS_PRIVATE) == 0)
                     _last_user_thread = curthread;
     
    + /* Save floating point state. */
    + _thread_machdep_save_float_state(&curthread->_machdep);
    +
             /* Save errno. */
             curthread->error = errno;
     
    -501,41 +499,38 _thread_kern_sched(struct sigcontext * s
                             /* Restore errno. */
                             errno = curthread->error;
     
    + /* Restore floating point state. */
    + _thread_machdep_restore_float_state(&curthread->_machdep);
    +
                             /*
                              * Restore the new thread, saving current.
                              */
                             _thread_machdep_switch(&curthread->_machdep,
    - &old_thread_run->_machdep);
    + &old_thread_run->_machdep);
     
    - /* Check if a signal context was saved: */
    - if (curthread->sig_saved == 1) {
    - /*
    - * Restore floating point state.
    - */
    - _thread_machdep_restore_float_state(&curthread->_machdep);
    -
    - /*
    - * Do a sigreturn to restart the thread that
    - * was interrupted by a signal:
    - */
    - _thread_kern_in_sched = 0;
    + /*
    + * Process any pending signals for the thread we
    + * just switched to.
    + */
    + _thread_kern_in_sched = 0;
    + _dispatch_signals(scp);
     
    - /*
    - * If we had a context switch, run any
    - * installed switch hooks.
    - */
    - if ((_sched_switch_hook != NULL) &&
    - (_last_user_thread != curthread)) {
    - _thread_run_switch_hook(_last_user_thread,
    - curthread);
    - }
    + /* run any installed switch-hooks */
    + if ((_sched_switch_hook != NULL) &&
    + (_last_user_thread != curthread)) {
    + _thread_run_switch_hook(_last_user_thread,
    + curthread);
    + }
     
    - if (((curthread->cancelflags &
    - PTHREAD_AT_CANCEL_POINT) == 0) &&
    - ((curthread->cancelflags &
    - PTHREAD_CANCEL_ASYNCHRONOUS) != 0))
    - pthread_testcancel();
    + /* check for thread cancellation */
    + if (((curthread->cancelflags &
    + PTHREAD_AT_CANCEL_POINT) == 0) &&
    + ((curthread->cancelflags &
    + PTHREAD_CANCEL_ASYNCHRONOUS) != 0))
    + pthread_testcancel();
     
    + /* Check if a signal context was saved: */
    + if (curthread->sig_saved == 1) {
                                     /* return to signal handler. This code
                                        should be:
                                        _thread_sys_sigreturn(&curthread->saved_sigcontext);
    -543,22 +538,7 _thread_kern_sched(struct sigcontext * s
                                        sparc */
                                     return;
                             } else {
    - /*
    - * This is the normal way out of the scheduler.
    - */
    - _thread_kern_in_sched = 0;
    -
    - if (_sched_switch_hook != NULL) {
    - /* Run the installed switch hook: */
    - _thread_run_switch_hook(_last_user_thread,
    - curthread);
    - }
    -
    - if (((curthread->cancelflags &
    - PTHREAD_AT_CANCEL_POINT) == 0) &&
    - ((curthread->cancelflags &
    - PTHREAD_CANCEL_ASYNCHRONOUS) != 0))
    - pthread_testcancel();
    + /* This is the normal way out */
                                     return;
                             }
     
    Index: uthread/uthread_sig.c
    ===================================================================
    RCS file: /cvs/src/lib/libpthread/uthread/uthread_sig.c,v
    retrieving revision 1.16
    diff -u -p -r1.16 uthread_sig.c
    --- uthread/uthread_sig.c 8 Nov 2002 23:18:25 -0000 1.16
    +++ uthread/uthread_sig.c 23 Jan 2003 00:12:01 -0000
    -75,8 +75,8 _thread_sig_process(int sig, struct sigc
                     locked = 1;
     
             if (locked || _thread_sigact[sig - 1].sa_flags & SA_NODEFER) {
    - _thread_sig_handle(sig, scp);
                     pending_sigs[sig - 1] = 0;
    + _thread_sig_handle(sig, scp);
             } else
                     check_pending = 1;
             if (locked)
    -167,7 +167,7 _thread_sig_handler(int sig, siginfo_t *
                             signal_lock = _SPINLOCK_UNLOCKED;
                             for (i = 1; i < NSIG; i++)
                                     if (pending_sigs[i - 1])
    - _thread_sig_process(i, scp);
    + _thread_sig_process(i, scp);
                     }
             }
     
    -268,22 +268,14 _thread_sig_handle(int sig, struct sigco
                                             pthread->sig_defer_count--;
                             }
                             /*
    - * give each thread a chance to dispatch pending
    - * signals.
    + * Give the current thread a chance to dispatch
    + * the signals. Other threads will get thier
    + * chance (if the signal is still pending) later.
                              */
    - TAILQ_FOREACH(pthread, &_thread_list, tle) {
    - /* Current thread inside critical region? */
    - if (curthread->sig_defer_count > 0)
    - pthread->sig_defer_count++;
    - _dispatch_signals(pthread, scp);
    - if (curthread->sig_defer_count > 0)
    - pthread->sig_defer_count--;
    - }
    + _dispatch_signals(scp);
    +
                     }
             }
    -
    - /* Returns nothing. */
    - return;
     }
     
     /* Perform thread specific actions in response to a signal: */
    -385,12 +377,12 _thread_signal(pthread_t pthread, int si
     }
     
     /*
    - * possibly dispatch a signal to the given thread.
    + * possibly dispatch a signal to the current thread.
      */
     void
    -_dispatch_signals(pthread_t pthread, struct sigcontext * scp)
    +_dispatch_signals(struct sigcontext * scp)
     {
    - pthread_t pthread_saved;
    + struct pthread *curthread = _get_curthread();
             struct sigaction act;
             void (*action)(int, siginfo_t *, void *);
             int i;
    -399,7 +391,7 _dispatch_signals(pthread_t pthread, str
              * Check if there are pending signals for the running
              * thread that aren't blocked:
              */
    - if ((pthread->sigpend & ~pthread->sigmask) != 0)
    + if ((curthread->sigpend & ~curthread->sigmask) != 0)
                     /* Look for all possible pending signals: */
                     for (i = 1; i < NSIG; i++)
                             /*
    -408,8 +400,8 _dispatch_signals(pthread_t pthread, str
                              */
                             if (_thread_sigact[i - 1].sa_handler != SIG_DFL &&
                                 _thread_sigact[i - 1].sa_handler != SIG_IGN &&
    - sigismember(&pthread->sigpend,i) &&
    - !sigismember(&pthread->sigmask,i)) {
    + sigismember(&curthread->sigpend,i) &&
    + !sigismember(&curthread->sigmask,i)) {
                                     action = _thread_sigact[i - 1].sa_sigaction;
                                     _clear_pending_flag(i);
     
    -426,10 +418,7 _dispatch_signals(pthread_t pthread, str
                                      * Dispatch the signal via the custom signal
                                      * handler.
                                      */
    - pthread_saved = _get_curthread();
    - _set_curthread(pthread);
                                     (*action)(i, &info_queue[i - 1], scp);
    - _set_curthread(pthread_saved);
                             }
     }
     #endif