Bug 176030 - [PATCH] lang/mono: bugfixes for mono's 'sgen' GC
Summary: [PATCH] lang/mono: bugfixes for mono's 'sgen' GC
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-mono (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-11 14:00 UTC by Jack Pappas
Modified: 2013-03-19 19:40 UTC (History)
0 users

See Also:


Attachments
file.diff (8.30 KB, patch)
2013-02-11 14:00 UTC, Jack Pappas
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jack Pappas 2013-02-11 14:00:00 UTC
I recently submitted a patch for lang/mono which was just a small configuration change to enable the new 'sgen' garbage collector. After using the new GC for a while, I noticed that mono sometimes crashed with an assertion -- which turned out to be a bug in 'sgen'. I figured out the bug and submitted fixes to the Mono team for inclusion in the next release (3.0.4).

This patch also includes a small configuration change (along with a new patch file, detailed below) which enables Mono's 'sigaltstack' support on FreeBSD.

This patch includes four (4) new files:
- 'files/patch-mono_metadata_sgen-gc.c' and 'files/patch-mono_metadata_threads.c' contain the fixes for the 'sgen' garbage collector.
- 'files/patch-mono_mini_mini-exceptions.c' contains a fix that allows the 'sigaltstack' option to work correctly on FreeBSD.
- 'files/patch-mcs_tools_gacutil_driver.cs' contains a fix for the Mono 'gacutil' tool. This fix is required for my submitted patch to upgrade lang/fsharp to the latest version -- without this fix, the port can't 'deinstall' properly.

Fix: Please see the included 'diff'.

Patch attached with submission follows:
Comment 1 Romain Tartière freebsd_committer freebsd_triage 2013-02-11 15:42:08 UTC
Responsible Changed
From-To: freebsd-ports-bugs->romain

mono@ wants it!
Comment 2 Romain Tartière freebsd_committer freebsd_triage 2013-02-11 15:42:46 UTC
Hi

Can you please provide a link to the mono bug-report so that I can sort
out what is reported upstream and what is not ?

Thanks!

-- 
Romain Tartière <romain@FreeBSD.org>  http://people.FreeBSD.org/~romain/
pgp: 8234 9A78 E7C0 B807 0B59  80FF BA4D 1D95 5112 336F (ID: 0x5112336F)
(plain text =non-HTML= PGP/GPG encrypted/signed e-mail much appreciated)
Comment 3 Romain Tartière freebsd_committer freebsd_triage 2013-02-11 15:46:04 UTC
Responsible Changed
From-To: romain->mono

Another attempt to set the correct responsible.
Comment 4 Jack Pappas 2013-02-11 16:10:47 UTC
Sure -- here are the pull-requests I submitted:
https://github.com/mono/mono/pull/551
https://github.com/mono/mono/pull/553
https://github.com/mono/mono/pull/555
https://github.com/mono/mono/pull/557

Basically, everything included in the diff attached to this PR has also
been submitted upstream. There are some trivial differences, e.g., my
upstream patch modifies 'configure.in' while this PR modifies 'configure'.

-----Original Message-----
From: Romain Tarti=E8re [mailto:romain@FreeBSD.org]
Sent: Monday, February 11, 2013 10:43 AM
To: bug-followup@FreeBSD.org; jack.pappas@tidepowerd.com
Subject: Re: ports/176030: [PATCH] Bugfixes for mono's 'sgen' GC

Hi

Can you please provide a link to the mono bug-report so that I can sort
out what is reported upstream and what is not ?

Thanks!

--
Romain Tarti=E8re <romain@FreeBSD.org>  http://people.FreeBSD.org/~romain/
pgp: 8234 9A78 E7C0 B807 0B59  80FF BA4D 1D95 5112 336F (ID: 0x5112336F)
(plain text =3Dnon-HTML=3D PGP/GPG encrypted/signed e-mail much appreciated=
)
Comment 5 dfilter service freebsd_committer freebsd_triage 2013-03-19 19:38:16 UTC
Author: romain
Date: Tue Mar 19 19:38:01 2013
New Revision: 314682
URL: http://svnweb.freebsd.org/changeset/ports/314682

Log:
  Various bugfixes
  
  While these fixes should have been applied upstream, and new tarballs are
  available, I am quite out of time ATM so after having successfully tested these
  changes, push them to FreeBSD and provide appropriate kudos to appropriate
  People.
  
  Updating to more recent lang/mono is in the pipeline, and should reach the
  ports tree soon.
  
  PR:		ports/176030
  Submitted by:	Jack Pappas <jack.pappas@tidepowerd.com>

Added:
  head/lang/mono/files/patch-mcs_tools_gacutil_driver.cs   (contents, props changed)
  head/lang/mono/files/patch-mono_metadata_sgen-gc.c   (contents, props changed)
  head/lang/mono/files/patch-mono_metadata_threads.c   (contents, props changed)
  head/lang/mono/files/patch-mono_mini_mini-exceptions.c   (contents, props changed)
Modified:
  head/lang/mono/Makefile
  head/lang/mono/files/patch-configure

Modified: head/lang/mono/Makefile
==============================================================================
--- head/lang/mono/Makefile	Tue Mar 19 19:31:57 2013	(r314681)
+++ head/lang/mono/Makefile	Tue Mar 19 19:38:01 2013	(r314682)
@@ -3,7 +3,7 @@
 
 PORTNAME=	mono
 PORTVERSION=	3.0.3
-PORTREVISION=	1
+PORTREVISION=	2
 CATEGORIES=	lang
 MASTER_SITES=	http://download.mono-project.com/sources/${PORTNAME}/
 

Modified: head/lang/mono/files/patch-configure
==============================================================================
--- head/lang/mono/files/patch-configure	Tue Mar 19 19:31:57 2013	(r314681)
+++ head/lang/mono/files/patch-configure	Tue Mar 19 19:38:01 2013	(r314682)
@@ -3,10 +3,12 @@ $FreeBSD$
 
 --- configure.orig
 +++ configure
-@@ -3995,10 +3995,6 @@
+@@ -3993,12 +3993,6 @@
+ 
+ 		libdl=
  		libgc_threads=pthreads
- 		# This doesn't seem to work as of 7.0 on amd64
- 		with_sigaltstack=no
+-		# This doesn't seem to work as of 7.0 on amd64
+-		with_sigaltstack=no
 -# TLS is only partially implemented on -CURRENT (compiler support
 -# but NOT library support)
 -#

Added: head/lang/mono/files/patch-mcs_tools_gacutil_driver.cs
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/lang/mono/files/patch-mcs_tools_gacutil_driver.cs	Tue Mar 19 19:38:01 2013	(r314682)
@@ -0,0 +1,41 @@
+
+$FreeBSD$
+
+--- mcs/tools/gacutil/driver.cs.orig
++++ mcs/tools/gacutil/driver.cs
+@@ -427,15 +427,33 @@
+ 					break;
+ 
+ 				string dir = directories [i];
++				string extension = null;
++				
++				if (File.Exists(Path.Combine (dir, assembly_name + ".dll"))) {
++					extension = ".dll";
++				} else if (File.Exists(Path.Combine (dir, assembly_name + ".DLL"))) {
++					extension = ".DLL";
++				} else if (File.Exists(Path.Combine (dir, assembly_name + ".exe"))) {
++					extension = ".exe";
++				} else if (File.Exists(Path.Combine (dir, assembly_name + ".EXE"))) {
++					extension = ".EXE";
++				} else {
++					failures++;
++					WriteLine("Cannot find the assembly: " + assembly_name);
++					continue;
++				}
++
++				string AssemblyFilename = assembly_name + extension;
+ 
+ 				AssemblyName an = AssemblyName.GetAssemblyName (
+-					Path.Combine (dir, assembly_name + ".dll"));
++					Path.Combine(dir, AssemblyFilename));
+ 				WriteLine ("Assembly: " + an.FullName);
+ 
+ 				Directory.Delete (dir, true);
+ 				if (package != null) {
+ 					string link_dir = Path.Combine (libdir, package);
+-					string link = Path.Combine (link_dir, assembly_name + ".dll");
++					string link = Path.Combine(link_dir, AssemblyFilename);
++
+ 					try { 
+ 						File.Delete (link);
+ 					} catch {

Added: head/lang/mono/files/patch-mono_metadata_sgen-gc.c
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/lang/mono/files/patch-mono_metadata_sgen-gc.c	Tue Mar 19 19:38:01 2013	(r314682)
@@ -0,0 +1,55 @@
+
+$FreeBSD$
+
+--- mono/metadata/sgen-gc.c.orig
++++ mono/metadata/sgen-gc.c
+@@ -179,6 +179,9 @@
+ #ifdef HAVE_PTHREAD_H
+ #include <pthread.h>
+ #endif
++#ifdef HAVE_PTHREAD_ATTR_GET_NP
++#include <pthread_np.h>
++#endif
+ #ifdef HAVE_SEMAPHORE_H
+ #include <semaphore.h>
+ #endif
+@@ -3990,17 +3993,28 @@
+ #endif
+ 
+ 	/* try to get it with attributes first */
+-#if defined(HAVE_PTHREAD_GETATTR_NP) && defined(HAVE_PTHREAD_ATTR_GETSTACK)
+-	{
+-		size_t size;
+-		void *sstart;
+-		pthread_attr_t attr;
+-		pthread_getattr_np (pthread_self (), &attr);
+-		pthread_attr_getstack (&attr, &sstart, &size);
+-		info->stack_start_limit = sstart;
+-		info->stack_end = (char*)sstart + size;
+-		pthread_attr_destroy (&attr);
+-	}
++#if (defined(HAVE_PTHREAD_GETATTR_NP) || defined(HAVE_PTHREAD_ATTR_GET_NP)) && defined(HAVE_PTHREAD_ATTR_GETSTACK)
++  {
++	size_t size;
++	void *sstart;
++	pthread_attr_t attr;
++
++#if defined(HAVE_PTHREAD_GETATTR_NP)
++	/* Linux */
++	pthread_getattr_np (pthread_self (), &attr);
++#elif defined(HAVE_PTHREAD_ATTR_GET_NP)
++	/* BSD */
++	pthread_attr_init (&attr);
++	pthread_attr_get_np (pthread_self (), &attr);
++#else
++#error Cannot determine which API is needed to retrieve pthread attributes.
++#endif
++
++	pthread_attr_getstack (&attr, &sstart, &size);
++	info->stack_start_limit = sstart;
++	info->stack_end = (char*)sstart + size;
++	pthread_attr_destroy (&attr);
++  }
+ #elif defined(HAVE_PTHREAD_GET_STACKSIZE_NP) && defined(HAVE_PTHREAD_GET_STACKADDR_NP)
+ 		 info->stack_end = (char*)pthread_get_stackaddr_np (pthread_self ());
+ 		 info->stack_start_limit = (char*)info->stack_end - pthread_get_stacksize_np (pthread_self ());

Added: head/lang/mono/files/patch-mono_metadata_threads.c
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/lang/mono/files/patch-mono_metadata_threads.c	Tue Mar 19 19:38:01 2013	(r314682)
@@ -0,0 +1,110 @@
+
+$FreeBSD$
+
+--- mono/metadata/threads.c.orig
++++ mono/metadata/threads.c
+@@ -785,7 +785,16 @@
+ void
+ mono_thread_get_stack_bounds (guint8 **staddr, size_t *stsize)
+ {
+-#if defined(HAVE_PTHREAD_GET_STACKSIZE_NP) && defined(HAVE_PTHREAD_GET_STACKADDR_NP)
++#if defined(HOST_WIN32)
++	/* FIXME: 	If this function won't (or shouldn't) ever be called when running on
++			Windows, use the error preprocessor declaration here instead of this
++			default code (to _ensure_ we don't call this function on Windows). */
++	*staddr = NULL;
++	*stsize = (size_t)-1;
++	return;
++
++#elif defined(HAVE_PTHREAD_GET_STACKSIZE_NP) && defined(HAVE_PTHREAD_GET_STACKADDR_NP)
++	/* Mac OS X */
+ 	*staddr = (guint8*)pthread_get_stackaddr_np (pthread_self ());
+ 	*stsize = pthread_get_stacksize_np (pthread_self ());
+ 
+@@ -793,52 +802,54 @@
+ 	*staddr -= *stsize;
+ 	*staddr = (guint8*)((gssize)*staddr & ~(mono_pagesize () - 1));
+ 	return;
+-	/* FIXME: simplify the mess below */
+-#elif !defined(HOST_WIN32)
++
++#elif (defined(HAVE_PTHREAD_GETATTR_NP) || defined(HAVE_PTHREAD_ATTR_GET_NP)) && defined(HAVE_PTHREAD_ATTR_GETSTACK)
++	/* Linux, BSD */
++
+ 	pthread_attr_t attr;
+ 	guint8 *current = (guint8*)&attr;
+ 
+-	pthread_attr_init (&attr);
+-#  ifdef HAVE_PTHREAD_GETATTR_NP
+-	pthread_getattr_np (pthread_self(), &attr);
+-#  else
+-#    ifdef HAVE_PTHREAD_ATTR_GET_NP
+-	pthread_attr_get_np (pthread_self(), &attr);
+-#    elif defined(sun)
+-	*staddr = NULL;
+-	pthread_attr_getstacksize (&attr, &stsize);
+-#    elif defined(__OpenBSD__)
+-	stack_t ss;
+-	int rslt;
+-
+-	rslt = pthread_stackseg_np(pthread_self(), &ss);
+-	g_assert (rslt == 0);
++	#if defined(HAVE_PTHREAD_GETATTR_NP)
++	/* Linux */
++	pthread_getattr_np (pthread_self (), &attr);
+ 
+-	*staddr = (guint8*)((size_t)ss.ss_sp - ss.ss_size);
+-	*stsize = ss.ss_size;
+-#    else
+-	*staddr = NULL;
+-	*stsize = 0;
+-	return;
+-#    endif
+-#  endif
++	#elif defined(HAVE_PTHREAD_ATTR_GET_NP)
++	/* BSD */
++	pthread_attr_init (&attr);
++	pthread_attr_get_np (pthread_self (), &attr);
++	
++	#else
++	#error Cannot determine which API is needed to retrieve pthread attributes.
++	#endif
+ 
+-#  if !defined(sun)
+-#    if !defined(__OpenBSD__)
+ 	pthread_attr_getstack (&attr, (void**)staddr, stsize);
+-#    endif
++	pthread_attr_destroy (&attr);
++
+ 	if (*staddr)
+ 		g_assert ((current > *staddr) && (current < *staddr + *stsize));
+-#  endif
+ 
++	/* When running under emacs, sometimes staddr is not aligned to a page size */
++	*staddr = (guint8*)((gssize)*staddr & ~(mono_pagesize () - 1));
++
++#elif defined(sun)
++	/* What OS / architecture is this for? */
++	pthread_attr_t attr;
++	pthread_attr_init (&attr);
++	pthread_attr_getstacksize (&attr, &stsize);
+ 	pthread_attr_destroy (&attr);
+-#else
+ 	*staddr = NULL;
+-	*stsize = (size_t)-1;
+-#endif
+ 
+ 	/* When running under emacs, sometimes staddr is not aligned to a page size */
+ 	*staddr = (guint8*)((gssize)*staddr & ~(mono_pagesize () - 1));
++	return;
++
++#else
++	/* FIXME:	It'd be better to use the 'error' preprocessor macro here so we know
++			at compile-time if the target platform isn't supported. */
++	*staddr = NULL;
++	*stsize = 0;
++	return;
++#endif
+ }	
+ 
+ MonoThread *

Added: head/lang/mono/files/patch-mono_mini_mini-exceptions.c
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/lang/mono/files/patch-mono_mini_mini-exceptions.c	Tue Mar 19 19:38:01 2013	(r314682)
@@ -0,0 +1,19 @@
+
+$FreeBSD$
+
+--- mono/mini/mini-exceptions.c.orig
++++ mono/mini/mini-exceptions.c
+@@ -2002,10 +2002,10 @@
+ 
+ 	sa.ss_sp = tls->signal_stack;
+ 	sa.ss_size = MONO_ARCH_SIGNAL_STACK_SIZE;
+-#if __APPLE__
+-	sa.ss_flags = 0;
+-#else
++#ifdef __linux__
+ 	sa.ss_flags = SS_ONSTACK;
++#else
++	sa.ss_flags = 0;
+ #endif
+ 	g_assert (sigaltstack (&sa, NULL) == 0);
+ 
_______________________________________________
svn-ports-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-ports-all
To unsubscribe, send any mail to "svn-ports-all-unsubscribe@freebsd.org"
Comment 6 Romain Tartière freebsd_committer freebsd_triage 2013-03-19 19:39:48 UTC
State Changed
From-To: open->closed

Committed. Thanks!