On Tue, 2014-09-23 at 23:27 +0000, Joseph S. Myers wrote:
[...]
> The code for compiling a .s file should:
[...]
> * use libiberty's pexecute to run subprocesses, not "system" (building up 
> a string to pass to the shell always looks like a security hole, though in 
> this case it may in fact be safe);
[...]

libiberty.h has this deprecation comment about pexecute:

/* pexecute and pwait are the old pexecute interface, still here for
   backward compatibility.  Don't use these for new code.  Instead,
   use pex_init/pex_run/pex_get_status/pex_free, or pex_one.  */

so I used pex_one when eliminating the "system" callsite.

Committed to branch dmalcolm/jit:

gcc/jit/ChangeLog.jit:
        * internal-api.c (gcc::jit::playback::context::compile): Use
        pex_one rather than system when invoking "gcc" to go from a .s
        file to a .so file.
---
 gcc/jit/ChangeLog.jit  |  6 ++++++
 gcc/jit/internal-api.c | 46 +++++++++++++++++++++++++++++++++++++---------
 2 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/gcc/jit/ChangeLog.jit b/gcc/jit/ChangeLog.jit
index d66203a..9cbba20 100644
--- a/gcc/jit/ChangeLog.jit
+++ b/gcc/jit/ChangeLog.jit
@@ -1,5 +1,11 @@
 2014-09-25  David Malcolm  <dmalc...@redhat.com>
 
+       * internal-api.c (gcc::jit::playback::context::compile): Use
+       pex_one rather than system when invoking "gcc" to go from a .s
+       file to a .so file.
+
+2014-09-25  David Malcolm  <dmalc...@redhat.com>
+
        * internal-api.c (make_tempdir_path_template): New.
        (gcc::jit::playback::context::compile): Call
        make_tempdir_path_template to make m_path_template, rather than
diff --git a/gcc/jit/internal-api.c b/gcc/jit/internal-api.c
index 50fd83b..05ef544 100644
--- a/gcc/jit/internal-api.c
+++ b/gcc/jit/internal-api.c
@@ -4992,18 +4992,46 @@ compile ()
      We could reuse parts of gcc/gcc.c to do this.
      For now, just use the /usr/bin/gcc on the system...
    */
-  /* FIXME: totally faking it for now, not even using pex */
   {
     auto_timevar assemble_timevar (TV_ASSEMBLE);
+    const char *errmsg;
+    const char *argv[6];
+    int exit_status = 0;
+    int err = 0;
+
+    argv[0] = "gcc";
+    argv[1] = "-shared";
+    /* The input: assembler.  */
+    argv[2] = m_path_s_file;
+    /* The output: shared library.  */
+    argv[3] = "-o";
+    argv[4] = m_path_so_file;
+    /* pex argv arrays are NULL-terminated.  */
+    argv[5] = NULL;
+
+    errmsg = pex_one (PEX_SEARCH, /* int flags, */
+                     "gcc", /* const char *executable */
+                     const_cast<char * const *> (argv),
+                     ctxt_progname, /* const char *pname */
+                     NULL, /* const char *outname */
+                     NULL, /* const char *errname */
+                     &exit_status, /* int *status */
+                     &err); /* int *err*/
+    if (errmsg)
+      {
+       add_error (NULL, "error invoking gcc harness: %s", errmsg);
+       return NULL;
+      }
 
-    char cmd[1024];
-    snprintf (cmd, 1024, "gcc -shared %s -o %s",
-              m_path_s_file, m_path_so_file);
-    if (0)
-      printf ("cmd: %s\n", cmd);
-    int ret = system (cmd);
-    if (ret)
-      return NULL;
+    /* pex_one can return a NULL errmsg when the executable wasn't
+       found (or doesn't exist), so trap these cases also.  */
+    if (exit_status || err)
+      {
+       add_error (NULL,
+                  "error invoking gcc harness: exit_status: %i err: %i",
+                  exit_status, err);
+       return NULL;
+      }
   }
 
   // TODO: split out assembles vs linker
-- 
1.7.11.7

Reply via email to