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 <[email protected]>
+ * 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 <[email protected]>
+
* 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