We are some 9 years overdue for this cleanup. If somebody wants to give it a try on more-exotic-than amd64 architectures, we'll know that some time in the future when we update our port, it won't break for a stupid reason (i.e. my mistake). My tested by running this:
AUTOMAKE_VERSION=1.16 AUTOCONF_VERSION=2.71 ./autogen.sh && \ ./configure --with-libatomic-ops=no --enable-cplusplus \ --enable-threads=pthreads --enable-mmap --enable-static=yes && \ gmake gmake check I admit I reindented the code be less ugly and a bit harder to review. OTOH, it's more local context. Thanks Greg Applies to they develeopment branch (exact commit included) https://github.com/ivmai/bdwgc/commits/master >From 7d8cbc8ad5beb68194c32722f5b6c9e19847b07e Mon Sep 17 00:00:00 2001 From: Greg Steuck <g...@nest.cx> Date: Sat, 19 Nov 2022 22:58:41 -0800 Subject: [PATCH] Remove GC_OPENBSD_UTHREADS support This will never be relevant now that 2012 is well behind the 1 year support horizon for OpenBSD. --- include/private/gc_priv.h | 2 - include/private/gcconfig.h | 10 +- os_dep.c | 60 +----------- pthread_stop_world.c | 194 +++++++++++++++---------------------- tests/initfromthread.c | 2 +- 5 files changed, 80 insertions(+), 188 deletions(-) diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index c93710be..f3e1b7c3 100644 --- a/include/private/gc_priv.h +++ b/include/private/gc_priv.h @@ -3023,9 +3023,7 @@ GC_INNER void *GC_store_debug_info_inner(void *p, word sz, const char *str, # define SIG_SUSPEND SIGUSR1 /* SIGTSTP and SIGCONT could be used alternatively on FreeBSD. */ # elif defined(GC_OPENBSD_THREADS) && !defined(GC_USESIGRT_SIGNALS) -# ifndef GC_OPENBSD_UTHREADS # define SIG_SUSPEND SIGXFSZ -# endif # elif defined(_SIGRTMIN) && !defined(CPPCHECK) # define SIG_SUSPEND _SIGRTMIN + 6 # else diff --git a/include/private/gcconfig.h b/include/private/gcconfig.h index 146124d0..799c849f 100644 --- a/include/private/gcconfig.h +++ b/include/private/gcconfig.h @@ -2504,11 +2504,6 @@ EXTERN_C_BEGIN EXTERN_C_END # include <sys/param.h> EXTERN_C_BEGIN - /* Prior to 5.2 release, OpenBSD had user threads and required */ - /* special handling. */ -# if OpenBSD < 201211 -# define GC_OPENBSD_UTHREADS 1 -# endif #endif /* GC_OPENBSD_THREADS */ #if defined(SVR4) || defined(LINUX) || defined(IRIX5) || defined(HPUX) \ @@ -2738,7 +2733,7 @@ EXTERN_C_BEGIN || (defined(CYGWIN32) && defined(I386) && defined(USE_MMAP) \ && !defined(USE_WINALLOC)) \ || (defined(NETBSD) && defined(__ELF__)) \ - || (defined(OPENBSD) && !defined(GC_OPENBSD_UTHREADS)) \ + || defined(OPENBSD) \ || ((defined(SVR4) || defined(AIX) || defined(DGUX) \ || defined(DATASTART_USES_BSDGETDATASTART) \ || (defined(LINUX) && defined(SPARC))) && !defined(PCR)) @@ -2857,8 +2852,7 @@ EXTERN_C_BEGIN # define PTHREAD_STOP_WORLD_IMPL #endif -#if defined(PTHREAD_STOP_WORLD_IMPL) && !defined(NACL) \ - && !defined(GC_OPENBSD_UTHREADS) +#if defined(PTHREAD_STOP_WORLD_IMPL) && !defined(NACL) # define SIGNAL_BASED_STOP_WORLD #endif diff --git a/os_dep.c b/os_dep.c index 16e0ffd8..f51569aa 100644 --- a/os_dep.c +++ b/os_dep.c @@ -524,60 +524,6 @@ GC_INNER const char * GC_get_maps(void) LONGJMP(GC_jmp_buf_openbsd, 1); } -# ifdef GC_OPENBSD_UTHREADS -# include <sys/syscall.h> - EXTERN_C_BEGIN - extern sigset_t __syscall(quad_t, ...); - EXTERN_C_END - - /* Don't use GC_find_limit() because siglongjmp() outside of the */ - /* signal handler by-passes our userland pthreads lib, leaving */ - /* SIGSEGV and SIGPROF masked. Instead, use this custom one that */ - /* works-around the issues. */ - - /* Return the first non-addressable location > p or bound. */ - STATIC ptr_t GC_find_limit_openbsd(ptr_t p, ptr_t bound) - { - static volatile ptr_t result; - /* Safer if static, since otherwise it may not be */ - /* preserved across the longjmp. Can safely be */ - /* static since it's only called with the */ - /* allocation lock held. */ - - struct sigaction act; - word pgsz = (word)sysconf(_SC_PAGESIZE); - - GC_ASSERT((word)bound >= pgsz); - GC_ASSERT(I_HOLD_LOCK()); - - act.sa_handler = GC_fault_handler_openbsd; - sigemptyset(&act.sa_mask); - act.sa_flags = SA_NODEFER | SA_RESTART; - /* act.sa_restorer is deprecated and should not be initialized. */ - sigaction(SIGSEGV, &act, &old_segv_act); - - if (SETJMP(GC_jmp_buf_openbsd) == 0) { - result = (ptr_t)((word)p & ~(pgsz-1)); - for (;;) { - if ((word)result >= (word)bound - pgsz) { - result = bound; - break; - } - result += pgsz; /* no overflow expected */ - GC_noop1((word)(*result)); - } - } - -# ifdef THREADS - /* Due to the siglongjump we need to manually unmask SIGPROF. */ - __syscall(SYS_sigprocmask, SIG_UNBLOCK, sigmask(SIGPROF)); -# endif - - sigaction(SIGSEGV, &old_segv_act, 0); - return result; - } -# endif /* GC_OPENBSD_UTHREADS */ - static volatile int firstpass; /* Return first addressable location > p or bound. */ @@ -2080,11 +2026,7 @@ void GC_register_data_segments(void) ABORT_ARG2("Wrong DATASTART/END pair", ": %p .. %p", (void *)region_start, (void *)DATAEND); for (;;) { -# ifdef GC_OPENBSD_UTHREADS - ptr_t region_end = GC_find_limit_openbsd(region_start, DATAEND); -# else - ptr_t region_end = GC_find_limit_with_bound(region_start, TRUE, DATAEND); -# endif + ptr_t region_end = GC_find_limit_with_bound(region_start, TRUE, DATAEND); GC_add_roots_inner(region_start, region_end, FALSE); if ((word)region_end >= (word)DATAEND) diff --git a/pthread_stop_world.c b/pthread_stop_world.c index f9e78ad7..440f8668 100644 --- a/pthread_stop_world.c +++ b/pthread_stop_world.c @@ -23,36 +23,32 @@ #ifdef NACL # include <unistd.h> # include <sys/time.h> -#elif defined(GC_OPENBSD_UTHREADS) -# include <pthread_np.h> #else # include <signal.h> # include <semaphore.h> # include <errno.h> # include <time.h> /* for nanosleep() */ # include <unistd.h> -#endif /* !GC_OPENBSD_UTHREADS && !NACL */ +#endif /* !NACL */ -#ifndef GC_OPENBSD_UTHREADS - GC_INLINE void GC_usleep(unsigned us) - { +GC_INLINE void GC_usleep(unsigned us) +{ # if defined(LINT2) || defined(THREAD_SANITIZER) - /* Workaround "waiting while holding a lock" static analyzer warning. */ - /* Workaround a rare hang in usleep() trying to acquire TSan Lock. */ - while (us-- > 0) - sched_yield(); /* pretending it takes 1us */ + /* Workaround "waiting while holding a lock" static analyzer warning. */ + /* Workaround a rare hang in usleep() trying to acquire TSan Lock. */ + while (us-- > 0) + sched_yield(); /* pretending it takes 1us */ # elif defined(CPPCHECK) /* || _POSIX_C_SOURCE >= 199309L */ - struct timespec ts; + struct timespec ts; - ts.tv_sec = 0; - ts.tv_nsec = us * 1000; - /* This requires _POSIX_TIMERS feature. */ - (void)nanosleep(&ts, NULL); + ts.tv_sec = 0; + ts.tv_nsec = us * 1000; + /* This requires _POSIX_TIMERS feature. */ + (void)nanosleep(&ts, NULL); # else - usleep(us); + usleep(us); # endif - } -#endif /* !GC_OPENBSD_UTHREADS */ +} #ifdef NACL @@ -66,7 +62,7 @@ volatile int GC_nacl_thread_parked[MAX_NACL_GC_THREADS]; int GC_nacl_thread_used[MAX_NACL_GC_THREADS]; -#elif !defined(GC_OPENBSD_UTHREADS) +#else #if (!defined(AO_HAVE_load_acquire) || !defined(AO_HAVE_store_release)) \ && !defined(CPPCHECK) @@ -769,7 +765,7 @@ STATIC void GC_restart_handler(int sig) # undef ao_load_async # undef ao_store_async # undef ao_store_release_async -#endif /* !GC_OPENBSD_UTHREADS && !NACL */ +#endif /* !NACL */ /* Should do exactly the right thing if the world is stopped; should */ /* not fail if it is not. */ @@ -923,66 +919,45 @@ STATIC int GC_suspend_all(void) # ifndef NACL GC_thread p; pthread_t self = pthread_self(); -# ifndef GC_OPENBSD_UTHREADS - int result; + int result; - GC_ASSERT((GC_stop_count & THREAD_RESTARTED) == 0); -# endif + GC_ASSERT((GC_stop_count & THREAD_RESTARTED) == 0); GC_ASSERT(I_HOLD_LOCK()); for (i = 0; i < THREAD_TABLE_SZ; i++) { for (p = GC_threads[i]; p != NULL; p = p -> tm.next) { if (!THREAD_EQUAL(p -> id, self)) { if ((p -> flags & (FINISHED | DO_BLOCKING)) != 0) continue; -# ifndef GC_OPENBSD_UTHREADS -# ifdef GC_ENABLE_SUSPEND_THREAD - if ((p -> ext_suspend_cnt & 1) != 0) continue; -# endif - if (AO_load(&(p -> last_stop_count)) == GC_stop_count) - continue; /* matters only if GC_retry_signals */ - n_live_threads++; +# ifdef GC_ENABLE_SUSPEND_THREAD + if ((p -> ext_suspend_cnt & 1) != 0) continue; # endif + if (AO_load(&(p -> last_stop_count)) == GC_stop_count) + continue; /* matters only if GC_retry_signals */ + n_live_threads++; # ifdef DEBUG_THREADS GC_log_printf("Sending suspend signal to %p\n", (void *)p->id); # endif -# ifdef GC_OPENBSD_UTHREADS - { - stack_t stack; - - GC_acquire_dirty_lock(); - if (pthread_suspend_np(p -> id) != 0) - ABORT("pthread_suspend_np failed"); - GC_release_dirty_lock(); - if (pthread_stackseg_np(p->id, &stack)) - ABORT("pthread_stackseg_np failed"); - p -> stack_ptr = (ptr_t)stack.ss_sp - stack.ss_size; - if (GC_on_thread_event) - GC_on_thread_event(GC_EVENT_THREAD_SUSPENDED, - (void *)p->id); - } -# else - /* The synchronization between GC_dirty (based on */ - /* test-and-set) and the signal-based thread suspension */ - /* is performed in GC_stop_world because */ - /* GC_release_dirty_lock cannot be called before */ - /* acknowledging the thread is really suspended. */ - result = raise_signal(p, GC_sig_suspend); - switch(result) { - case ESRCH: - /* Not really there anymore. Possible? */ - n_live_threads--; - break; - case 0: - if (GC_on_thread_event) - GC_on_thread_event(GC_EVENT_THREAD_SUSPENDED, - (void *)(word)THREAD_SYSTEM_ID(p)); - /* Note: thread id might be truncated. */ - break; - default: - ABORT_ARG1("pthread_kill failed at suspend", - ": errcode= %d", result); - } -# endif + /* The synchronization between GC_dirty (based on */ + /* test-and-set) and the signal-based thread suspension */ + /* is performed in GC_stop_world because */ + /* GC_release_dirty_lock cannot be called before */ + /* acknowledging the thread is really suspended. */ + result = raise_signal(p, GC_sig_suspend); + switch(result) { + case ESRCH: + /* Not really there anymore. Possible? */ + n_live_threads--; + break; + case 0: + if (GC_on_thread_event) + GC_on_thread_event(GC_EVENT_THREAD_SUSPENDED, + (void *)(word)THREAD_SYSTEM_ID(p)); + /* Note: thread id might be truncated. */ + break; + default: + ABORT_ARG1("pthread_kill failed at suspend", + ": errcode= %d", result); + } } } } @@ -1042,7 +1017,7 @@ STATIC int GC_suspend_all(void) GC_INNER void GC_stop_world(void) { -# if !defined(GC_OPENBSD_UTHREADS) && !defined(NACL) +# if !defined(NACL) int n_live_threads; # endif GC_ASSERT(I_HOLD_LOCK()); @@ -1065,7 +1040,7 @@ GC_INNER void GC_stop_world(void) } # endif /* PARALLEL_MARK */ -# if defined(GC_OPENBSD_UTHREADS) || defined(NACL) +# if defined(NACL) (void)GC_suspend_all(); # else AO_store(&GC_stop_count, GC_stop_count + THREAD_RESTARTED); @@ -1259,16 +1234,13 @@ GC_INNER void GC_stop_world(void) int i; pthread_t self = pthread_self(); GC_thread p; -# ifndef GC_OPENBSD_UTHREADS - int result; + int result; - GC_ASSERT((GC_stop_count & THREAD_RESTARTED) != 0); -# endif + GC_ASSERT((GC_stop_count & THREAD_RESTARTED) != 0); for (i = 0; i < THREAD_TABLE_SZ; i++) { for (p = GC_threads[i]; p != NULL; p = p -> tm.next) { if (!THREAD_EQUAL(p -> id, self)) { if ((p -> flags & (FINISHED | DO_BLOCKING)) != 0) continue; -# ifndef GC_OPENBSD_UTHREADS # ifdef GC_ENABLE_SUSPEND_THREAD if ((p -> ext_suspend_cnt & 1) != 0) continue; # endif @@ -1276,32 +1248,24 @@ GC_INNER void GC_stop_world(void) && AO_load(&(p -> last_stop_count)) == GC_stop_count) continue; /* The thread has been restarted. */ n_live_threads++; -# endif # ifdef DEBUG_THREADS GC_log_printf("Sending restart signal to %p\n", (void *)p->id); # endif -# ifdef GC_OPENBSD_UTHREADS - if (pthread_resume_np(p -> id) != 0) - ABORT("pthread_resume_np failed"); - if (GC_on_thread_event) - GC_on_thread_event(GC_EVENT_THREAD_UNSUSPENDED, (void *)p->id); -# else - result = raise_signal(p, GC_sig_thr_restart); - switch(result) { - case ESRCH: - /* Not really there anymore. Possible? */ - n_live_threads--; - break; - case 0: - if (GC_on_thread_event) - GC_on_thread_event(GC_EVENT_THREAD_UNSUSPENDED, - (void *)(word)THREAD_SYSTEM_ID(p)); - break; - default: - ABORT_ARG1("pthread_kill failed at resume", - ": errcode= %d", result); - } -# endif + result = raise_signal(p, GC_sig_thr_restart); + switch(result) { + case ESRCH: + /* Not really there anymore. Possible? */ + n_live_threads--; + break; + case 0: + if (GC_on_thread_event) + GC_on_thread_event(GC_EVENT_THREAD_UNSUSPENDED, + (void *)(word)THREAD_SYSTEM_ID(p)); + break; + default: + ABORT_ARG1("pthread_kill failed at resume", + ": errcode= %d", result); + } } } } @@ -1318,27 +1282,21 @@ GC_INNER void GC_start_world(void) # ifdef DEBUG_THREADS GC_log_printf("World starting\n"); # endif -# ifndef GC_OPENBSD_UTHREADS - AO_store_release(&GC_stop_count, GC_stop_count + THREAD_RESTARTED); - /* The updated value should now be visible to the */ - /* signal handler (note that pthread_kill is not on */ - /* the list of functions which synchronize memory). */ -# endif + AO_store_release(&GC_stop_count, GC_stop_count + THREAD_RESTARTED); + /* The updated value should now be visible to the */ + /* signal handler (note that pthread_kill is not on */ + /* the list of functions which synchronize memory). */ n_live_threads = GC_restart_all(); -# ifdef GC_OPENBSD_UTHREADS - (void)n_live_threads; -# else - if (GC_retry_signals) { - resend_lost_signals_retry(n_live_threads, GC_restart_all); - } else { + if (GC_retry_signals) { + resend_lost_signals_retry(n_live_threads, GC_restart_all); + } else { # ifndef GC_NETBSD_THREADS_WORKAROUND - if (GC_sig_suspend == GC_sig_thr_restart) + if (GC_sig_suspend == GC_sig_thr_restart) # endif - { - suspend_restart_barrier(n_live_threads); - } + { + suspend_restart_barrier(n_live_threads); } -# endif + } # ifdef DEBUG_THREADS GC_log_printf("World started\n"); # endif @@ -1355,7 +1313,7 @@ GC_INNER void GC_start_world(void) GC_INNER void GC_stop_init(void) { -# if !defined(GC_OPENBSD_UTHREADS) && !defined(NACL) +# if !defined(NACL) struct sigaction act; char *str; @@ -1433,7 +1391,7 @@ GC_INNER void GC_stop_init(void) /* Explicitly unblock the signals once before new threads creation. */ GC_unblock_gc_signals(); # endif -# endif /* !GC_OPENBSD_UTHREADS && !NACL */ +# endif /* !NACL */ } #endif /* PTHREAD_STOP_WORLD_IMPL */ diff --git a/tests/initfromthread.c b/tests/initfromthread.c index 1a799e9e..9917309d 100644 --- a/tests/initfromthread.c +++ b/tests/initfromthread.c @@ -74,7 +74,7 @@ int main(void) DWORD thread_id; # endif # if !(defined(BEOS) || defined(MSWIN32) || defined(MSWINCE) \ - || defined(CYGWIN32) || defined(GC_OPENBSD_UTHREADS) \ + || defined(CYGWIN32) \ || (defined(DARWIN) && !defined(NO_PTHREAD_GET_STACKADDR_NP)) \ || ((defined(FREEBSD) || defined(LINUX) || defined(NETBSD) \ || defined(HOST_ANDROID)) && !defined(NO_PTHREAD_GETATTR_NP) \ -- 2.38.1
