Re: Minor boehm-gc fixes

[Rainer Orth <ro at CeBiTec dot Uni-Bielefeld dot DE>]

Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

> [Please keep me on the Cc:, I'm not subscribed to java-patches.]
>
> While investigating the boehm-gc gctest crash on Solaris 11/x86 with CVS
> GNU ld that led to this patch
>
> 	More Solaris 2 ABI fixes: emit _DYNAMIC etc. into .dynsym
>         http://sourceware.org/ml/binutils/2010-09/msg00482.html
>
> I've found two minor issues where the fixes are obvious.  They are no
> longer applicable to CVS boehm-gc, though.
>
> I'm not sure what to do about the third one: for unknown reasons,
> configure.ac has
>
> dnl We need to override the top-level CFLAGS.  This is how we do it.
> MY_CFLAGS="$CFLAGS"
> AC_SUBST(MY_CFLAGS)
>
> yet MY_CFLAGS isn't set anywhere else.  This causes problems because it
> isn't passed to subdirs (missing from AM_MAKEFLAGS) or multilibs
> (missing from multi-do in the toplevel config-ml.in), as I noticed when
> trying to rebuild with make CFLAGS='-g3 -O0'.  I'd suggest to just scrap
> it to avoid those problems, but perhaps Tom (who introduced this 12
> years ago) can shed some light on this?

Unfortunately, I haven't received any response to the question above.
Further investigation shows that boehm-gc/Makefile.am can be simplified
considerably: I can not only remove MY_CFLAGS completely (which is just
an alias for CFLAGS), but also the private definitions of LTCOMPILE,
LTLDFLAGS, and LINK: after replacing MY_CLAGS by CFLAGS in the current
definitions, LTLDFLAGS is identical to the default, LINK as well (with
the exception of using $(CC) instead of $(CCLD)), and the only
difference between the LTCOMPILE default and the local one is the
additional passing of GC_CFLAGS.  But GC_CFLAGS is already assigned to
AM_CFLAGS, so those flags are simply passed twice.  If you remove the
whole mess, everything works as before.  If building with
CFLAGS='<something>', they are properly passed down to non-default
multilibs.

Tested by configuring and building on i386-pc-solaris2.10 with i386 and
amd64 multilibs.

Ok for mainline now?

> With the gld patch above (not yet approved), boehm-gc and libjava test
> results on i386-pc-solaris2.11 are just as good as with Sun ld, with one
> exception: while gctest works fine if run manually, it fails if the
> 32-bit version is run from make check:
>
> % ./gctest
> Completed 3 tests
> Allocated 5680907 collectable objects
> Allocated 306 uncollectable objects
> Allocated 3743702 atomic objects
> Allocated 34440 stubborn objects
> Finalized 6615/6615 objects - finalization is probably ok
> Total number of bytes allocated is 190636656
> Final heap size is 11628544 bytes
> Collector appears to work
> Completed 66 collections
> % make check
> [...]
> Completed 3 tests
> Allocated 5682423 collectable objects
> Allocated 306 uncollectable objects
> Allocated 3749384 atomic objects
> Allocated 34421 stubborn objects
> Finalized 6583/6583 objects - finalization is probably ok
> Total number of bytes allocated is 189829352
> Final heap size is 77918208 bytes
> Unexpected heap growth - collector may be broken
> Test failed
>
> On the other hand, for 64-bit gctest, both manual invocation and make
> check work fine.
>
> Maybe Hans has a suggesting how to debug this?  Unfortunately, due to PR
> boehm-gc/11412 (boehm-gc testing problems), such failures easily go
> unnoticed right now.  I plan to convert the boehm-gc testing to DejaGnu
> at some point to remedy this.
>
> I'm going to check in the patch below as obvious unless someone objects.

I'll wait for approval of the MY_CFLAGS etc. removal and check the whole
bunch in as a unit.

Thanks.
	Rainer


2010-09-24  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	boehm-gc:
	* dyn_load.c: Fix typo.
	* pthread_support.c (GC_get_thread_stack_base) [DEBUG_THREADS]:
	Use GC_printf0.

	* configure.ac (MY_CFLAGS): Remove.
	* configure: Regenerate.
	* Makefile.am (LTCOMPILE): Remove.
	(LTLDFLAGS): Remove.
	(LINK): Remove.
	(AM_MAKEFLAGS): Don't pass MY_CLAGS.
	* Makefile.in: Regenerate.
	* include/Makefile.in: Regenerate.

diff -r c16225ebe332 boehm-gc/Makefile.am
--- a/boehm-gc/Makefile.am	Wed Sep 29 12:45:38 2010 +0200
+++ b/boehm-gc/Makefile.am	Fri Oct 01 13:05:51 2010 +0200
@@ -94,15 +94,6 @@
 .S.lo:
 	$(LTCOMPILE) -Wp,-P -x assembler-with-cpp -c $<
 
-## We have our own definition of LTCOMPILE because we want to use our
-## CFLAGS, not those passed in from the top level make.
-LTCOMPILE = $(LIBTOOL) $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=compile \
-	$(CC) $(DEFS) $(AM_CPPFLAGS) $(CPPFLAGS) \
-	$(AM_CFLAGS) $(MY_CFLAGS) $(GC_CFLAGS) 
-LTLDFLAGS = $(shell $(top_srcdir)/../libtool-ldflags $(LDFLAGS))
-LINK = $(LIBTOOL) $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) --mode=link \
-	$(CC) $(AM_CFLAGS) $(MY_CFLAGS) $(LTLDFLAGS) -o $@
-
 # Work around what appears to be a GNU make bug handling MAKEFLAGS
 # values defined in terms of make variables, as is the case for CC and
 # friends when we are called from the top level Makefile.
@@ -122,7 +113,6 @@
 	"LIBCFLAGS_FOR_TARGET=$(LIBCFLAGS_FOR_TARGET)" \
 	"MAKE=$(MAKE)" \
 	"MAKEINFO=$(MAKEINFO) $(MAKEINFOFLAGS)" \
-	"MY_CFLAGS=$(MY_CFLAGS)" \
 	"PICFLAG=$(PICFLAG)" \
 	"PICFLAG_FOR_TARGET=$(PICFLAG_FOR_TARGET)" \
 	"SHELL=$(SHELL)" \
diff -r c16225ebe332 boehm-gc/configure.ac
--- a/boehm-gc/configure.ac	Wed Sep 29 12:45:38 2010 +0200
+++ b/boehm-gc/configure.ac	Fri Oct 01 13:05:51 2010 +0200
@@ -498,10 +498,6 @@
  *) AC_MSG_RESULT(no) ;;
 esac
 
-dnl We need to override the top-level CFLAGS.  This is how we do it.
-MY_CFLAGS="$CFLAGS"
-AC_SUBST(MY_CFLAGS)
-
 dnl Include defines that have become de facto standard.
 dnl ALL_INTERIOR_POINTERS can be overridden in startup code.
 AC_DEFINE(SILENT,1,[disables statistics printing])
diff -r c16225ebe332 boehm-gc/dyn_load.c
--- a/boehm-gc/dyn_load.c	Wed Sep 29 12:45:38 2010 +0200
+++ b/boehm-gc/dyn_load.c	Fri Oct 01 13:05:51 2010 +0200
@@ -122,7 +122,7 @@
 #    endif
 #  endif
 
-/* An user-supplied routine that is called to dtermine if a DSO must
+/* An user-supplied routine that is called to determine if a DSO must
    be scanned by the gc.  */
 static int (*GC_has_static_roots)(const char *, void *, size_t);
 /* Register the routine.  */
diff -r c16225ebe332 boehm-gc/pthread_support.c
--- a/boehm-gc/pthread_support.c	Wed Sep 29 12:45:38 2010 +0200
+++ b/boehm-gc/pthread_support.c	Fri Oct 01 13:05:51 2010 +0200
@@ -1139,7 +1139,7 @@
   if (pthread_getattr_np (my_pthread, &attr) != 0)
     {
 #   ifdef DEBUG_THREADS
-      GC_printf1("Can not determine stack base for attached thread");
+      GC_printf0("Can not determine stack base for attached thread");
 #   endif
       return 0;
     }


-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University