[committed] PR jit/69144: Ensure that libgccjit's tempdir is fully cleaned-up

[committed] PR jit/69144: Ensure that libgccjit's tempdir is fully cleaned-up

[David Malcolm]

There were a couple of ways that libgccjit could fail to unlink all
of its tempfiles, leading to /tmp/libgccjit-* tempdirs lingering
after the build:
- dumpfiles requested by gcc_jit_context_enable_dump
- ahead-of-time compilation artifacts which lingered in the tempdir
  after they've been copied up to the output_path.  This was only
  the case for GCC_JIT_OUTPUT_KIND_OBJECT_FILE and
  GCC_JIT_OUTPUT_KIND_EXECUTABLE.

The following patch fixes these by introducing a vec of additional
cleanups to be performed by gcc:jit::tempdir's dtor.

In addition, if a gcc_jit_result * is leaked and
GCC_JIT_BOOL_OPTION_DEBUGINFO is enabled, the tempdir will also
not be cleaned up.  This was the case for tut04-toyvm/toyvm.cc
which the patch fixes by introducing a wrapper around
gcc_jit_result *.  Doing this required some updates to the
corresponding docs.

Committed to trunk as r232582.

The fix is probably applicable to the gcc 5 branch, but AIUI that
branch is only open for regression fixes and documentation fixes,
which isn't the case for this.

gcc/jit/ChangeLog:
	PR jit/69144
	* jit-playback.c (gcc::jit::playback::compile_to_file::postprocess):
	Potentially add the temporary artifact to the tempdir's list of
	tempfiles needing additional cleanup.
	(gcc::jit::playback::context::extract_any_requested_dumps): Likewise
	for the dumpfile.
	* jit-tempdir.c (gcc::jit::tempdir::~tempdir): Clean up additional
	tempfiles.
	* jit-tempdir.h (gcc::jit::tempdir::add_temp_file): New method.
	(gcc::jit::tempdir::m_tempfiles): New field.
	* docs/cp/intro/tutorial04.rst: Update for changes to toyvm.cc.
	* docs/examples/tut04-toyvm/toyvm.cc (class compilation_result):
	New.
	(toyvm_function::compile): Change return type from function ptr
	to a compilation_result.
	(toyvm_function::get_function_name): New accessor.
	(toyvm_function::m_funcname): New field.
	(get_function_name): Convert to...
	(toyvm_function::make_function_name): ...this new method.
	(toyvm_function::parse): Call make_function_name.
	(toyvm_function::compile): Convert return type from function ptr
	to a compilation_result.  Use get_function_name.
	(compilation_state::compile): Convert return type from
	gcc_jit_result * to a compilation_result.
	(test_script): Update for above changes, extracting the code from
	the compilation_result.
	(main): Likewise.
	* docs/_build/texinfo/libgccjit.texi: Regenerate.
---
 gcc/jit/docs/_build/texinfo/libgccjit.texi | 1035 +++++++++++++++-------------
 gcc/jit/docs/cp/intro/tutorial04.rst       |    9 +-
 gcc/jit/docs/examples/tut04-toyvm/toyvm.cc |   74 +-
 gcc/jit/jit-playback.c                     |   24 +-
 gcc/jit/jit-tempdir.c                      |   18 +-
 gcc/jit/jit-tempdir.h                      |    8 +
 6 files changed, 660 insertions(+), 508 deletions(-)

diff --git a/gcc/jit/docs/cp/intro/tutorial04.rst b/gcc/jit/docs/cp/intro/tutorial04.rst
index 1eb7454..443180c 100644
--- a/gcc/jit/docs/cp/intro/tutorial04.rst
+++ b/gcc/jit/docs/cp/intro/tutorial04.rst
@@ -297,15 +297,14 @@ Compiling the context
 Having finished looping over the blocks and populating them with
 statements, the context is complete.
 
-We can now compile it, and extract machine code from the result:
+We can now compile it, extract machine code from the result, and
+run it:
 
    .. literalinclude:: ../../examples/tut04-toyvm/toyvm.cc
-    :start-after: /* We've now finished populating the context.  Compile it.  */
-    :end-before: /* (this leaks "result" and "funcname") */
+    :start-after: /* Wrapper around a gcc_jit_result *.  */
+    :end-before: /* Functions are compiled to this function ptr type.  */
     :language: c++
 
-We can now run the result:
-
    .. literalinclude:: ../../examples/tut04-toyvm/toyvm.cc
     :start-after: /* JIT-compilation.  */
     :end-before: return 0;
diff --git a/gcc/jit/docs/examples/tut04-toyvm/toyvm.cc b/gcc/jit/docs/examples/tut04-toyvm/toyvm.cc
index 8fd9b65e..73089da 100644
--- a/gcc/jit/docs/examples/tut04-toyvm/toyvm.cc
+++ b/gcc/jit/docs/examples/tut04-toyvm/toyvm.cc
@@ -28,6 +28,29 @@ along with GCC; see the file COPYING3.  If not see
 
 #include <libgccjit++.h>
 
+/* Wrapper around a gcc_jit_result *.  */
+
+class compilation_result
+{
+public:
+  compilation_result (gcc_jit_result *result) :
+    m_result (result)
+  {
+  }
+  ~compilation_result ()
+  {
+    gcc_jit_result_release (m_result);
+  }
+
+  void *get_code (const char *funcname)
+  {
+    return gcc_jit_result_get_code (m_result, funcname);
+  }
+
+private:
+  gcc_jit_result *m_result;
+};
+
 /* Functions are compiled to this function ptr type.  */
 typedef int (*toyvm_compiled_func) (int);
 
@@ -100,11 +123,19 @@ public:
   int
   interpret (int arg, FILE *trace);
 
-  toyvm_compiled_func
+  compilation_result
   compile ();
 
+  const char *
+  get_function_name () const { return m_funcname; }
+
+private:
+  void
+  make_function_name (const char *filename);
+
 private:
   const char *fn_filename;
+  char       *m_funcname;
   int         fn_num_ops;
   toyvm_op    fn_ops[MAX_OPS];
   friend struct compilation_state;
@@ -149,8 +180,8 @@ toyvm_function::add_unary_op (enum opcode opcode,
   add_op (opcode, operand, linenum);
 }
 
-static char *
-get_function_name (const char *filename)
+void
+toyvm_function::make_function_name (const char *filename)
 {
   /* Skip any path separators.  */
   const char *pathsep = strrchr (filename, '/');
@@ -158,14 +189,12 @@ get_function_name (const char *filename)
     filename = pathsep + 1;
 
   /* Copy filename to funcname.  */
-  char *funcname = (char *)malloc (strlen (filename) + 1);
+  m_funcname = (char *)malloc (strlen (filename) + 1);
 
-  strcpy (funcname, filename);
+  strcpy (m_funcname, filename);
 
   /* Convert "." to NIL terminator.  */
-  *(strchr (funcname, '.')) = '\0';
-
-  return funcname;
+  *(strchr (m_funcname, '.')) = '\0';
 }
 
 toyvm_function *
@@ -197,6 +226,7 @@ toyvm_function::parse (const char *filename, const char *name)
       goto error;
     }
   fn->fn_filename = filename;
+  fn->make_function_name (filename);
 
   /* Read the lines of the file.  */
   while ((linelen = getline (&line, &bufsize, f)) != -1)
@@ -420,7 +450,7 @@ public:
   void create_types ();
   void create_locations ();
   void create_function (const char *funcname);
-  gcc_jit_result *compile ();
+  compilation_result compile ();
 
 private:
   void
@@ -462,24 +492,18 @@ private:
 
 /* The main compilation hook.  */
 
-toyvm_compiled_func
+compilation_result
 toyvm_function::compile ()
 {
   compilation_state state (*this);
-  char *funcname;
-
-  funcname = get_function_name (fn_filename);
 
   state.create_context ();
   state.create_types ();
   state.create_locations ();
-  state.create_function (funcname);
+  state.create_function (get_function_name ());
 
   /* We've now finished populating the context.  Compile it.  */
-  gcc_jit_result *result = state.compile ();
-
-  return (toyvm_compiled_func)gcc_jit_result_get_code (result, funcname);
-  /* (this leaks "result" and "funcname") */
+  return state.compile ();
 }
 
 /* Stack manipulation.  */
@@ -767,7 +791,7 @@ compilation_state::create_function (const char *funcname)
     } /* end of loop on PC locations.  */
 }
 
-gcc_jit_result *
+compilation_result
 compilation_state::compile ()
 {
   return ctxt.compile ();
@@ -825,7 +849,10 @@ test_script (const char *scripts_dir, const char *script_name, int input,
   interpreted_result = fn->interpret (input, NULL);
   CHECK_VALUE (interpreted_result, expected_result);
 
-  code = fn->compile ();
+  compilation_result compiler_result = fn->compile ();
+
+  const char *funcname = fn->get_function_name ();
+  code = (toyvm_compiled_func)compiler_result.get_code (funcname);
   CHECK_NON_NULL (code);
 
   compiled_result = code (input);
@@ -894,7 +921,12 @@ main (int argc, char **argv)
 	  fn->interpret (atoi (argv[2]), NULL));
 
   /* JIT-compilation.  */
-  toyvm_compiled_func code = fn->compile ();
+  compilation_result compiler_result = fn->compile ();
+
+  const char *funcname = fn->get_function_name ();
+  toyvm_compiled_func code
+    = (toyvm_compiled_func)compiler_result.get_code (funcname);
+
   printf ("compiler result: %d\n",
 	  code (atoi (argv[2])));
 
diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index ee129ae..d150ec1 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -1888,6 +1888,7 @@ playback::compile_to_file::postprocess (const char *ctxt_progname)
     case GCC_JIT_OUTPUT_KIND_ASSEMBLER:
       copy_file (get_tempdir ()->get_path_s_file (),
 		 m_output_path);
+      /* The .s file is automatically unlinked by tempdir::~tempdir.  */
       break;
 
     case GCC_JIT_OUTPUT_KIND_OBJECT_FILE:
@@ -1902,9 +1903,13 @@ playback::compile_to_file::postprocess (const char *ctxt_progname)
 		       false, /* bool shared, */
 		       false);/* bool run_linker */
 	if (!errors_occurred ())
-	  copy_file (tmp_o_path,
-		     m_output_path);
-	free (tmp_o_path);
+	  {
+	    copy_file (tmp_o_path,
+		       m_output_path);
+	    get_tempdir ()->add_temp_file (tmp_o_path);
+	  }
+	else
+	  free (tmp_o_path);
       }
       break;
 
@@ -1918,6 +1923,7 @@ playback::compile_to_file::postprocess (const char *ctxt_progname)
       if (!errors_occurred ())
 	copy_file (get_tempdir ()->get_path_so_file (),
 		   m_output_path);
+      /* The .so file is automatically unlinked by tempdir::~tempdir.  */
       break;
 
     case GCC_JIT_OUTPUT_KIND_EXECUTABLE:
@@ -1932,9 +1938,13 @@ playback::compile_to_file::postprocess (const char *ctxt_progname)
 		       false, /* bool shared, */
 		       true);/* bool run_linker */
 	if (!errors_occurred ())
-	  copy_file (tmp_exe_path,
-		     m_output_path);
-	free (tmp_exe_path);
+	  {
+	    copy_file (tmp_exe_path,
+		       m_output_path);
+	    get_tempdir ()->add_temp_file (tmp_exe_path);
+	  }
+	else
+	  free (tmp_exe_path);
       }
       break;
 
@@ -2279,7 +2289,7 @@ extract_any_requested_dumps (vec <recording::requested_dump> *requested_dumps)
       filename = g->get_dumps ()->get_dump_file_name (dfi);
       content = read_dump_file (filename);
       *(d->m_out_ptr) = content;
-      free (filename);
+      m_tempdir->add_temp_file (filename);
     }
 }
 
diff --git a/gcc/jit/jit-tempdir.c b/gcc/jit/jit-tempdir.c
index f370e7b..0e11c42 100644
--- a/gcc/jit/jit-tempdir.c
+++ b/gcc/jit/jit-tempdir.c
@@ -121,7 +121,7 @@ gcc::jit::tempdir::~tempdir ()
     fprintf (stderr, "intermediate files written to %s\n", m_path_tempdir);
   else
     {
-      /* Clean up .s/.so and tempdir. */
+      /* Clean up .s/.so.  */
       if (m_path_s_file)
 	{
 	  log ("unlinking .s file: %s", m_path_s_file);
@@ -132,6 +132,17 @@ gcc::jit::tempdir::~tempdir ()
 	  log ("unlinking .so file: %s", m_path_so_file);
 	  unlink (m_path_so_file);
 	}
+
+      /* Clean up any other tempfiles.  */
+      int i;
+      char *tempfile;
+      FOR_EACH_VEC_ELT (m_tempfiles, i, tempfile)
+	{
+	  log ("unlinking tempfile: %s", tempfile);
+	  unlink (tempfile);
+	}
+
+      /* The tempdir should now be empty; remove it.  */
       if (m_path_tempdir)
 	{
 	  log ("removing tempdir: %s", m_path_tempdir);
@@ -145,4 +156,9 @@ gcc::jit::tempdir::~tempdir ()
   free (m_path_c_file);
   free (m_path_s_file);
   free (m_path_so_file);
+
+  int i;
+  char *tempfile;
+  FOR_EACH_VEC_ELT (m_tempfiles, i, tempfile)
+    free (tempfile);
 }
diff --git a/gcc/jit/jit-tempdir.h b/gcc/jit/jit-tempdir.h
index 3695df6..cae25ef 100644
--- a/gcc/jit/jit-tempdir.h
+++ b/gcc/jit/jit-tempdir.h
@@ -58,6 +58,10 @@ class tempdir : public log_user
   const char * get_path_s_file () const { return m_path_s_file; }
   const char * get_path_so_file () const { return m_path_so_file; }
 
+  /* Add PATH to the vec of tempfiles that must be unlinked.
+     Take ownership of the buffer PATH; it will be freed.  */
+  void add_temp_file (char *path) { m_tempfiles.safe_push (path); }
+
  private:
   /* Was GCC_JIT_BOOL_OPTION_KEEP_INTERMEDIATES set?  If so, keep the
      on-disk tempdir around after this wrapper object goes away.  */
@@ -74,6 +78,10 @@ class tempdir : public log_user
   char *m_path_s_file;
   char *m_path_so_file;
 
+  /* Other files within the tempdir to be cleaned up:
+     - certain ahead-of-time compilation artifacts (.o and .exe files)
+     - dumpfiles that were requested via gcc_jit_context_enable_dump.  */
+  auto_vec <char *> m_tempfiles;
 };
 
 } // namespace gcc::jit
-- 
1.8.5.3