[JAVA] /bin/sh portability issues in scripts/check_jni_methods.sh

[JAVA] /bin/sh portability issues in scripts/check_jni_methods.sh

[Roger Sayle]

This patch, or more likely request for help, is the fourth and final
in the series to enable mainline GCC to be bootstrapped with the native
/bin/sh on Solaris/x86.  The current build failures are a regression
from gcc 3.4.  These four patches appear to address all of the build
system related issues with the default build.

Now for the admission.  This final patch, to scripts/check_jni_methods.sh
is more of a workaround (hack?) than a solution to the portability issues.
The problem is that this shell script contains the command "set -C" which
isn't supported by Solaris' sh.  Commenting it out, stops the script from
terminating with a hard error, and enables libjava to reach the
application linking steps at the end of the build (which suffer from
boehmgc issues that I may not have resolved correctly).

However, I've no idea what the correct way to fix this problem is.  I
did discover that this issue is related to PR other/2141, which was a
similar use of "set -C" in gccbug.  The solution there was to reorganize
the script to avoid the need for this functionality.

If any shell scripting gurus could take a look and see what the
correct fix for this problem is, it'd be much appreciated.

Many thanks in advance,



2006-07-06  Roger Sayle  <roger@eyesopen.com>

	* scripts/check_jni_methods.sh: Don't use the "set -C" command
	which isn't available in all shells.


Index: scripts/check_jni_methods.sh
===================================================================
*** scripts/check_jni_methods.sh	(revision 115223)
--- scripts/check_jni_methods.sh	(working copy)
***************
*** 3,9 ****
  # Fail if any command fails
  set -e
  # Don't override existing files
! set -C

  TMPFILE=/tmp/check-jni-methods.$$.1
  TMPFILE2=/tmp/check-jni-methods.$$.2
--- 3,10 ----
  # Fail if any command fails
  set -e
  # Don't override existing files
! # Alas, this doesn't appear to be portable
! # set -C

  TMPFILE=/tmp/check-jni-methods.$$.1
  TMPFILE2=/tmp/check-jni-methods.$$.2


Roger
--

Re: [JAVA] /bin/sh portability issues in scripts/check_jni_methods.sh

[Rainer Orth]

Roger Sayle <roger@eyesopen.com> writes:

> terminating with a hard error, and enables libjava to reach the
> application linking steps at the end of the build (which suffer from
> boehmgc issues that I may not have resolved correctly).

This is probably PR boehm-gc/21942.

I've recently sent a new message about this to the boehm-gc list:

	http://www.hpl.hp.com/hosted/linux/mail-archives/gc/2006-June/001334.html

In the past, it was possible to get around boehm-gc test failures by
switching to pthreads on Solaris/x86 (though I vaguely recall that this
broke Solaris/SPARC).  I'm still waiting for Hans to comment on my patch
and eventually make a copy of gc6.[89] which has made this switch available
for testing.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Faculty of Technology, Bielefeld University

Re: [JAVA] /bin/sh portability issues in scripts/check_jni_methods.sh

[Roger Sayle]

On 7 Jul 2006, Rainer Orth wrote:
> Roger Sayle <roger@eyesopen.com> writes:
> > terminating with a hard error, and enables libjava to reach the
> > application linking steps at the end of the build (which suffer from
> > boehmgc issues that I may not have resolved correctly).
>
> This is probably PR boehm-gc/21942.
> I've recently sent a new message about this to the boehm-gc list:
> http://www.hpl.hp.com/hosted/linux/mail-archives/gc/2006-June/001334.html

Cool!  Yes, this exactly the issue and your patch above is nearly
identical to the one I have in my tree to allow the build to reach
libjava.  Unfortunately, I'm seeing a unresolved symbol reference to
GC_register_has_static_roots_callback from libgcj.so when attempting
to link jv-convert, so I wasn't sure if it was my X86_64/SUNOS5 changes
in boehmgc/include/private/gcconfig.h that were at fault, or another
latent problem in the libjava build system.

Anyway, thanks again for pointing out that you're on top of this
particular issue (one less thing for me to worry about).  Hopefully,
Hans will comment soon.  I did check the upstream sources to see if
it was just an import of the latest boehmgc that was required.

Thanks again for addressing this.

Roger
--

Solaris/X96 GC issues (was /bin/sh portability issues ...)

[Boehm, Hans]

I checked Rainer's patch into the 7.0 tree, and added it to my current
6.8 tree.  Thanks.

That doesn't help with the Studio 11 cc problems that Rainer reported
earlier.  Someone with access to a suitable machine will have to track
those down.  The next step is to identify which objects are getting
prematurely collected and why.  It would be useful to see whether the
offending list is referenced by an automatic or static pointer.  In
either case, it is also worth checking that GC_with_callee_saves_pushed
ends up calling getcontext(), as it should.  If it is a static pointer,
and getcontext() is called correctly, I would next check that roots are
being registered correctly, and the offending pointer is in a root
segment.  Setting the GC_DUMP_REGULARLY environment variable prints
roots on a regular basis.

It is possible that the studio 11 issue also affects gcc, but just
happened to not be exercised.

I think the GC_register_has_static_roots_callback issue was
libjava-specific, since I hadn't gotten around to putting the patch that
introduced it into the CVS tree.  (I just corrected that).  It should be
defined in dyn_load.c, though it would be undefined if DYNAMIC_LOADING
somehow didn't get defined in gcconfig.h.  (That's probably not the way
things should work.  I made it a noop in that case for 7.0.  But the
real problem is probably in your gcconfig.h.)

Slightly longer term, I'm not sure about the right path.  I don't really
have enough cycles to work on 7.0, so I don't expect to spend time on
6.8, except for relatively small and critical patches.  If someone can
generate a 6.8 patch to make Solaris/X86 work, I'm certainly fine with
just putting it into gcc.  I suspect it would be big enough to be
problematic.

Overall, my vote would be for

1) Making 7.0 work reliably on Solaris/X86.  (I have been testing
occasionally on Solaris/SPARC, and it seems OK there.)
2) Merging 7.0 into 4.3 early in that cycle.  My expectation is that
that would generate some breakage, mostly on less common platforms.  But
on platforms like Linux, I think 7.0 is actually now fairly stable.

Hans

> -----Original Message-----
> From: java-patches-owner@gcc.gnu.org 
> [mailto:java-patches-owner@gcc.gnu.org] On Behalf Of Roger Sayle
> Sent: Friday, July 07, 2006 6:25 AM
> To: Rainer Orth
> Cc: java-patches@gcc.gnu.org; gcc-patches@gcc.gnu.org
> Subject: Re: [JAVA] /bin/sh portability issues in 
> scripts/check_jni_methods.sh
> 
> 
> On 7 Jul 2006, Rainer Orth wrote:
> > Roger Sayle <roger@eyesopen.com> writes:
> > > terminating with a hard error, and enables libjava to reach the 
> > > application linking steps at the end of the build (which 
> suffer from 
> > > boehmgc issues that I may not have resolved correctly).
> >
> > This is probably PR boehm-gc/21942.
> > I've recently sent a new message about this to the boehm-gc list:
> > 
> http://www.hpl.hp.com/hosted/linux/mail-archives/gc/2006-June/001334.h
> > tml
> 
> Cool!  Yes, this exactly the issue and your patch above is 
> nearly identical to the one I have in my tree to allow the 
> build to reach libjava.  Unfortunately, I'm seeing a 
> unresolved symbol reference to 
> GC_register_has_static_roots_callback from libgcj.so when 
> attempting to link jv-convert, so I wasn't sure if it was my 
> X86_64/SUNOS5 changes in boehmgc/include/private/gcconfig.h 
> that were at fault, or another latent problem in the libjava 
> build system.
> 
> Anyway, thanks again for pointing out that you're on top of 
> this particular issue (one less thing for me to worry about). 
>  Hopefully, Hans will comment soon.  I did check the upstream 
> sources to see if it was just an import of the latest boehmgc 
> that was required.
> 
> Thanks again for addressing this.
> 
> Roger
> --
> 
> 

RE: Solaris/X86 GC issues (was /bin/sh portability issues ...)

[Boehm, Hans]

Sorry about the typo in the previous subject.  Reposting to make
searches work.

Hans

> -----Original Message-----
> From: java-patches-owner@gcc.gnu.org 
> [mailto:java-patches-owner@gcc.gnu.org] On Behalf Of Boehm, Hans
> Sent: Friday, July 07, 2006 3:45 PM
> To: Roger Sayle; Rainer Orth
> Cc: java-patches@gcc.gnu.org; gcc-patches@gcc.gnu.org; 
> gc@linux.hpl.hp.com
> Subject: Solaris/X96 GC issues (was /bin/sh portability issues ...)
> 
> I checked Rainer's patch into the 7.0 tree, and added it to my current
> 6.8 tree.  Thanks.
> 
> That doesn't help with the Studio 11 cc problems that Rainer 
> reported earlier.  Someone with access to a suitable machine 
> will have to track those down.  The next step is to identify 
> which objects are getting prematurely collected and why.  It 
> would be useful to see whether the offending list is 
> referenced by an automatic or static pointer.  In either 
> case, it is also worth checking that 
> GC_with_callee_saves_pushed ends up calling getcontext(), as 
> it should.  If it is a static pointer, and getcontext() is 
> called correctly, I would next check that roots are being 
> registered correctly, and the offending pointer is in a root 
> segment.  Setting the GC_DUMP_REGULARLY environment variable 
> prints roots on a regular basis.
> 
> It is possible that the studio 11 issue also affects gcc, but 
> just happened to not be exercised.
> 
> I think the GC_register_has_static_roots_callback issue was 
> libjava-specific, since I hadn't gotten around to putting the 
> patch that introduced it into the CVS tree.  (I just 
> corrected that).  It should be defined in dyn_load.c, though 
> it would be undefined if DYNAMIC_LOADING somehow didn't get 
> defined in gcconfig.h.  (That's probably not the way things 
> should work.  I made it a noop in that case for 7.0.  But the 
> real problem is probably in your gcconfig.h.)
> 
> Slightly longer term, I'm not sure about the right path.  I 
> don't really have enough cycles to work on 7.0, so I don't 
> expect to spend time on 6.8, except for relatively small and 
> critical patches.  If someone can generate a 6.8 patch to 
> make Solaris/X86 work, I'm certainly fine with just putting 
> it into gcc.  I suspect it would be big enough to be problematic.
> 
> Overall, my vote would be for
> 
> 1) Making 7.0 work reliably on Solaris/X86.  (I have been 
> testing occasionally on Solaris/SPARC, and it seems OK there.)
> 2) Merging 7.0 into 4.3 early in that cycle.  My expectation 
> is that that would generate some breakage, mostly on less 
> common platforms.  But on platforms like Linux, I think 7.0 
> is actually now fairly stable.
> 
> Hans
> 
> > -----Original Message-----
> > From: java-patches-owner@gcc.gnu.org
> > [mailto:java-patches-owner@gcc.gnu.org] On Behalf Of Roger Sayle
> > Sent: Friday, July 07, 2006 6:25 AM
> > To: Rainer Orth
> > Cc: java-patches@gcc.gnu.org; gcc-patches@gcc.gnu.org
> > Subject: Re: [JAVA] /bin/sh portability issues in 
> > scripts/check_jni_methods.sh
> > 
> > 
> > On 7 Jul 2006, Rainer Orth wrote:
> > > Roger Sayle <roger@eyesopen.com> writes:
> > > > terminating with a hard error, and enables libjava to reach the 
> > > > application linking steps at the end of the build (which
> > suffer from
> > > > boehmgc issues that I may not have resolved correctly).
> > >
> > > This is probably PR boehm-gc/21942.
> > > I've recently sent a new message about this to the boehm-gc list:
> > > 
> > 
> http://www.hpl.hp.com/hosted/linux/mail-archives/gc/2006-June/001334.h
> > > tml
> > 
> > Cool!  Yes, this exactly the issue and your patch above is nearly 
> > identical to the one I have in my tree to allow the build to reach 
> > libjava.  Unfortunately, I'm seeing a unresolved symbol 
> reference to 
> > GC_register_has_static_roots_callback from libgcj.so when 
> attempting 
> > to link jv-convert, so I wasn't sure if it was my
> > X86_64/SUNOS5 changes in boehmgc/include/private/gcconfig.h
> > that were at fault, or another latent problem in the libjava build 
> > system.
> > 
> > Anyway, thanks again for pointing out that you're on top of this 
> > particular issue (one less thing for me to worry about).
> >  Hopefully, Hans will comment soon.  I did check the 
> upstream sources 
> > to see if it was just an import of the latest boehmgc that was 
> > required.
> > 
> > Thanks again for addressing this.
> > 
> > Roger
> > --
> > 
> > 
> 

Re: [JAVA] /bin/sh portability issues in scripts/check_jni_methods.sh

[Mike Stump]

On Jul 6, 2006, at 9:44 PM, Roger Sayle wrote:
> Now for the admission.  This final patch, to scripts/ 
> check_jni_methods.sh
> is more of a workaround (hack?) than a solution to the portability  
> issues.

After reviewing the script, I see no reason why they use -C.  I think  
your patch is the right solution.  I might possibly add:

   rm -f $TMPFILE $TMPFILE2 $TMPFILE3

to the top of the script, if people are worried about _something_...

Re: [JAVA] /bin/sh portability issues in scripts/check_jni_methods.sh

[Ralf Wildenhues]

* Roger Sayle wrote on Fri, Jul 07, 2006 at 06:44:19AM CEST:
> 
> However, I've no idea what the correct way to fix this problem is.  I
> did discover that this issue is related to PR other/2141, which was a
> similar use of "set -C" in gccbug.  The solution there was to reorganize
> the script to avoid the need for this functionality.

Here's a patch (well, it turned more or less into a rewrite), to use
a temporary directory and avoid 'set -C' and some other glitches and
non-Posix flags, as a suggestion.  (There is no need for backslash-
newline after a pipe.)

I think the noclobber may have been used to prevent a security issue
(the file names below /tmp were easily guessed).

I don't know whether it would be better for you to have configure
compute $EGREP, $FGREP and use a check_jni_methods.sh.in.

Beware, I have only lightly tested this patch.

(Note I do have papers for GCC in place, but I'm no regular contributor,
nor have any commit rights or anything.)

Cheers,
Ralf

2006-07-07  Ralf Wildenhues  <Ralf.Wildenhues@gmx.de>

	* scripts/check_jni_methods.sh: Don't use `set -C'; rewrite to
	use a temporary directory, honor $TMPDIR.  Do not use `diff -U',
	`grep -h', cope with either of egrep/fgrep or `grep -E'/`grep -F'.
	Cope with special characters in file names.

Index: scripts/check_jni_methods.sh
===================================================================
--- scripts/check_jni_methods.sh	(revision 115253)
+++ scripts/check_jni_methods.sh	(working copy)
@@ -2,61 +2,88 @@
 
 # Fail if any command fails
 set -e
-# Don't override existing files
-set -C
 
-TMPFILE=/tmp/check-jni-methods.$$.1
-TMPFILE2=/tmp/check-jni-methods.$$.2
-TMPFILE3=/tmp/check-jni-methods.$$.3
+# The following code taken from the Autoconf manual.
+# Create a temporary directory $tmp in $TMPDIR (default /tmp).
+# Use mktemp if possible; otherwise fall back on mkdir,
+# with $RANDOM to make collisions less likely.
+: ${TMPDIR=/tmp}
+{
+  tmp=`
+    (umask 077 && mktemp -d "$TMPDIR/check-jni-methods-XXXXXX") 2>/dev/null
+  ` &&
+  test -n "$tmp" && test -d "$tmp"
+} || {
+  tmp=$TMPDIR/check-jni-methods-$$-$RANDOM
+  (umask 077 && mkdir "$tmp")
+} || exit $?
 
+TMPFILE=$tmp/headers
+TMPFILE2=$tmp/sources
+TMPFILE3=$tmp/ignore
+
+if echo 'ab*c' | grep -F 'ab*c' >/dev/null 2>&1; then
+  FGREP='grep -F'
+else
+  FGREP=fgrep
+fi
+if echo a | grep -E '(a|b)' >/dev/null 2>&1; then
+  EGREP='grep -E'
+else
+  EGREP=egrep
+fi
+
 # Find all methods defined in the header files generated
 # from the java source files.
-grep -h '^JNIEXPORT .* Java_' include/*.h | \
-        LC_ALL=C sed -e 's,.*JNICALL \(Java_[a-z_A-Z0-9]*\).*$,\1,' | \
+cat include/*.h | grep '^JNIEXPORT .* Java_' |
+        LC_ALL=C sed 's,.*JNICALL \(Java_[a-z_A-Z0-9]*\).*$,\1,' |
 	sort > $TMPFILE
 
 # Find all methods in the JNI C source files.
-find native/jni -name \*.c | \
-	xargs grep -h '^Java_' | \
-        LC_ALL=C sed -e 's,^\(Java_[a-z_A-Z0-9]*\) *(.*$,\1,' > $TMPFILE2
+find native/jni -name \*.c |
+	xargs cat |
+	grep '^Java_' |
+        LC_ALL=C sed 's,^\(Java_[a-z_A-Z0-9]*\) *(.*$,\1,' > $TMPFILE2
 # Or in the the C++ files. (Note that cpp doesn't follow gnu conventions atm)
 # So we try to match both GNU style and some other style.
-find native/jni -name \*.cpp | \
-	xargs grep -h '^Java_' | \
-        LC_ALL=C sed -e 's,^\(Java_[a-z_A-Z0-9]*\) *(.*$,\1,' >> $TMPFILE2
-find native/jni -name \*.cpp | \
-	xargs egrep -h '^(JNIEXPORT .* JNICALL )?Java_' | \
-	cut -f4 -d\  | \
-        LC_ALL=C sed -e 's,^\JNIEXPORT .* JNICALL \(Java_[a-z_A-Z0-9]*\) *(.*$,\1,' >> $TMPFILE2
-mv $TMPFILE2 $TMPFILE3
-sort $TMPFILE3 > $TMPFILE2
-rm $TMPFILE3
+find native/jni -name \*.cpp |
+	xargs cat |
+	grep '^Java_' |
+        LC_ALL=C sed 's,^\(Java_[a-z_A-Z0-9]*\) *(.*$,\1,' >> $TMPFILE2
+find native/jni -name \*.cpp |
+	xargs cat |
+	$EGREP '^(JNIEXPORT .* JNICALL )?Java_' |
+	cut -f4 -d\  |
+        LC_ALL=C sed 's,^\JNIEXPORT .* JNICALL \(Java_[a-z_A-Z0-9]*\) *(.*$,\1,' >> $TMPFILE2
+mv "$TMPFILE2" "$TMPFILE3"
+sort "$TMPFILE3" > $TMPFILE2
+rm "$TMPFILE3"
 
 # Write temporary ignore file.
 cat > $TMPFILE3 << EOF
--Java_gnu_java_awt_peer_gtk_GtkMenuComponentPeer_dispose
--Java_java_lang_VMSystem_arraycopy
--Java_java_lang_VMSystem_identityHashCode
+< Java_gnu_java_awt_peer_gtk_GtkMenuComponentPeer_dispose
+< Java_java_lang_VMSystem_arraycopy
+< Java_java_lang_VMSystem_identityHashCode
 EOF
 
 # Compare again silently.
 # Use fgrep and direct the output to /dev/null for compatibility with older
 # grep instead of using the non portable -q.
-if diff -b -U 0 $TMPFILE $TMPFILE2 | grep '^[+-]Java' | \
-    fgrep -v -f $TMPFILE3 > /dev/null;
+if diff -b "$TMPFILE" "$TMPFILE2" | grep '^[<>] Java' |
+    $FGREP -v -f "$TMPFILE3" > /dev/null
 then
   PROBLEM=1
   echo "Found a problem with the JNI methods declared and implemented."
-  echo "(-) missing in implementation, (+) missing in header files"
+  echo "(<) missing in implementation, (>) missing in header files"
 
   # Compare the found method lists.
-  diff -b -U 0 $TMPFILE $TMPFILE2  | grep '^[+-]Java' | fgrep -v -f $TMPFILE3
+  diff -b "$TMPFILE" "$TMPFILE2"  | grep '^[<>] Java' | $FGREP -v -f "$TMPFILE3"
 fi
 
 # Cleanup.
-rm -f $TMPFILE $TMPFILE2 $TMPFILE3
+rm -rf "$tmp"
 
-if test "$PROBLEM" = "1" ; then
+if test "$PROBLEM" = 1; then
   exit 1
 fi
 

Re: [JAVA] /bin/sh portability issues in scripts/check_jni_methods.sh

[Andrew Pinski]

On Jul 7, 2006, at 5:51 PM, Ralf Wildenhues wrote:

> Here's a patch (well, it turned more or less into a rewrite), to use
> a temporary directory and avoid 'set -C' and some other glitches and
> non-Posix flags, as a suggestion.  (There is no need for backslash-
> newline after a pipe.)


But "set -C" is POSIX.

-- Pinski

Re: [JAVA] /bin/sh portability issues in scripts/check_jni_methods.sh

[Ralf Wildenhues]

* Andrew Pinski wrote on Sat, Jul 08, 2006 at 02:57:31AM CEST:
> On Jul 7, 2006, at 5:51 PM, Ralf Wildenhues wrote:
> 
> >Here's a patch (well, it turned more or less into a rewrite), to use
> >a temporary directory and avoid 'set -C' and some other glitches and
> >non-Posix flags, as a suggestion.  (There is no need for backslash-
> >newline after a pipe.)
> 
> But "set -C" is POSIX.

Yes.  The 'other' was supposed to refer to glitches only, not to the
POSIX-ness of things.  Sorry if I was not clear.

(If you'd stick to POSIX only without taking additional limitations into
account, there'd be more trouble...)

Cheers,
Ralf

Re: Solaris/X96 GC issues (was /bin/sh portability issues ...)

[Rainer Orth]

Hans,

> I checked Rainer's patch into the 7.0 tree, and added it to my current
> 6.8 tree.  Thanks.

great, thanks.

> That doesn't help with the Studio 11 cc problems that Rainer reported
> earlier.  Someone with access to a suitable machine will have to track
> those down.  The next step is to identify which objects are getting
> prematurely collected and why.  It would be useful to see whether the
> offending list is referenced by an automatic or static pointer.  In
> either case, it is also worth checking that GC_with_callee_saves_pushed
> ends up calling getcontext(), as it should.  If it is a static pointer,
> and getcontext() is called correctly, I would next check that roots are
> being registered correctly, and the offending pointer is in a root
> segment.  Setting the GC_DUMP_REGULARLY environment variable prints
> roots on a regular basis.
> 
> It is possible that the studio 11 issue also affects gcc, but just
> happened to not be exercised.

I'll start investigating this once I find some time.

> Slightly longer term, I'm not sure about the right path.  I don't really
> have enough cycles to work on 7.0, so I don't expect to spend time on
> 6.8, except for relatively small and critical patches.  If someone can
> generate a 6.8 patch to make Solaris/X86 work, I'm certainly fine with
> just putting it into gcc.  I suspect it would be big enough to be
> problematic.

Could you please provide a snapshot of 6.8 so I can test it on
Solaris/SPARC and Solaris/x86, try the integration into GCC and see if it
works?  Depending on the amount of changes since 6.6, it might be
acceptable for 4.2.  At the very least, I'd like to get my patch for 64-bit
Solaris/x86 support into 4.2 so it at least compiles out of the box.

> Overall, my vote would be for
> 
> 1) Making 7.0 work reliably on Solaris/X86.  (I have been testing
> occasionally on Solaris/SPARC, and it seems OK there.)
> 2) Merging 7.0 into 4.3 early in that cycle.  My expectation is that
> that would generate some breakage, mostly on less common platforms.  But
> on platforms like Linux, I think 7.0 is actually now fairly stable.

That's probably the best course for 4.3, but not an option for 4.2.

	Rainer

-----------------------------------------------------------------------------
Rainer Orth, Faculty of Technology, Bielefeld University

Re: [JAVA] /bin/sh portability issues in scripts/check_jni_methods.sh

[Tom Tromey]

>>>>> "Roger" == Roger Sayle <roger@eyesopen.com> writes:

Roger> This patch, or more likely request for help, is the fourth and final
Roger> in the series to enable mainline GCC to be bootstrapped with the native
Roger> /bin/sh on Solaris/x86.

Roger> Now for the admission.  This final patch, to
Roger> scripts/check_jni_methods.sh is more of a workaround (hack?)
Roger> than a solution to the portability issues.  The problem is that
Roger> this shell script contains the command "set -C" which isn't
Roger> supported by Solaris' sh.

I don't think we need 'set -C' in this script.  I think plenty of
other scripts run during the build create temporary files using a
similar approach.  I'm just going to completely remove the 'set'.

I'll put this in classpath and svn head libjava.

thanks,
Tom