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