[Mono-dev] NetBSD patches

[Paolo Molaro]

On 01/29/07 kangaroo wrote:
> I'm obviously having a difficult morning with this.

> Index: libgc/configure.in
> ===================================================================
> --- libgc/configure.in	(revision 71807)
> +++ libgc/configure.in	(working copy)
> @@ -151,6 +151,14 @@
>  	  AC_DEFINE(PARALLEL_MARK)
>  	fi
>  	;;
> +     *-*-netbsd*)
> +	AC_DEFINE(GC_NETBSD_THREADS)
> +	AC_DEFINE(THREAD_LOCAL_ALLOC)
> +	if test "${enable_parallel_mark}" = yes; then
> +	  AC_DEFINE(PARALLEL_MARK)
> +	fi
> +	AC_DEFINE(THREAD_LOCAL_ALLOC)

THREAD_LOCAL_ALLOC is defined twice.

> Index: libgc/pthread_stop_world.c
> ===================================================================
> --- libgc/pthread_stop_world.c	(revision 71807)
> +++ libgc/pthread_stop_world.c	(working copy)
> @@ -111,6 +111,7 @@
>  #  endif
>  #endif
>  
> +#if !defined(GC_NETBSD_THREADS)
>  sem_t GC_suspend_ack_sem;
>  
>  static void _GC_suspend_handler(int sig)
> @@ -220,6 +221,7 @@
>      GC_printf1("In GC_restart_handler for 0x%lx\n", pthread_self());
>  #endif
>  }
> +#endif /* !GC_NETBSD_THREADS */
>  
>  # ifdef IA64
>  #   define IF_IA64(x) x
> @@ -295,12 +297,14 @@
>        ABORT("Collecting from unknown thread.");
>  }
>  
> +#if !defined(GC_NETBSD_THREADS)
>  void GC_restart_handler(int sig)
>  {
>  	int old_errno = errno;
>  	_GC_restart_handler (sig);
>  	errno = old_errno;
>  }
> +#endif
>  
>  /* We hold allocation lock.  Should do exactly the right thing if the	*/
>  /* world is stopped.  Should not fail if it isn't.			*/
> @@ -309,6 +313,28 @@
>      pthread_push_all_stacks();
>  }
>  
> +#if defined(GC_NETBSD_THREADS)
> +/* 
> + * Get the stack start address for the specified address.
> + */ 
> +int 
> +np_stackinfo(pthread_t p, void **addr) 
> +{ 
> +	pthread_attr_t attr; 
> +	int ret = -1; 
> +
> +	if (pthread_attr_init(&attr)) 
> +		return -1;
> +
> +	if (!pthread_attr_get_np(p, &attr))
> +		if (!pthread_attr_getstackaddr(&attr, addr)) 
> +			ret = 0;
> +
> +	pthread_attr_destroy(&attr); 
> +	return ret; 
> +}
> +#endif
> +
>  /* There seems to be a very rare thread stopping problem.  To help us  */
>  /* debug that, we save the ids of the stopping thread. */
>  pthread_t GC_stopping_thread;
> @@ -334,6 +360,7 @@
>              if (p -> stop_info.last_stop_count == GC_stop_count) continue;
>  	    if (p -> thread_blocked) /* Will wait */ continue;
>              n_live_threads++;
> +#if !defined(GC_NETBSD_THREADS)
>  	    #if DEBUG_THREADS
>  	      GC_printf1("Sending suspend signal to 0x%lx\n", p -> id);
>  	    #endif
> @@ -349,6 +376,23 @@
>                  default:
>                      ABORT("pthread_kill failed");
>              }
> +#else
> +	    #if DEBUG_THREADS
> +	      GC_printf1("Suspending 0x%lx ...\n", p -> id);
> +	    #endif
> +
> +		if(pthread_suspend_np(p -> id) != 0)
> +			GC_printf1("Could not susend thread... 0x%lx\n", p -> id);
> +
> +		/* Right now, this is not enough. Retreiving the stack base address is not the correct */
> +		/* info to give to the GC, but since there is no way to get the current stack pointer  */
> +		/* for the suspended thread, base pointer will have to be enough. Mono seems to be     */
> +		/* happy with it so... */
> +		if(np_stackinfo(p -> id, &(p -> stop_info.stack_ptr)) != 0)
> +			GC_err_printf1("Could not get thread stack address... 0x%lx\n", p -> id);			
> +
> +        n_live_threads--;
> +#endif /* !GC_NETBSD_THREADS */

Why do you need these stop/start changes? The thread stack addresse is
definitely not enough: we need all the registers from a suspended
thread. What doesn't work in the signal code for stopping the threads?

> Index: libgc/dyn_load.c
> ===================================================================
> --- libgc/dyn_load.c	(revision 71807)
> +++ libgc/dyn_load.c	(working copy)
> @@ -75,6 +75,11 @@
>  #   include <dlfcn.h>
>  #   include <link.h>
>  #endif
> +
> +#ifdef NETBSD 
> +#include <dlfcn.h>
> +#endif

There is already a #if defined(NETBSD) a few lines below...
Anyway, for mono we should just #ifdef out this whole file.

> Index: configure.in
> ===================================================================
> --- configure.in	(revision 71807)
> +++ configure.in	(working copy)
> @@ -82,14 +82,16 @@
>  		;;
>  	*-*-*netbsd*)
>  		platform_win32=no
> -		CPPFLAGS="$CPPFLAGS -D_REENTRANT"
> +		CPPFLAGS="$CPPFLAGS -D_REENTRANT -DGC_NETBSD_THREADS -D_GNU_SOURCE"
>  		libmono_cflags="-D_REENTRANT"
>  		LDFLAGS="$LDFLAGS -pthread"
>  		CPPFLAGS="$CPPFLAGS -DPLATFORM_BSD"
>  		libmono_ldflags="-pthread"
>  		need_link_unlink=yes
> -		libdl=
> -		libgc_threads=no
> +		libdl="-ldl /libexec/ld.elf_so"

What is /libexec/ld.elf_so needed for?

> +		libgc_threads=pthreads
> +		with_tls=__thread

You should not force-set libgc_threads. configure will check if it
works.

lupus

-- 
-----------------------------------------------------------------
lupus at debian.org                                     debian/rules
lupus at ximian.com                             Monkeys do it better