[RFA] JVMTI Agent Loading

[RFA] JVMTI Agent Loading

[Kyle Galloway]

[-- Attachment #1: Type: text/plain, Size: 1144 bytes --]

This patch allows the -agentlib and -agentpath command line options for 
gij.  These are used when loading JVMTI agents, and, in Java1.5 (and 
most notably in eclipse), they are used to launch JDWP as follows:

gij 
-agentlib:jdwp=transport=dt_socket,address=localhost:6969,server=n,suspend=y 
HelloWorld

This patch changes the code in gij.cc to add these options to the 
vm_args, instead of ignoring them, and there is code in prims.cc which 
will find the agent library, using ltdl, and load and (optionally) 
unload it by calling the appropriate functions if they are found.  It 
will also check to see if the agent is jdwp and in that case will setup 
the options and start JDWP in the same way as -Xdebug -Xrunjdwp.  It 
will also report errors if the library is not found, the arguments are 
malformed, or the proper function is not found inside the agent library

ChangeLog
2007-03-28  Kyle Galloway  <kgallowa@redhat.com>
    * gij.cc (main): Accept -agentlib and -agentpath options.
    * prims.cc (parse_init_args): Deal with -agentlib and -agentpath.
    (load_jvmti_agent) New function.

Questions/comments/concerns?

Thanks,
Kyle

[-- Attachment #2: agentlib.patch --]
[-- Type: text/x-patch, Size: 7539 bytes --]

Index: libjava/gij.cc
===================================================================
--- libjava/gij.cc	(revision 123266)
+++ libjava/gij.cc	(working copy)
@@ -121,11 +121,11 @@
         continue;
       else if (! strcmp (arg, "-jrockit"))
         continue;
-      // Ignore JVM Tool Interface options
+      // JVM Tool Interface options
       else if (! strncmp (arg, "-agentlib:", sizeof ("-agentlib:") - 1))
-        continue;
+        add_option (vm_args, arg, NULL);
       else if (! strncmp (arg, "-agentpath:", sizeof ("-agentpath:") - 1))
-        continue;
+        add_option (vm_args, arg, NULL);
       else if (! strcmp (arg, "-classpath") || ! strcmp (arg, "-cp"))
         {
           if (i >= argc - 1)
Index: libjava/prims.cc
===================================================================
--- libjava/prims.cc	(revision 123266)
+++ libjava/prims.cc	(working copy)
@@ -108,6 +108,15 @@
 static char defaultJdwpOptions[] = "";
 static char *jdwpOptions = defaultJdwpOptions;
 
+// Typedefs for JVMTI agent functions
+typedef jint (*jvmti_agent_onload_func) (JavaVM *vm, char *options, void *reserved);
+typedef jint (*jvmti_agent_onunload_func) (JavaVM *vm);
+
+// JVMTI agent functions
+static jvmti_agent_onload_func *jvmti_agentonload = NULL;
+static jvmti_agent_onunload_func *jvmti_agentonunload = NULL;
+static char *jvmti_agent_opts;
+
 // Argument support.
 int
 _Jv_GetNbArgs (void)
@@ -1358,7 +1367,84 @@
   return 0;
 }
 
+// This function loads the agent functions for JVMTI from the library indicated
+// by name.  It returns a negative value on failure, the value of which
+// indicates where ltdl failed, it also prints an error message.
 static jint
+load_jvmti_agent (const char *name)
+{
+#ifdef USE_LTDL
+  lt_dlhandle lib;
+
+  if (lt_dlinit ())
+    {
+      fprintf (stderr, 
+              "libgcj: Error in ltdl init loading agent library\nlibgcj: %s\n",
+               (char *) lt_dlerror ());
+      return -1;
+    }
+  
+  if (!(lib = lt_dlopenext (name)))
+    {
+      fprintf (stderr, 
+               "libgcj: Error opening agent library\nlibgcj: %s\n",
+               (char *) lt_dlerror ());
+      return -2;
+    }
+  
+  if (lib)
+    {
+      // first try to load the unmangled symbol name
+      jvmti_agentonload 
+			  = (jvmti_agent_onload_func *) lt_dlsym (lib, "Agent_OnLoad");
+      
+      // if the unmangled name fails, try the mangled name
+      if (!jvmti_agentonload)
+			  {
+          jvmti_agentonload 
+					  = (jvmti_agent_onload_func *) lt_dlsym (lib,
+                                          "_Z12Agent_OnLoadP10_Jv_JavaVMPcPv");
+				}
+      
+      if (!jvmti_agentonload)
+        {
+          fprintf (stderr, 
+                   "libgcj: Error finding agent function in %s\nlibgcj: %s\n",
+                   name, (char *) lt_dlerror ());
+          lt_dlclose (lib);
+          lib = NULL;
+          return -4;
+        }
+      else
+        {
+          // We have found an Agent_OnLoad function, so search for the optional
+          // Agent_OnUnload
+          jvmti_agentonunload
+					  = (jvmti_agent_onunload_func *) lt_dlsym (lib, "Agent_OnUnload");
+          
+          if (!jvmti_agentonunload)
+					  {
+              jvmti_agentonunload 
+                = (jvmti_agent_onunload_func *) lt_dlsym (lib,
+                                             "_Z14Agent_OnUnloadP7JavaVM_");
+						}
+        	
+          return 0;
+        }
+    }
+  else
+    {
+      fprintf (stderr, "libgcj: %s not found in library path\n", name);
+      return -3;
+    }
+
+#endif /* USE_LTDL */
+
+// If LTDL cannot be used, return an error code indicating this
+return -99;
+}
+
+static jint
 parse_init_args (JvVMInitArgs* vm_args)
 {
   // if _Jv_Compiler_Properties is non-NULL then it needs to be
@@ -1383,6 +1469,7 @@
   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"))
@@ -1410,6 +1497,95 @@
 
 	  continue;
 	}
+	  else if (! strncmp (option_string, "-agentlib", sizeof ("-agentlib") - 1))
+	{
+      char *strPtr;
+	                                              
+      if (strlen(option_string) > 10)
+        strPtr = &option_string[10];
+      else
+        {
+          fprintf (stderr,
+                   "libgcj: Malformed agentlib argument %s: expected lib name\n",
+                   option_string);
+          return -1;
+        }
+
+      // This is optional
+      jvmti_agent_opts = strchr (strPtr, '=');
+   
+      if (! strncmp (strPtr, "jdwp", 4))
+        {    	
+          // We want to run JDWP here so set the variables
+          remoteDebug = true;
+          jdwpOptions = ++jvmti_agent_opts;
+        }
+      else
+        {
+          jint nameLength;
+   
+          if (jvmti_agent_opts == NULL)
+            nameLength = strlen (strPtr);
+          else
+            {
+              nameLength = jvmti_agent_opts - strPtr;
+              jvmti_agent_opts++;
+            }
+               
+          char lib_name[nameLength + 3 + 1];
+          strcpy (lib_name, "lib");
+          strncat (lib_name, strPtr, nameLength);
+      
+          jint result = load_jvmti_agent (lib_name);
+      
+	      if (result < 0)
+	        {
+	          return -1;
+	        }
+        }
+	
+	  continue;
+	}
+      else if (! strncmp (option_string, "-agentpath:", 
+                          sizeof ("-agentpath:") - 1))
+	{
+      char *strPtr;
+	                                              
+      if (strlen(option_string) > 10)
+        strPtr = &option_string[10];
+      else
+        {
+          fprintf (stderr,
+                   "libgcj: Malformed agentlib argument %s: expected lib path\n",
+                   option_string);
+          return -1;
+        }
+		
+      // This is optional
+      jvmti_agent_opts = strchr (strPtr, '=');
+    
+      jint nameLength;
+   
+      if (jvmti_agent_opts == NULL)
+        nameLength = strlen (strPtr);
+      else
+        {
+          nameLength = jvmti_agent_opts - strPtr;
+     	  jvmti_agent_opts++;
+        }
+    
+      char lib_name[nameLength + 3 + 1];
+      strcpy (lib_name, "lib");
+      strncat (lib_name, strPtr, nameLength);
+      jint result = load_jvmti_agent (strPtr);
+
+      if (result < 0)
+        {
+          return -1;
+        }
+	
+      continue;
+	}
       else if (vm_args->ignoreUnrecognized)
         {
           if (option_string[0] == '_')
@@ -1571,13 +1747,17 @@
 				      arg_vec, is_jar);
       _Jv_AttachCurrentThread (main_thread);
 
+      //start JVMTI if an agent function has been found
+      if (jvmti_agentonload)
+        (*jvmti_agentonload) (_Jv_GetJavaVM (), jvmti_agent_opts, NULL);
+
       // Start JDWP
       if (remoteDebug)
 	{
 	  using namespace gnu::classpath::jdwp;
 	  VMVirtualMachine::initialize ();
 	  Jdwp *jdwp = new Jdwp ();
-	  jdwp->setDaemon (true);
+	  jdwp->setDaemon (true);	  
 	  jdwp->configure (JvNewStringLatin1 (jdwpOptions));
 	  jdwp->start ();
 
@@ -1609,6 +1789,10 @@
       JNIEnv *jni_env = _Jv_GetCurrentJNIEnv ();
       _Jv_JVMTI_PostEvent (JVMTI_EVENT_VM_DEATH, thread, jni_env);
     }
+    
+   // Run JVMTI AgentOnUnload if it exists and an agent is loaded
+  if (jvmti_agentonunload)
+    (*jvmti_agentonunload) (_Jv_GetJavaVM ());
 
   // If we got here then something went wrong, as MainThread is not
   // supposed to terminate.

Re: [RFA] JVMTI Agent Loading

[Tom Tromey]

>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:

Kyle> 2007-03-28  Kyle Galloway  <kgallowa@redhat.com>
Kyle>     * gij.cc (main): Accept -agentlib and -agentpath options.
Kyle>     * prims.cc (parse_init_args): Deal with -agentlib and -agentpath.
Kyle>     (load_jvmti_agent) New function.

Kyle> Questions/comments/concerns?

Just a few random nits.

Kyle> +// Typedefs for JVMTI agent functions
Kyle> +typedef jint (*jvmti_agent_onload_func) (JavaVM *vm, char *options, void *reserved);

This is > 79 columns, so there should be a break after the last ",".

Kyle> +      // if the unmangled name fails, try the mangled name

Weird... do we really need this?

Some of the formatting in this function looks wacky but I didn't look
to see if that is just due to the patch format.  Just double-check it.

Kyle> +// If LTDL cannot be used, return an error code indicating this
Kyle> +return -99;

This isn't indented.

Kyle> +	  else if (! strncmp (option_string, "-agentlib", sizeof ("-agentlib") - 1))
Kyle> +	{
Kyle> +      char *strPtr;
Kyle> +	                                              
Kyle> +      if (strlen(option_string) > 10)
Kyle> +        strPtr = &option_string[10];

Instead of 10, maybe sizeof("-agentlib")?

Tom

Re: [RFA] JVMTI Agent Loading

[Kyle Galloway]

Tom Tromey wrote:
>>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:
>>>>>>             
>
> Kyle> 2007-03-28  Kyle Galloway  <kgallowa@redhat.com>
> Kyle>     * gij.cc (main): Accept -agentlib and -agentpath options.
> Kyle>     * prims.cc (parse_init_args): Deal with -agentlib and -agentpath.
> Kyle>     (load_jvmti_agent) New function.
>
> Kyle> Questions/comments/concerns?
>
> Just a few random nits.
>
> Kyle> +// Typedefs for JVMTI agent functions
> Kyle> +typedef jint (*jvmti_agent_onload_func) (JavaVM *vm, char *options, void *reserved);
>
> This is > 79 columns, so there should be a break after the last ",".
>
> Kyle> +      // if the unmangled name fails, try the mangled name
>
> Weird... do we really need this?
>   
The reason for this is that if the library is a C library, it will have 
unmangled names, but if the library is a C++ library the names will be 
mangled.
> Some of the formatting in this function looks wacky but I didn't look
> to see if that is just due to the patch format.  Just double-check it.
>   
I'll make sure I double check the formatting before it goes in.
> Kyle> +	  else if (! strncmp (option_string, "-agentlib", sizeof ("-agentlib") - 1))
> Kyle> +	{
> Kyle> +      char *strPtr;
> Kyle> +	                                              
> Kyle> +      if (strlen(option_string) > 10)
> Kyle> +        strPtr = &option_string[10];
>
> Instead of 10, maybe sizeof("-agentlib")?
>   
Is it better to have sizeof("-agentlib:") - 1 to be more clear about 
what's happening or sizeof(agentlib) because it avoids the subtraction?

- Kyle

Re: [RFA] JVMTI Agent Loading

[Tom Tromey]

>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:

Kyle> The reason for this is that if the library is a C library, it will
Kyle> have unmangled names, but if the library is a C++ library the names
Kyle> will be mangled.

Yeah, but the declaration of Agent_OnLoad and Agent_OnUnload is
wrapped in 'extern "C"'... so I'd assume this case can't happen.

Kyle> Is it better to have sizeof("-agentlib:") - 1 to be more clear about
Kyle> what's happening or sizeof(agentlib) because it avoids the subtraction?

The first one I think.  Don't worry about the subtraction, the
compiler will fold this to a constant.

Tom 

Re: [RFA] JVMTI Agent Loading

[Kyle Galloway]

[-- Attachment #1: Type: text/plain, Size: 967 bytes --]

Tom Tromey wrote:
>>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:
>>>>>>             
>
> Kyle> The reason for this is that if the library is a C library, it will
> Kyle> have unmangled names, but if the library is a C++ library the names
> Kyle> will be mangled.
>
> Yeah, but the declaration of Agent_OnLoad and Agent_OnUnload is
> wrapped in 'extern "C"'... so I'd assume this case can't happen.
>   
I think your right, I could swear I've seen a mangled AgentOnLoad 
before, but I can't reproduce it so I whacked the mangled check.
> Kyle> Is it better to have sizeof("-agentlib:") - 1 to be more clear about
> Kyle> what's happening or sizeof(agentlib) because it avoids the subtraction?
>
> The first one I think.  Don't worry about the subtraction, the
> compiler will fold this to a constant.
>   
I think I have all the changes made now, but I've attached a revised 
patch here in case there are any other issues before I check this in.

- Kyle

[-- Attachment #2: agentlib.patch --]
[-- Type: text/x-patch, Size: 6976 bytes --]

Index: libjava/gij.cc
===================================================================
--- libjava/gij.cc	(revision 123426)
+++ libjava/gij.cc	(working copy)
@@ -121,11 +121,11 @@
         continue;
       else if (! strcmp (arg, "-jrockit"))
         continue;
-      // Ignore JVM Tool Interface options
+      // JVM Tool Interface options
       else if (! strncmp (arg, "-agentlib:", sizeof ("-agentlib:") - 1))
-        continue;
+        add_option (vm_args, arg, NULL);
       else if (! strncmp (arg, "-agentpath:", sizeof ("-agentpath:") - 1))
-        continue;
+        add_option (vm_args, arg, NULL);
       else if (! strcmp (arg, "-classpath") || ! strcmp (arg, "-cp"))
         {
           if (i >= argc - 1)
Index: libjava/prims.cc
===================================================================
--- libjava/prims.cc	(revision 123426)
+++ libjava/prims.cc	(working copy)
@@ -108,6 +108,16 @@
 static char defaultJdwpOptions[] = "";
 static char *jdwpOptions = defaultJdwpOptions;
 
+// Typedefs for JVMTI agent functions
+typedef jint jvmti_agent_onload_func (JavaVM *vm, char *options,
+                                      void *reserved);
+typedef jint jvmti_agent_onunload_func (JavaVM *vm);
+
+// JVMTI agent functions
+static jvmti_agent_onload_func *jvmti_agentonload = NULL;
+static jvmti_agent_onunload_func *jvmti_agentonunload = NULL;
+static char *jvmti_agent_opts;
+
 // Argument support.
 int
 _Jv_GetNbArgs (void)
@@ -1358,7 +1368,65 @@
   return 0;
 }
 
+// This function loads the agent functions for JVMTI from the library indicated
+// by name.  It returns a negative value on failure, the value of which
+// indicates where ltdl failed, it also prints an error message.
 static jint
+load_jvmti_agent (const char *name)
+{
+#ifdef USE_LTDL
+  if (lt_dlinit ())
+    {
+      fprintf (stderr, 
+              "libgcj: Error in ltdl init loading agent library\nlibgcj: %s\n",
+               (char *) lt_dlerror ());
+      return -1;
+    }
+ 
+  lt_dlhandle lib = lt_dlopenext (name);
+  if (!lib)
+    {
+      fprintf (stderr, 
+               "libgcj: Error opening agent library\nlibgcj: %s\n",
+               (char *) lt_dlerror ());
+      return -2;
+    }
+
+  if (lib)
+    {
+      jvmti_agentonload 
+        = (jvmti_agent_onload_func *) lt_dlsym (lib, "Agent_OnLoad");
+ 
+      if (!jvmti_agentonload)
+        {
+          fprintf (stderr, 
+                   "libgcj: Error finding agent function in %s\nlibgcj: %s\n",
+                   name, (char *) lt_dlerror ());
+          lt_dlclose (lib);
+          lib = NULL;
+          return -4;
+        }
+      else
+        {
+          jvmti_agentonunload
+            = (jvmti_agent_onunload_func *) lt_dlsym (lib, "Agent_OnUnload");
+	   
+          return 0;
+        }
+    }
+  else
+    {
+      fprintf (stderr, "libgcj: %s not found in library path\n", name);
+      return -3;
+    }
+
+#endif /* USE_LTDL */
+
+// If LTDL cannot be used, return an error code indicating this
+return -99;
+}
+
+static jint
 parse_init_args (JvVMInitArgs* vm_args)
 {
   // if _Jv_Compiler_Properties is non-NULL then it needs to be
@@ -1383,6 +1451,7 @@
   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"))
@@ -1410,6 +1479,95 @@
 
 	  continue;
 	}
+      else if (! strncmp (option_string, "-agentlib", sizeof ("-agentlib") - 1))
+	{
+          char *strPtr;
+	                                              
+          if (strlen(option_string) > (sizeof ("-agentlib:") - 1))
+            strPtr = &option_string[sizeof ("-agentlib:") - 1];
+          else
+            {
+              fprintf (stderr,
+                "libgcj: Malformed agentlib argument %s: expected lib name\n",
+                option_string);
+              return -1;
+            }
+
+          // This is optional
+          jvmti_agent_opts = strchr (strPtr, '=');
+   
+          if (! strncmp (strPtr, "jdwp", 4))
+            {    	
+              // We want to run JDWP here so set the variables
+              remoteDebug = true;
+              jdwpOptions = ++jvmti_agent_opts;
+            }
+          else
+            {
+              jint nameLength;
+   
+              if (jvmti_agent_opts == NULL)
+                nameLength = strlen (strPtr);
+              else
+                {
+                  nameLength = jvmti_agent_opts - strPtr;
+                  jvmti_agent_opts++;
+                }
+               
+              char lib_name[nameLength + 3 + 1];
+              strcpy (lib_name, "lib");
+              strncat (lib_name, strPtr, nameLength);
+      
+              jint result = load_jvmti_agent (lib_name);
+      
+              if (result < 0)
+	        {
+	          return -1;
+	        }
+            }
+    
+          continue;
+	}
+      else if (! strncmp (option_string, "-agentpath:", 
+                          sizeof ("-agentpath:") - 1))
+	{
+          char *strPtr;
+	                                              
+          if (strlen(option_string) > 10)
+            strPtr = &option_string[10];
+          else
+            {
+              fprintf (stderr,
+                "libgcj: Malformed agentlib argument %s: expected lib path\n",
+                option_string);
+              return -1;
+            }
+		
+          // This is optional
+          jvmti_agent_opts = strchr (strPtr, '=');
+    
+          jint nameLength;
+   
+          if (jvmti_agent_opts == NULL)
+            nameLength = strlen (strPtr);
+          else
+            {
+              nameLength = jvmti_agent_opts - strPtr;
+              jvmti_agent_opts++;
+            }
+    
+          char lib_name[nameLength + 3 + 1];
+          strcpy (lib_name, "lib");
+          strncat (lib_name, strPtr, nameLength);
+          jint result = load_jvmti_agent (strPtr);
+
+          if (result < 0)
+            {
+              return -1;
+            }
+	
+          continue;
+	}
       else if (vm_args->ignoreUnrecognized)
         {
           if (option_string[0] == '_')
@@ -1570,6 +1728,10 @@
 	main_thread = new MainThread (JvNewStringUTF (name),
 				      arg_vec, is_jar);
       _Jv_AttachCurrentThread (main_thread);
+      
+      //start JVMTI if an agent function has been found
+      if (jvmti_agentonload)
+        (*jvmti_agentonload) (_Jv_GetJavaVM (), jvmti_agent_opts, NULL);
 
       // Start JDWP
       if (remoteDebug)
@@ -1609,6 +1770,10 @@
       JNIEnv *jni_env = _Jv_GetCurrentJNIEnv ();
       _Jv_JVMTI_PostEvent (JVMTI_EVENT_VM_DEATH, thread, jni_env);
     }
+    
+   // Run JVMTI AgentOnUnload if it exists and an agent is loaded
+  if (jvmti_agentonunload)
+    (*jvmti_agentonunload) (_Jv_GetJavaVM ());
 
   // If we got here then something went wrong, as MainThread is not
   // supposed to terminate.

Re: [RFA] JVMTI Agent Loading

[Andrew Haley]

Kyle Galloway writes:

 > Index: libjava/gij.cc
 > ===================================================================
 > --- libjava/gij.cc	(revision 123426)
 > +++ libjava/gij.cc	(working copy)
 > @@ -121,11 +121,11 @@
 >          continue;
 >        else if (! strcmp (arg, "-jrockit"))
 >          continue;
 > -      // Ignore JVM Tool Interface options
 > +      // JVM Tool Interface options
 >        else if (! strncmp (arg, "-agentlib:", sizeof ("-agentlib:") - 1))

FYI -- and not theis is not an objection to your patch --
strlen("-agentlib:") would be more idiomatic and just as efficient.

"sizeof is faster than strlen()" is one of those urban legends that
will never die...

Andrew.

-- 
Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, UK
Registered in England and Wales No. 3798903

Re: [RFA] JVMTI Agent Loading

[Tom Tromey]

>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:

Kyle> I think I have all the changes made now, but I've attached a revised
Kyle> patch here in case there are any other issues before I check this in.

Just a couple little nits, please fix them & commit.
Thanks.

Kyle> +// If LTDL cannot be used, return an error code indicating this
Kyle> +return -99;

This is still not indented correctly.

Also, comments in libgcj are (or ought to be :-) full sentences
starting with a capital letter (unless there's a good reason why not)
and ending in a period.  Most of the comments being added need a
little tweak here.

Tom