Re: libgcj/7568: Runtime.exec ignores directory argument

Re: libgcj/7568: Runtime.exec ignores directory argument

[Tom Tromey]

The following reply was made to PR libgcj/7568; it has been noted by GNATS.

From: Tom Tromey <tromey@redhat.com>
To: jmr@ugcs.caltech.edu
Cc: gcc-gnats@gcc.gnu.org
Subject: Re: libgcj/7568: Runtime.exec ignores directory argument
Date: 13 Aug 2002 18:09:09 -0600

 >>>>> "Jesse" == Jesse Rosenstock <jmr@ugcs.caltech.edu> writes:
 
 Jesse> Perhaps I'm missing something.  If it were really this easy, it
 Jesse> would probably have been fixed before.
 
 No, it really is that easy.  It's just that nobody has bothered to do
 the updates since Runtime was changed around.
 
 I think this patch is fine.  I don't think there is any problem
 allocating memory in the child after a fork (with vfork there might be
 a problem, but we explicitly don't do that).
 
 I'm testing this and will check it in once it is done.
 I'm simultaneously putting in the PR 7570 patch.
 
 
 I think you've reached the limit of freebie patches :-(.
 After this, we'll have to get paperwork from you.  Do you know about
 this process?  (Maybe you already have the paperwork and I just don't
 know or have forgotten?)
 
 Thanks,
 Tom

Re: libgcj/7568: Runtime.exec ignores directory argument

[tromey]

Synopsis: Runtime.exec ignores directory argument

Responsible-Changed-From-To: unassigned->tromey
Responsible-Changed-By: tromey
Responsible-Changed-When: Tue Aug 13 18:01:58 2002
Responsible-Changed-Why:
    I'm handling this
State-Changed-From-To: open->closed
State-Changed-By: tromey
State-Changed-When: Tue Aug 13 18:01:58 2002
State-Changed-Why:
    I'll check in the patch shortly.
    Thanks.

http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=7568

Re: libgcj/7568: Runtime.exec ignores directory argument

[Jesse Rosenstock]

The following reply was made to PR libgcj/7568; it has been noted by GNATS.

From: jmr@ugcs.caltech.edu (Jesse Rosenstock)
To: Andrew Pinski <pinskia@physics.uc.edu>
Cc: gcc-gnats@gcc.gnu.org
Subject: Re: libgcj/7568: Runtime.exec ignores directory argument 
Date: Sat, 10 Aug 2002 14:05:44 -0700

 pinskia@physics.uc.edu's message dated: Sat, 10 Aug 2002 16:39:37 EDT
 >Yes you did forget something, this is wrong when the process 
 >already is multithreaded,
 >it creates memory which I think violates POSIX for what is able 
 >to do after forking.
 >Get the string before forking would be better anyway.
 
 OK, take 2.
 
 Index: java/lang/natPosixProcess.cc
 ===================================================================
 RCS file: /cvsroot/gcc/gcc/libjava/java/lang/natPosixProcess.cc,v
 retrieving revision 1.12
 diff -c -r1.12 natPosixProcess.cc
 *** java/lang/natPosixProcess.cc	6 Mar 2002 05:13:58 -0000	1.12
 --- java/lang/natPosixProcess.cc	10 Aug 2002 21:00:24 -0000
 ***************
 *** 30,35 ****
 --- 30,36 ----
   #include <java/lang/InterruptedException.h>
   #include <java/lang/NullPointerException.h>
   #include <java/lang/Thread.h>
 + #include <java/io/File.h>
   #include <java/io/FileDescriptor.h>
   #include <java/io/FileInputStream.h>
   #include <java/io/FileOutputStream.h>
 ***************
 *** 87,93 ****
   }
   
   static void
 ! cleanup (char **args, char **env)
   {
     if (args != NULL)
       {
 --- 88,94 ----
   }
   
   static void
 ! cleanup (char **args, char **env, char *path)
   {
     if (args != NULL)
       {
 ***************
 *** 101,106 ****
 --- 102,109 ----
   	_Jv_Free (env[i]);
         _Jv_Free (env);
       }
 +   if (path != NULL)
 +     _Jv_Free (path);
   }
   
   // This makes our error handling a bit simpler and it lets us avoid
 ***************
 *** 116,122 ****
   
   void
   java::lang::ConcreteProcess::startProcess (jstringArray progarray,
 ! 					   jstringArray envp)
   {
     using namespace java::io;
   
 --- 119,126 ----
   
   void
   java::lang::ConcreteProcess::startProcess (jstringArray progarray,
 ! 					   jstringArray envp,
 ! 					   java::io::File *dir)
   {
     using namespace java::io;
   
 ***************
 *** 125,130 ****
 --- 129,135 ----
     // Initialize all locals here to make cleanup simpler.
     char **args = NULL;
     char **env = NULL;
 +   char *path = NULL;
     int inp[2], outp[2], errp[2], msgp[2];
     inp[0] = -1;
     inp[1] = -1;
 ***************
 *** 168,173 ****
 --- 173,181 ----
   	  env[envp->length] = NULL;
   	}
   
 +       if (dir != NULL)
 + 	path = new_string (dir->getPath ());
 + 
         // Create pipes for I/O.  MSGP is for communicating exec()
         // status.
         if (pipe (inp) || pipe (outp) || pipe (errp) || pipe (msgp)
 ***************
 *** 188,194 ****
   
         if (pid == 0)
   	{
 ! 	  // Child process, so remap descriptors and exec.
   
   	  if (envp)
   	    {
 --- 196,202 ----
   
         if (pid == 0)
   	{
 ! 	  // Child process, so remap descriptors, chdir and exec.
   
   	  if (envp)
   	    {
 ***************
 *** 229,234 ****
 --- 237,251 ----
   	  close (outp[0]);
   	  close (outp[1]);
   	  close (msgp[0]);
 +           
 + 	  // Change directory
 + 	  if (path != NULL)
 + 	    if (chdir (path) != 0)
 + 	      {
 + 		char c = errno;
 + 		write (msgp[1], &c, 1);
 + 		_exit (127);
 + 	      }
   
   	  execvp (args[0], args);
   
 ***************
 *** 304,310 ****
       }
   
     myclose (msgp[0]);
 !   cleanup (args, env);
   
     if (exc != NULL)
       throw exc;
 --- 321,327 ----
       }
   
     myclose (msgp[0]);
 !   cleanup (args, env, path);
   
     if (exc != NULL)
       throw exc;

Re: libgcj/7568: Runtime.exec ignores directory argument

[Andrew Pinski]

The following reply was made to PR libgcj/7568; it has been noted by GNATS.

From: Andrew Pinski <pinskia@physics.uc.edu>
To: jmr@ugcs.caltech.edu
Cc: gcc-gnats@gcc.gnu.org
Subject: Re: libgcj/7568: Runtime.exec ignores directory argument
Date: Sat, 10 Aug 2002 16:39:37 -0400

 Yes you did forget something, this is wrong when the process 
 already is multithreaded,
 it creates memory which I think violates POSIX for what is able 
 to do after forking.
 Get the string before forking would be better anyway.
 
 
 Thanks,
 Andrew Pinski
 
 
 On Saturday, August 10, 2002, at 04:21 , jmr@ugcs.caltech.edu wrote:
 
 >
 > +
 > + 	  // Change directory
 > + 	  if (dir != NULL)
 > + 	    if (chdir (new_string (dir->getPath ())) != 0)
 > + 	      {
 > + 		char c = errno;
 > + 		write (msgp[1], &c, 1);
 > + 		_exit (127);
 > + 	      }
 

libgcj/7568: Runtime.exec ignores directory argument

[jmr]

>Number:         7568
>Category:       libgcj
>Synopsis:       Runtime.exec ignores directory argument
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    unassigned
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Aug 10 13:26:00 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator:     Jesse Rosenstock
>Release:        3.3 20020810 (experimental)
>Organization:
>Environment:
System: Linux churchill 2.4.3-12 #1 Fri Jun 8 15:05:56 EDT 2001 i686 unknown
Architecture: i686

	
host: i686-pc-linux-gnu
build: i686-pc-linux-gnu
target: i686-pc-linux-gnu
configured with: ../gcc/configure --enable-languages=c++,java --enable-threads=posix --enable-shared --prefix=/scratch/app/gcc --enable-hash-synchronization : (reconfigured) 
>Description:
	Runtime.exec ignores the File argument, which specifies the
        directory to run the command in.
>How-To-Repeat:

        ExecDir2.java:

import java.io.File;

public class ExecDir2 {
    private static void runPwdInDir(String dir) throws Exception {
        Process proc =
            Runtime.getRuntime().exec("/bin/pwd", null,
                    dir != null ? new File(dir) : null);
        int status = proc.waitFor();

        byte[] buf = new byte[proc.getInputStream().available()];
        proc.getInputStream().read(buf, 0, buf.length);

        System.out.write(buf, 0, buf.length);
    }

    public static void main(String[] args) throws Exception {
        runPwdInDir("/");
    }
}

; javac ExecDir2.java
; /bin/pwd 
/home/user/jmr/gcj_test/exec_dir
; gij ExecDir2
/home/user/jmr/gcj_test/exec_dir
; java ExecDir2
/

>Fix:

Perhaps I'm missing something.  If it were really this easy, it would
probably have been fixed before.

Index: java/lang/EcosProcess.java
===================================================================
RCS file: /cvsroot/gcc/gcc/libjava/java/lang/EcosProcess.java,v
retrieving revision 1.3
diff -c -r1.3 EcosProcess.java
*** java/lang/EcosProcess.java	7 Mar 2000 19:55:26 -0000	1.3
--- java/lang/EcosProcess.java	10 Aug 2002 20:01:21 -0000
***************
*** 10,15 ****
--- 10,16 ----
  
  package java.lang;
  
+ import java.io.File;
  import java.io.InputStream;
  import java.io.OutputStream;
  import java.io.IOException;
***************
*** 52,58 ****
      return 0;
    }
  
!   public ConcreteProcess (String[] progarray, String[] envp) throws IOException
    {
      throw new IOException ("eCos processes unimplemented");
    }
--- 53,62 ----
      return 0;
    }
  
!   public ConcreteProcess (String[] progarray,
!                           String[] envp,
!                           File dir)
!     throws IOException
    {
      throw new IOException ("eCos processes unimplemented");
    }
Index: java/lang/PosixProcess.java
===================================================================
RCS file: /cvsroot/gcc/gcc/libjava/java/lang/PosixProcess.java,v
retrieving revision 1.4
diff -c -r1.4 PosixProcess.java
*** java/lang/PosixProcess.java	24 Sep 2001 04:51:50 -0000	1.4
--- java/lang/PosixProcess.java	10 Aug 2002 20:01:21 -0000
***************
*** 10,15 ****
--- 10,16 ----
  
  package java.lang;
  
+ import java.io.File;
  import java.io.InputStream;
  import java.io.OutputStream;
  import java.io.IOException;
***************
*** 53,67 ****
  
    // This is used for actual initialization, as we can't write a
    // native constructor.
!   public native void startProcess (String[] progarray, String[] envp)
      throws IOException;
  
    // This file is copied to `ConcreteProcess.java' before
    // compilation.  Hence the constructor name apparently does not
    // match the file name.
!   public ConcreteProcess (String[] progarray, String[] envp) throws IOException
    {
!     startProcess (progarray, envp);
    }
  
    // The process id.  This is cast to a pid_t on the native side.
--- 54,73 ----
  
    // This is used for actual initialization, as we can't write a
    // native constructor.
!   public native void startProcess (String[] progarray,
!                                    String[] envp,
!                                    File dir)
      throws IOException;
  
    // This file is copied to `ConcreteProcess.java' before
    // compilation.  Hence the constructor name apparently does not
    // match the file name.
!   public ConcreteProcess (String[] progarray,
!                           String[] envp,
!                           File dir)
!     throws IOException
    {
!     startProcess (progarray, envp, dir);
    }
  
    // The process id.  This is cast to a pid_t on the native side.
Index: java/lang/Runtime.java
===================================================================
RCS file: /cvsroot/gcc/gcc/libjava/java/lang/Runtime.java,v
retrieving revision 1.11
diff -c -r1.11 Runtime.java
*** java/lang/Runtime.java	24 Jul 2002 23:17:39 -0000	1.11
--- java/lang/Runtime.java	10 Aug 2002 20:01:21 -0000
***************
*** 526,532 ****
     *         entries
     * @throws IndexOutOfBoundsException if cmd is length 0
     * @since 1.3
-    * @XXX Ignores dir, for now
     */
    public Process exec(String[] cmd, String[] env, File dir)
      throws IOException
--- 526,531 ----
***************
*** 536,543 ****
        sm.checkExec(cmd[0]);
      if (env == null)
        env = new String[0];
!     //XXX Should be:    return execInternal(cmd, env, dir);
!     return execInternal(cmd, env);
    }
  
    /**
--- 535,541 ----
        sm.checkExec(cmd[0]);
      if (env == null)
        env = new String[0];
!     return execInternal(cmd, env, dir);
    }
  
    /**
***************
*** 729,735 ****
     * the environment should contain name=value mappings. If directory is null,
     * use the current working directory; otherwise start the process in that
     * directory.
-    * XXX Add directory support.
     *
     * @param cmd the non-null command tokens
     * @param env the non-null environment setup
--- 727,732 ----
***************
*** 737,744 ****
     * @return the newly created process
     * @throws NullPointerException if cmd or env have null elements
     */
!   //  native Process execInternal(String[] cmd, String[] env, File dir);
!   native Process execInternal(String[] cmd, String[] env);
  
    /**
     * Get the system properties. This is done here, instead of in System,
--- 734,740 ----
     * @return the newly created process
     * @throws NullPointerException if cmd or env have null elements
     */
!   native Process execInternal(String[] cmd, String[] env, File dir);
  
    /**
     * Get the system properties. This is done here, instead of in System,
Index: java/lang/Win32Process.java
===================================================================
RCS file: /cvsroot/gcc/gcc/libjava/java/lang/Win32Process.java,v
retrieving revision 1.4
diff -c -r1.4 Win32Process.java
*** java/lang/Win32Process.java	10 Mar 2002 17:59:22 -0000	1.4
--- java/lang/Win32Process.java	10 Aug 2002 20:01:21 -0000
***************
*** 10,15 ****
--- 10,16 ----
  
  package java.lang;
  
+ import java.io.File;
  import java.io.InputStream;
  import java.io.OutputStream;
  import java.io.IOException;
***************
*** 60,66 ****
      throw new Error("not implemented");
    }
  
!   public ConcreteProcess (String[] progarray, String[] envp) throws IOException
    {
      throw new IOException("not implemented");
    }
--- 61,70 ----
      throw new Error("not implemented");
    }
  
!   public ConcreteProcess (String[] progarray,
!                           String[] envp,
!                           File dir)
!     throws IOException
    {
      throw new IOException("not implemented");
    }
Index: java/lang/natPosixProcess.cc
===================================================================
RCS file: /cvsroot/gcc/gcc/libjava/java/lang/natPosixProcess.cc,v
retrieving revision 1.12
diff -c -r1.12 natPosixProcess.cc
*** java/lang/natPosixProcess.cc	6 Mar 2002 05:13:58 -0000	1.12
--- java/lang/natPosixProcess.cc	10 Aug 2002 20:01:21 -0000
***************
*** 30,35 ****
--- 30,36 ----
  #include <java/lang/InterruptedException.h>
  #include <java/lang/NullPointerException.h>
  #include <java/lang/Thread.h>
+ #include <java/io/File.h>
  #include <java/io/FileDescriptor.h>
  #include <java/io/FileInputStream.h>
  #include <java/io/FileOutputStream.h>
***************
*** 116,122 ****
  
  void
  java::lang::ConcreteProcess::startProcess (jstringArray progarray,
! 					   jstringArray envp)
  {
    using namespace java::io;
  
--- 117,124 ----
  
  void
  java::lang::ConcreteProcess::startProcess (jstringArray progarray,
! 					   jstringArray envp,
! 					   java::io::File *dir)
  {
    using namespace java::io;
  
***************
*** 188,194 ****
  
        if (pid == 0)
  	{
! 	  // Child process, so remap descriptors and exec.
  
  	  if (envp)
  	    {
--- 190,196 ----
  
        if (pid == 0)
  	{
! 	  // Child process, so remap descriptors, chdir and exec.
  
  	  if (envp)
  	    {
***************
*** 229,234 ****
--- 231,245 ----
  	  close (outp[0]);
  	  close (outp[1]);
  	  close (msgp[0]);
+           
+ 	  // Change directory
+ 	  if (dir != NULL)
+ 	    if (chdir (new_string (dir->getPath ())) != 0)
+ 	      {
+ 		char c = errno;
+ 		write (msgp[1], &c, 1);
+ 		_exit (127);
+ 	      }
  
  	  execvp (args[0], args);
  
Index: java/lang/natRuntime.cc
===================================================================
RCS file: /cvsroot/gcc/gcc/libjava/java/lang/natRuntime.cc,v
retrieving revision 1.24
diff -c -r1.24 natRuntime.cc
*** java/lang/natRuntime.cc	6 Jul 2002 05:11:53 -0000	1.24
--- java/lang/natRuntime.cc	10 Aug 2002 20:01:21 -0000
***************
*** 21,26 ****
--- 21,27 ----
  #include <java/lang/UnsatisfiedLinkError.h>
  #include <gnu/gcj/runtime/FileDeleter.h>
  #include <gnu/gcj/runtime/FinalizerThread.h>
+ #include <java/io/File.h>
  #include <java/util/Properties.h>
  #include <java/util/TimeZone.h>
  #include <java/lang/StringBuffer.h>
***************
*** 538,546 ****
  
  java::lang::Process *
  java::lang::Runtime::execInternal (jstringArray cmd,
! 				   jstringArray env)
  {
!   return new java::lang::ConcreteProcess (cmd, env);
  }
  
  jint
--- 539,548 ----
  
  java::lang::Process *
  java::lang::Runtime::execInternal (jstringArray cmd,
! 				   jstringArray env,
! 				   java::io::File *dir)
  {
!   return new java::lang::ConcreteProcess (cmd, env, dir);
  }
  
  jint
>Release-Note:
>Audit-Trail:
>Unformatted: