diff mbox

[jit] Expose choose_tmpdir and use it when building tmpdir for jit compilation

Message ID 1411660984-30735-1-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm Sept. 25, 2014, 4:03 p.m. UTC
On Tue, 2014-09-23 at 23:27 +0000, Joseph S. Myers wrote:
[...]
> The code for compiling a .s file should:
> 
> * use choose_tmpdir from libiberty rather than hardcoding /tmp (or, 
> better, create the files directly with make_temp_file, and delete them 
> individual afterwards);
[...]

I believe that a tempdir is better than creating individual tempfiles:
for debugging all this it's handy to have dumpfiles, and if they're
enabled, they're written out relative to the supposed source file.
So if we have
  SOME_PATH_TO/fake.c
we get e.g.
  SOME_PATH_TO/fake.c.016t.ssa
and so on.  The simplest way to clean this all up seems to be to put
everything related to a compile in a tempdir, and to create that tempdir
as securely as we can.  The tempdir is deleted in the destructor for
the playback::context (unless GCC_JIT_BOOL_OPTION_KEEP_INTERMEDIATES
has been set).

So I went with the choose_tmpdir approach, to avoid hardcoding "/tmp/",
although it wasn't previously exposed in liberty.h.

I've committed the following to branch dmalcolm/jit:

Expose choose_tmpdir and use it when building tmpdir for jit                                                        
compilation:

Fix the return type of libiberty's choose_tmpdir.

Expose it in libiberty.h

Use it within the JIT's playback::context::compile when building the
tempdir to avoid hardcoding "/tmp/".

gcc/jit/ChangeLog.jit:
	* 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
	hardcoding "/tmp/" within "/tmp/libgccjit-XXXXXX".

include/ChangeLog.jit:
	* libiberty.h (choose_tmpdir): New prototype.
	* ChangeLog.jit: New.

libiberty/ChangeLog.jit:
	* choose-temp.c (choose_tmpdir): Remove now-redundant local
	copy of prototype.
	* functions.texi: Regenerate.
	* make-temp-file.c (choose_tmpdir): Convert return type from
	char * to const char * - given that this returns a pointer to
	a memoized allocation, the caller must not touch it.
---
 gcc/jit/ChangeLog.jit      |  7 +++++++
 gcc/jit/internal-api.c     | 40 +++++++++++++++++++++++++++++++++++++++-
 include/ChangeLog.jit      | 11 +++++++++++
 include/libiberty.h        |  5 +++++
 libiberty/ChangeLog.jit    |  9 +++++++++
 libiberty/choose-temp.c    |  1 -
 libiberty/functions.texi   | 13 ++++++-------
 libiberty/make-temp-file.c |  4 ++--
 8 files changed, 79 insertions(+), 11 deletions(-)
 create mode 100644 include/ChangeLog.jit
diff mbox

Patch

diff --git a/gcc/jit/ChangeLog.jit b/gcc/jit/ChangeLog.jit
index b4700e4..d66203a 100644
--- a/gcc/jit/ChangeLog.jit
+++ b/gcc/jit/ChangeLog.jit
@@ -1,3 +1,10 @@ 
+2014-09-25  David Malcolm  <dmalcolm@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
+	hardcoding "/tmp/" within "/tmp/libgccjit-XXXXXX".
+
 2014-09-24  David Malcolm  <dmalcolm@redhat.com>
 
 	* docs/internals/index.rst ("Overview of code structure"): Add
diff --git a/gcc/jit/internal-api.c b/gcc/jit/internal-api.c
index 32fe7cb..50fd83b 100644
--- a/gcc/jit/internal-api.c
+++ b/gcc/jit/internal-api.c
@@ -4826,6 +4826,44 @@  block (function *func,
   m_label_expr = NULL;
 }
 
+/* Construct a tempdir path template suitable for use by mkdtemp
+   e.g. "/tmp/libgccjit-XXXXXX", but respecting the rules in
+   libiberty's choose_tempdir rather than hardcoding "/tmp/".
+
+   The memory is allocated using malloc and must be freed.
+   Aborts the process if allocation fails. */
+
+static char *
+make_tempdir_path_template ()
+{
+  const char *tmpdir_buf;
+  size_t tmpdir_len;
+  const char *file_template_buf;
+  size_t file_template_len;
+  char *result;
+
+  /* The result of choose_tmpdir is a cached buffer within libiberty, so
+     we must *not* free it.  */
+  tmpdir_buf = choose_tmpdir ();
+
+  /* choose_tmpdir aborts on malloc failure.  */
+  gcc_assert (tmpdir_buf);
+
+  tmpdir_len = strlen (tmpdir_buf);
+  /* tmpdir_buf should now have a dir separator as the final byte.  */
+  gcc_assert (tmpdir_len > 0);
+  gcc_assert (tmpdir_buf[tmpdir_len - 1] == DIR_SEPARATOR);
+
+  file_template_buf = "libgccjit-XXXXXX";
+  file_template_len = strlen (file_template_buf);
+
+  result = XNEWVEC (char, tmpdir_len + file_template_len + 1);
+  strcpy (result, tmpdir_buf);
+  strcpy (result + tmpdir_len, file_template_buf);
+
+  return result;
+}
+
 /* Compile a playback::context:
 
    - Use the context's options to cconstruct command-line options, and
@@ -4845,7 +4883,7 @@  compile ()
   const char *fake_args[20];
   unsigned int num_args;
 
-  m_path_template = xstrdup ("/tmp/libgccjit-XXXXXX");
+  m_path_template = make_tempdir_path_template ();
   if (!m_path_template)
     return NULL;
 
diff --git a/include/ChangeLog.jit b/include/ChangeLog.jit
new file mode 100644
index 0000000..84acd33
--- /dev/null
+++ b/include/ChangeLog.jit
@@ -0,0 +1,11 @@ 
+2014-09-25  David Malcolm  <dmalcolm@redhat.com>
+
+	* libiberty.h (choose_tmpdir): New prototype.
+	* ChangeLog.jit: New.
+
+
+Copyright (C) 2014 Free Software Foundation, Inc.
+
+Copying and distribution of this file, with or without modification,
+are permitted in any medium without royalty provided the copyright
+notice and this notice are preserved.
diff --git a/include/libiberty.h b/include/libiberty.h
index 56b8b43..7802501 100644
--- a/include/libiberty.h
+++ b/include/libiberty.h
@@ -227,6 +227,11 @@  extern char *make_relative_prefix (const char *, const char *,
 extern char *make_relative_prefix_ignore_links (const char *, const char *,
 						const char *) ATTRIBUTE_MALLOC;
 
+/* Returns a pointer to a directory path suitable for creating temporary
+   files in.  */
+
+extern const char *choose_tmpdir (void) ATTRIBUTE_RETURNS_NONNULL;
+
 /* Choose a temporary directory to use for scratch files.  */
 
 extern char *choose_temp_base (void) ATTRIBUTE_MALLOC ATTRIBUTE_RETURNS_NONNULL;
diff --git a/libiberty/ChangeLog.jit b/libiberty/ChangeLog.jit
index 52328e3..5b64f80 100644
--- a/libiberty/ChangeLog.jit
+++ b/libiberty/ChangeLog.jit
@@ -1,3 +1,12 @@ 
+2014-09-25  David Malcolm  <dmalcolm@redhat.com>
+
+	* choose-temp.c (choose_tmpdir): Remove now-redundant local
+	copy of prototype.
+	* functions.texi: Regenerate.
+	* make-temp-file.c (choose_tmpdir): Convert return type from
+	char * to const char * - given that this returns a pointer to
+	a memoized allocation, the caller must not touch it.
+
 2014-09-24  David Malcolm  <dmalcolm@redhat.com>
 
 	* ChangeLog.jit: Add copyright footer.
diff --git a/libiberty/choose-temp.c b/libiberty/choose-temp.c
index 0a454cf..8e1e84b 100644
--- a/libiberty/choose-temp.c
+++ b/libiberty/choose-temp.c
@@ -34,7 +34,6 @@  Boston, MA 02110-1301, USA.  */
 #endif
 
 #include "libiberty.h"
-extern char *choose_tmpdir (void);
 
 /* Name of temporary file.
    mktemp requires 6 trailing X's.  */
diff --git a/libiberty/functions.texi b/libiberty/functions.texi
index 9323ff9..387aee0 100644
--- a/libiberty/functions.texi
+++ b/libiberty/functions.texi
@@ -125,7 +125,7 @@  Uses @code{malloc} to allocate storage for @var{nelem} objects of
 
 @end deftypefn
 
-@c choose-temp.c:46
+@c choose-temp.c:45
 @deftypefn Extension char* choose_temp_base (void)
 
 Return a prefix for temporary file names or @code{NULL} if unable to
@@ -139,7 +139,7 @@  not recommended.
 @end deftypefn
 
 @c make-temp-file.c:96
-@deftypefn Replacement char* choose_tmpdir ()
+@deftypefn Replacement const char* choose_tmpdir ()
 
 Returns a pointer to a directory path suitable for creating temporary
 files in.
@@ -160,9 +160,8 @@  number of seconds used.
   @dots{}, @code{NULL})
 
 Concatenate zero or more of strings and return the result in freshly
-@code{xmalloc}ed memory.  Returns @code{NULL} if insufficient memory is
-available.  The argument list is terminated by the first @code{NULL}
-pointer encountered.  Pointers to empty strings are ignored.
+@code{xmalloc}ed memory.  The argument list is terminated by the first
+@code{NULL} pointer encountered.  Pointers to empty strings are ignored.
 
 @end deftypefn
 
@@ -528,7 +527,7 @@  nineteen EBCDIC varying characters is tested; exercise caution.)
 @end ftable
 @end defvr
 
-@c hashtab.c:336
+@c hashtab.c:328
 @deftypefn Supplemental htab_t htab_create_typed_alloc (size_t @var{size}, @
 htab_hash @var{hash_f}, htab_eq @var{eq_f}, htab_del @var{del_f}, @
 htab_alloc @var{alloc_tab_f}, htab_alloc @var{alloc_f}, @
@@ -1163,7 +1162,7 @@  control over the state of the random number generator.
 
 @end deftypefn
 
-@c concat.c:174
+@c concat.c:160
 @deftypefn Extension char* reconcat (char *@var{optr}, const char *@var{s1}, @
   @dots{}, @code{NULL})
 
diff --git a/libiberty/make-temp-file.c b/libiberty/make-temp-file.c
index 7b74f81..244cc23 100644
--- a/libiberty/make-temp-file.c
+++ b/libiberty/make-temp-file.c
@@ -93,7 +93,7 @@  static char *memoized_tmpdir;
 
 /*
 
-@deftypefn Replacement char* choose_tmpdir ()
+@deftypefn Replacement const char* choose_tmpdir ()
 
 Returns a pointer to a directory path suitable for creating temporary
 files in.
@@ -102,7 +102,7 @@  files in.
 
 */
 
-char *
+const char *
 choose_tmpdir (void)
 {
   if (!memoized_tmpdir)