re PR libgcj/16923 (-D* Options passed to JNI_CreateJavaVM are ignored)

2005-02-23  Thomas Fitzsimmons  <fitzsim@redhat.com>

	PR libgcj/16923
	* gcj.texi (Invocation): Add descriptions of JvVMInitArgs and
	JvVMOption.

2005-02-23  Thomas Fitzsimmons  <fitzsim@redhat.com>

	PR libgcj/16923
	* jni.cc (JNI_CreateJavaVM): Check JNI version.  Cast args to
	JvVMInitArgs.  Pass args to _Jv_CreateJavaVM and check return
	value.  Move argument parsing code to prims.cc.
	* prims.cc (no_properties): Remove.
	(_Jv_Compiler_Properties): Initialize to NULL.
	(_Jv_Properties_Count): Initialize to 0.
	(parse_verbose_args): New function.
	(parse_init_args): New function.
	(_Jv_CreateJavaVM): Call parse_init_args.
	(_Jv_RunMain): Check return value of _Jv_CreateJavaVM.
	* gcj/cni.h (JvVMOption): New struct.
	(JvVMInitArgs): Likewise.
	(JvCreateJavaVM): Declare vm_args as JvVMInitArgs* rather than
	void*.
	* libjava/gcj/javaprims.h (_Jv_VMOption): New struct.
	(_Jv_VMInitArgs): Likewise.
	* include/java-props.h (_Jv_Properties_Count): Declare.
	* java/lang/natRuntime.cc (insertSystemProperties): Use
	_Jv_Properties_Count in for loop exit condition.
	* testsuite/libjava.jni/jni.exp
	(gcj_invocation_compile_c_to_binary): New procedure.
	(gcj_invocation_test_one): Likewise.
	(gcj_jni_run): Run JNI invocation API tests.
	* testsuite/libjava.jni/invocation/PR16923.c,
	testsuite/libjava.jni/invocation/PR16923.java,
	testsuite/libjava.jni/invocation/PR16923.out: New test.

From-SVN: r95459

11 files changed, 383 insertions, 52 deletions

diff --git a/libjava/ChangeLog b/libjava/ChangeLog
index 7b85ad9..b12241c 100644
--- a/libjava/ChangeLog
+++ b/libjava/ChangeLog
@@ -1,3 +1,33 @@
+2005-02-23  Thomas Fitzsimmons  <fitzsim@redhat.com>
+
+	PR libgcj/16923
+	* jni.cc (JNI_CreateJavaVM): Check JNI version.  Cast args to
+	JvVMInitArgs.  Pass args to _Jv_CreateJavaVM and check return
+	value.  Move argument parsing code to prims.cc.
+	* prims.cc (no_properties): Remove.
+	(_Jv_Compiler_Properties): Initialize to NULL.
+	(_Jv_Properties_Count): Initialize to 0.
+	(parse_verbose_args): New function.
+	(parse_init_args): New function.
+	(_Jv_CreateJavaVM): Call parse_init_args.
+	(_Jv_RunMain): Check return value of _Jv_CreateJavaVM.
+	* gcj/cni.h (JvVMOption): New struct.
+	(JvVMInitArgs): Likewise.
+	(JvCreateJavaVM): Declare vm_args as JvVMInitArgs* rather than
+	void*.
+	* libjava/gcj/javaprims.h (_Jv_VMOption): New struct.
+	(_Jv_VMInitArgs): Likewise.
+	* include/java-props.h (_Jv_Properties_Count): Declare.
+	* java/lang/natRuntime.cc (insertSystemProperties): Use
+	_Jv_Properties_Count in for loop exit condition.
+	* testsuite/libjava.jni/jni.exp
+	(gcj_invocation_compile_c_to_binary): New procedure.
+	(gcj_invocation_test_one): Likewise.
+	(gcj_jni_run): Run JNI invocation API tests.
+	* testsuite/libjava.jni/invocation/PR16923.c,
+	testsuite/libjava.jni/invocation/PR16923.java,
+	testsuite/libjava.jni/invocation/PR16923.out: New test.
+
 2005-02-23  Michael Koch  <konqueror@gmx.de>
 
 	* Makefile.am: Added new file gnu/java/nio/ChannelReader.java.
diff --git a/libjava/gcj/cni.h b/libjava/gcj/cni.h
index ee39738..4aea4b6 100644
--- a/libjava/gcj/cni.h
+++ b/libjava/gcj/cni.h
@@ -17,6 +17,7 @@ details.  */
 #include <java/lang/Class.h>
 
 #include <gcj/array.h>
+#include <gcj/javaprims.h>
 
 #include <string.h>
 
@@ -113,8 +114,11 @@ JvFree (void *ptr)
   return _Jv_Free (ptr);
 }
 
+typedef struct _Jv_VMOption JvVMOption;
+typedef struct _Jv_VMInitArgs JvVMInitArgs;
+
 extern inline jint
-JvCreateJavaVM (void* vm_args)
+JvCreateJavaVM (JvVMInitArgs* vm_args)
 {
   return _Jv_CreateJavaVM (vm_args);
 }
diff --git a/libjava/gcj/javaprims.h b/libjava/gcj/javaprims.h
index 53c26fe..e972f7b 100644
--- a/libjava/gcj/javaprims.h
+++ b/libjava/gcj/javaprims.h
@@ -487,7 +487,30 @@ extern "C" jsize _Jv_GetStringUTFLength (jstring);
 extern "C" jsize _Jv_GetStringUTFRegion (jstring, jsize, jsize, char *);
 extern "C" jint _Jv_hashUtf8String (char*, int);
 
-extern jint _Jv_CreateJavaVM (void* /*vm_args*/);
+struct _Jv_VMOption
+{
+  // a VM initialization option
+  char* optionString;
+  // extra information associated with this option
+  void* extraInfo;
+};
+
+struct _Jv_VMInitArgs
+{
+  // for compatibility with JavaVMInitArgs
+  jint version;
+
+  // number of VM initialization options
+  jint nOptions;
+
+  // an array of VM initialization options
+  struct _Jv_VMOption* options;
+
+  // true if the option parser should ignore unrecognized options
+  jboolean ignoreUnrecognized;
+};
+
+extern jint _Jv_CreateJavaVM (struct _Jv_VMInitArgs*);
 
 void
 _Jv_ThreadRun (java::lang::Thread* thread);
diff --git a/libjava/include/java-props.h b/libjava/include/java-props.h
index 9be3099..19d7106 100644
--- a/libjava/include/java-props.h
+++ b/libjava/include/java-props.h
@@ -21,6 +21,7 @@ typedef struct
 
 // Set to NULL-terminated list of properties set at compile time.
 extern const char **_Jv_Compiler_Properties;
+extern int _Jv_Properties_Count;
 
 // The JAR file to add to the beginning of java.class.path.
 extern const char *_Jv_Jar_Class_Path;
diff --git a/libjava/java/lang/natRuntime.cc b/libjava/java/lang/natRuntime.cc
index 37d7b4b..31f9b90 100644
--- a/libjava/java/lang/natRuntime.cc
+++ b/libjava/java/lang/natRuntime.cc
@@ -557,7 +557,7 @@ java::lang::Runtime::insertSystemProperties (java::util::Properties *newprops)
   // `-D'.  Important: after this point, the only properties that
   // should be set are those which either the user cannot meaningfully
   // override, or which augment whatever value the user has provided.
-  for (int i = 0; _Jv_Compiler_Properties[i]; ++i)
+  for (int i = 0; i < _Jv_Properties_Count; ++i)
     {
       const char *s, *p;
       // Find the `='.
diff --git a/libjava/jni.cc b/libjava/jni.cc
index b7c2084..5f9d5f7 100644
--- a/libjava/jni.cc
+++ b/libjava/jni.cc
@@ -2498,7 +2498,16 @@ JNI_CreateJavaVM (JavaVM **vm, void **penv, void *args)
 {
   JvAssert (! the_vm);
 
-  _Jv_CreateJavaVM (NULL);
+  jint version = * (jint *) args;
+  // We only support 1.2 and 1.4.
+  if (version != JNI_VERSION_1_2 && version != JNI_VERSION_1_4)
+    return JNI_EVERSION;
+
+  JvVMInitArgs* vm_args = reinterpret_cast<JvVMInitArgs *> (args);
+
+  jint result = _Jv_CreateJavaVM (vm_args);
+  if (result)
+    return result;
 
   // FIXME: synchronize
   JavaVM *nvm = (JavaVM *) _Jv_MallocUnchecked (sizeof (JavaVM));
@@ -2506,48 +2515,6 @@ JNI_CreateJavaVM (JavaVM **vm, void **penv, void *args)
     return JNI_ERR;
   nvm->functions = &_Jv_JNI_InvokeFunctions;
 
-  // Parse the arguments.
-  if (args != NULL)
-    {
-      jint version = * (jint *) args;
-      // We only support 1.2 and 1.4.
-      if (version != JNI_VERSION_1_2 && version != JNI_VERSION_1_4)
-	return JNI_EVERSION;
-      JavaVMInitArgs *ia = reinterpret_cast<JavaVMInitArgs *> (args);
-      for (int i = 0; i < ia->nOptions; ++i)
-	{
-	  if (! strcmp (ia->options[i].optionString, "vfprintf")
-	      || ! strcmp (ia->options[i].optionString, "exit")
-	      || ! strcmp (ia->options[i].optionString, "abort"))
-	    {
-	      // We are required to recognize these, but for now we
-	      // don't handle them in any way.  FIXME.
-	      continue;
-	    }
-	  else if (! strncmp (ia->options[i].optionString,
-			      "-verbose", sizeof ("-verbose") - 1))
-	    {
-	      // We don't do anything with this option either.  We
-	      // might want to make sure the argument is valid, but we
-	      // don't really care all that much for now.
-	      continue;
-	    }
-	  else if (! strncmp (ia->options[i].optionString, "-D", 2))
-	    {
-	      // FIXME.
-	      continue;
-	    }
-	  else if (ia->ignoreUnrecognized)
-	    {
-	      if (ia->options[i].optionString[0] == '_'
-		  || ! strncmp (ia->options[i].optionString, "-X", 2))
-		continue;
-	    }
-
-	  return JNI_ERR;
-	}
-    }
-
   jint r =_Jv_JNI_AttachCurrentThread (nvm, penv, NULL);
   if (r < 0)
     return r;
diff --git a/libjava/prims.cc b/libjava/prims.cc
index d12a242..cf0fed1 100644
--- a/libjava/prims.cc
+++ b/libjava/prims.cc
@@ -80,10 +80,9 @@ static java::lang::OutOfMemoryError *no_memory;
 // functions are changed to take a size_t argument instead of jint.
 #define MAX_OBJECT_SIZE ((1<<31) - 1)
 
-static const char *no_properties[] = { NULL };
-
 // Properties set at compile time.
-const char **_Jv_Compiler_Properties = no_properties;
+const char **_Jv_Compiler_Properties = NULL;
+int _Jv_Properties_Count = 0;
 
 // The JAR file to add to the beginning of java.class.path.
 const char *_Jv_Jar_Class_Path;
@@ -909,16 +908,181 @@ namespace gcj
   bool runtimeInitialized = false;
 }
 
+static jint
+parse_verbose_args (char* option_string,
+		    bool ignore_unrecognized)
+{
+  size_t len = sizeof ("-verbose");
+
+  if (strlen (option_string) < len)
+    return -1;
+
+  if (option_string[len] == ':'
+      && option_string[len + 1] != '\0')
+    {
+      char* verbose_args = option_string + len + 1;
+      size_t last = 0;
+
+      do
+	{
+	  if (! strncmp (verbose_args,
+			 "gc", (last = sizeof ("gc")) - 1)
+	      && (verbose_args[last] == '\0'
+		  || verbose_args[last] == ','))
+	    {
+	      // FIXME: we should add functions to boehm-gc that
+	      // toggle GC_print_stats, GC_PRINT_ADDRESS_MAP and
+	      // GC_print_back_height.
+
+	    }
+	  else if (! strncmp (verbose_args,
+			      "class",
+			      (last = sizeof ("class")) - 1)
+		   && (verbose_args[last] == '\0'
+		       || verbose_args[last] == ','))
+	    {
+	      gcj::verbose_class_flag = true;
+	    }
+	  else if (! strncmp (verbose_args, "jni",
+			      (last = sizeof ("jni")) - 1)
+		   && (verbose_args[last] == '\0'
+		       || verbose_args[last] == ','))
+	    {
+	      // FIXME: enable JNI messages.
+	    }
+	  else if (ignore_unrecognized
+		   && verbose_args[0] == 'X')
+	    {
+	      // ignore unrecognized non-standard verbose option
+	      last = 0;
+	      while (verbose_args[last] != '\0'
+		     && verbose_args[last++] != ',');
+	    }
+
+	  if (strlen (verbose_args) >= last)
+	    {
+	      if (verbose_args[last] == ',')
+		{
+		  if (verbose_args[last + 1] == '\0')
+		    // trailing comma
+		    return -1;
+		  else
+		    {
+		      verbose_args = verbose_args + last + 1;
+		      last = 0;
+		    }
+		}
+	      // here verbose_args[last] is either '\0' or
+	      // the first character in the next verbose
+	      // argument.
+	    }
+	  else
+	    // partial option
+	    return -1;
+
+	  // verbose_args[last] will be '\0' here if we're
+	  // done.
+	}
+      while (verbose_args[last] != '\0');
+    }
+  else if (option_string[len] == 'g'
+	   && option_string[len + 1] == 'c'
+	   && option_string[len + 2] == '\0')
+    {
+      // FIXME: we should add functions to boehm-gc that
+      // toggle GC_print_stats, GC_PRINT_ADDRESS_MAP and
+      // GC_print_back_height.
+      return 0;
+    }
+  else if (option_string[len] == '\0')
+    {
+      gcj::verbose_class_flag = true;
+      return 0;
+    }
+  else
+    {
+      // unrecognized option beginning with -verbose
+      return -1;
+    }
+  return 0;
+}
+
+static jint
+parse_init_args (JvVMInitArgs* vm_args)
+{
+  // if _Jv_Compiler_Properties is non-NULL then it needs to be
+  // re-allocated dynamically.
+  if (_Jv_Compiler_Properties)
+    {
+      const char** props = _Jv_Compiler_Properties;
+      _Jv_Compiler_Properties = NULL;
+
+      for (int i = 0; props[i]; i++)
+	{
+	  _Jv_Compiler_Properties = (const char**) _Jv_Realloc
+	    (_Jv_Compiler_Properties,
+	     (_Jv_Properties_Count + 1) * sizeof (const char*));
+	  _Jv_Compiler_Properties[_Jv_Properties_Count++] = props[i];
+	}
+    }
+
+  if (vm_args == NULL)
+    return 0;
+
+  for (int i = 0; i < vm_args->nOptions; ++i)
+    {
+      char* option_string = vm_args->options[i].optionString;
+      if (! strcmp (option_string, "vfprintf")
+	  || ! strcmp (option_string, "exit")
+	  || ! strcmp (option_string, "abort"))
+	{
+	  // FIXME: we are required to recognize these, but for
+	  // now we don't handle them in any way.
+	  continue;
+	}
+      else if (! strncmp (option_string,
+			  "-verbose", sizeof ("-verbose") - 1))
+	{
+	  jint result = parse_verbose_args (option_string,
+					    vm_args->ignoreUnrecognized);
+	  if (result < 0)
+	    return result;
+	}
+      else if (! strncmp (option_string, "-D", 2))
+	{
+	  _Jv_Compiler_Properties = (const char**) _Jv_Realloc
+	    (_Jv_Compiler_Properties,
+	     (_Jv_Properties_Count + 1) * sizeof (char*));
+
+	  _Jv_Compiler_Properties[_Jv_Properties_Count++] =
+	    strdup (option_string + 2);
+
+	  continue;
+	}
+      else if (vm_args->ignoreUnrecognized)
+	{
+	  if (option_string[0] == '_'
+	      || ! strncmp (option_string, "-X", 2))
+	    continue;
+	}
+    }
+  return 0;
+}
+
 jint
-_Jv_CreateJavaVM (void* /*vm_args*/)
+_Jv_CreateJavaVM (JvVMInitArgs* vm_args)
 {
   using namespace gcj;
-  
+
   if (runtimeInitialized)
     return -1;
 
   runtimeInitialized = true;
 
+  jint result = parse_init_args (vm_args);
+  if (result < 0)
+    return -1;
+
   PROCESS_GCJ_PROPERTIES;
 
   /* Threads must be initialized before the GC, so that it inherits the
@@ -1016,7 +1180,12 @@ _Jv_RunMain (jclass klass, const char *name, int argc, const char **argv,
       // is initialized.
       if (is_jar)
 	_Jv_Jar_Class_Path = strdup (name);
-      _Jv_CreateJavaVM (NULL);
+
+      if (_Jv_CreateJavaVM (NULL) < 0)
+	{
+	  fprintf (stderr, "libgcj: couldn't create virtual machine\n");
+	  exit (1);
+	}
 
       // Get the Runtime here.  We want to initialize it before searching
       // for `main'; that way it will be set up if `main' is a JNI method.
diff --git a/libjava/testsuite/libjava.jni/invocation/PR16923.c b/libjava/testsuite/libjava.jni/invocation/PR16923.c
new file mode 100644
index 0000000..881738b
--- /dev/null
+++ b/libjava/testsuite/libjava.jni/invocation/PR16923.c
@@ -0,0 +1,43 @@
+#include <assert.h>
+#include <jni.h>
+
+union env_union
+{
+  void *void_env;
+  JNIEnv *jni_env;
+};
+
+int
+main (int argc, const char** argv)
+{
+  union env_union tmp;
+  JNIEnv* env;
+  JavaVM* jvm;
+  JavaVMInitArgs vm_args;
+  JavaVMOption options[1];
+  jclass class_id;
+  jmethodID method_id;
+  jint result;
+
+  options[0].optionString = "-DPR16923=optionReceived";
+
+  vm_args.version = JNI_VERSION_1_2;
+  vm_args.ignoreUnrecognized = JNI_TRUE;
+  vm_args.options = options;
+  vm_args.nOptions = 1;
+
+  result = JNI_CreateJavaVM (&jvm, &tmp.void_env, &vm_args);
+  assert (result >= 0);
+
+  env = tmp.jni_env;
+
+  class_id = (*env)->FindClass (env, "PR16923");
+  assert (class_id);
+
+  method_id = (*env)->GetStaticMethodID (env, class_id, "printIt", "()V");
+  assert (method_id);
+
+  (*env)->CallStaticVoidMethod (env, class_id, method_id, NULL);
+
+  return 0;
+}
diff --git a/libjava/testsuite/libjava.jni/invocation/PR16923.java b/libjava/testsuite/libjava.jni/invocation/PR16923.java
new file mode 100644
index 0000000..efda4bd
--- /dev/null
+++ b/libjava/testsuite/libjava.jni/invocation/PR16923.java
@@ -0,0 +1,7 @@
+public class PR16923
+{
+  public static void printIt ()
+  {
+    System.out.println (System.getProperty ("PR16923"));
+  }
+}
diff --git a/libjava/testsuite/libjava.jni/invocation/PR16923.out b/libjava/testsuite/libjava.jni/invocation/PR16923.out
new file mode 100644
index 0000000..58bf3fe
--- /dev/null
+++ b/libjava/testsuite/libjava.jni/invocation/PR16923.out
@@ -0,0 +1 @@
+optionReceived
diff --git a/libjava/testsuite/libjava.jni/jni.exp b/libjava/testsuite/libjava.jni/jni.exp
index cbd90ca..d736b86 100644
--- a/libjava/testsuite/libjava.jni/jni.exp
+++ b/libjava/testsuite/libjava.jni/jni.exp
@@ -181,6 +181,85 @@ proc gcj_jni_test_one {file} {
   return 1
 }
 
+# Compile a single C file and produce a binary.  OPTIONS is a list of
+# options to pass to the compiler.  Returns 0 on failure, 1 on
+# success.
+proc gcj_jni_invocation_compile_c_to_binary {file {options {}}} {
+  global srcdir
+  global host_triplet
+  verbose "options: $options"
+  set options_cxx $options
+  set options ""
+
+  set filename [file tail $file]
+  set name [file rootname $filename]
+
+  # Find jni.h.
+  lappend options "additional_flags=-I$srcdir/../include"
+
+  # Append C++ options
+  lappend options "additional_flags=$options_cxx"
+
+  set x [libjava_prune_warnings \
+	   [target_compile $file $name executable $options]]
+  if {$x != ""} {
+    verbose "target_compile failed: $x" 2
+    fail "$filename compilation"
+    return 0
+  }
+
+  pass "$filename compilation"
+  return 1
+}
+
+# Do all the work for a single invocation API test.  Return 0 on
+# failure.
+proc gcj_jni_invocation_test_one {file} {
+  global runtests
+  global host_triplet
+  global INTERPRETER
+
+  # The base name.  We use it for several purposes.
+  set main [file rootname [file tail $file]]
+  if {! [runtest_file_p $runtests $main]} {
+    # Simply skip it.
+    return 1
+  }
+
+  if {! [bytecompile_file $file [pwd]]} {
+    fail "bytecompile $file"
+    # FIXME - should use `untested' on all remaining tests.
+    # But that is hard.
+    return 0
+  }
+  pass "bytecompile $file"
+
+  set cfile [file rootname $file].c
+  set cxxflags "-lgcj"
+
+  if {! [gcj_jni_invocation_compile_c_to_binary $cfile $cxxflags]} {
+    # FIXME
+    return 0
+  }
+
+  set resultfile [file rootname $file].out
+
+  if {! [gcj_invoke $main $resultfile ""]} {
+    # FIXME
+    return 0
+  }
+
+  # We purposely ignore errors here; we still want to run the other
+  # appropriate tests.
+  set errname [file rootname [file tail $file]]
+
+  # When we succeed we remove all our clutter.
+  eval gcj_cleanup [glob -nocomplain -- ${main}.*] \
+    [list $main]
+
+  return 1
+}
+
 # Run the JNI tests.
 proc gcj_jni_run {} {
   global srcdir subdir
@@ -193,6 +272,13 @@ proc gcj_jni_run {} {
     foreach x $srcfiles {
       gcj_jni_test_one $x
     }
+
+    # Run JNI invocation API tests
+    catch { lsort [glob -nocomplain ${srcdir}/${subdir}/invocation/*.java] } srcfiles
+
+    foreach x $srcfiles {
+      gcj_jni_invocation_test_one $x
+    }
   } else {
     verbose "JNI tests not run in cross-compilation environment"
   }