Re: [Gc] boehm-gc patch

Ivan Maidanski ivmai at mail.ru
Tue Dec 1 11:16:40 PST 2009


Hi!
Ben Elliston <bje at au1.ibm.com> wrote:
> Hi.
> 
> I would like to bring the following patches to your attention, which
> were contributed to the version of the Boehm GC in the GCC tree:
> 
>   http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01508.html

The message is:

	* os_dep.c: Use the POSIX signal API in preference to the BSD API.
	Generate a compilation error if neither the POSIX nor BSD APIs can
	be detected.

Index: os_dep.c
===================================================================
--- os_dep.c	(revision 154694)
+++ os_dep.c	(working copy)
@@ -501,7 +501,13 @@ void GC_enable_signals(void)
       && !defined(MACOS) && !defined(DJGPP) && !defined(DOS4GW) \
       && !defined(NOSYS) && !defined(ECOS)
 
-#   if defined(sigmask) && !defined(UTS4) && !defined(HURD)
+#   if defined(SIG_BLOCK)
+	/* Use POSIX/SYSV interface */
+#	define SIGSET_T sigset_t
+#	define SIG_DEL(set, signal) sigdelset(&(set), (signal))
+#	define SIG_FILL(set) sigfillset(&set)
+#	define SIGSETMASK(old, new) sigprocmask(SIG_SETMASK, &(new), &(old))
+#   elif defined(sigmask) && !defined(UTS4) && !defined(HURD)
 	/* Use the traditional BSD interface */
 #	define SIGSET_T int
 #	define SIG_DEL(set, signal) (set) &= ~(sigmask(signal))
@@ -511,11 +517,7 @@ void GC_enable_signals(void)
     	  /* a signal 32.						*/
 #	define SIGSETMASK(old, new) (old) = sigsetmask(new)
 #   else
-	/* Use POSIX/SYSV interface	*/
-#	define SIGSET_T sigset_t
-#	define SIG_DEL(set, signal) sigdelset(&(set), (signal))
-#	define SIG_FILL(set) sigfillset(&set)
-#	define SIGSETMASK(old, new) sigprocmask(SIG_SETMASK, &(new), &(old))
+#       error undetectable signal API
 #   endif
 
I'm sorry this seems to be version 6.x as there is no SIG_FILL/DEL() in the GC at present. Could you re-post it against the BDWGC CVS.

>   http://gcc.gnu.org/ml/gcc-patches/2009-11/msg01636.html

This message is:

When compiling mark_rts.c, GCC warns about taking the address of a local
variable.  This is not a bug, but an explicit hack to get the
approximate address of a new stack frame, to calculate the limits of the
current frame.  There is a cleaner way to do this with GCC: use
__builtin_frame_address.  My reading of the code suggests that this will
work just as well.

Tested with a bootstrap on x86_64-linux and a full regression testsuite
run, including make check-target-boehm-gc. Okay for mainline?

2009-11-30  Ben Elliston  <bje at au.ibm.com>

	* mark_rts.c (GC_approx_sp): Use __builtin_frame_address when
	compiling with GCC rather than taking the address of a local
	variable.

Index: mark_rts.c
===================================================================
--- mark_rts.c	(revision 154749)
+++ mark_rts.c	(working copy)
@@ -376,7 +376,13 @@ ptr_t GC_approx_sp()
 #   ifdef _MSC_VER
 #     pragma warning(disable:4172)
 #   endif
-    return((ptr_t)(&dummy));
+#ifdef __GNUC__
+    /* Eliminate a warning from GCC about taking the address of a
+       local variable.  */
+    return __builtin_frame_address (0);
+#else
+    return ((ptr_t)(&dummy));
+#endif /* __GNUC__ */
 #   ifdef _MSC_VER
 #     pragma warning(default:4172)
 #   endif

The warning problem was already solved (about a year ago).
As for __builtin_frame_address(0), I think it's worth using (although useless, in fact, except for, may be, the case when GC_approx_sp() is inlined by the compiler).
__builtin_frame_address is recognized even in pre-3.0 GCC (I checked it with EMX GCC 2.8.1).

Hans -

Do you think the same (about __builtin_frame_address(0))? If you, I'll add it to the repo (just "not to loose the code") after the tarball release.

> 
> I hope these can be integrated upstream.
> 
> Regards, Ben

Bye.


More information about the Gc mailing list