From d2286af3d3fe908f84957406aba98eefd1f6e05e Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Fri, 9 Jan 2015 17:01:04 +0000 Subject: [PATCH] PR jit/64206: delay cleanup of tempdir if the user has requested debuginfo gcc/jit/ChangeLog: PR jit/64206 * docs/internals/test-hello-world.exe.log.txt: Update, the log now shows tempdir creation/cleanup. * docs/_build/texinfo/libgccjit.texi: Regenerate. * jit-logging.h (class gcc::jit::log_user): Add gcc::jit::tempdir to the list of subclasses in the comment. * jit-playback.c (gcc::jit::playback::context::context): Add a comment clarifying when the tempdir gets cleaned up. (gcc::jit::playback::context::compile): Pass the context's logger, if any, to the tempdir. (gcc::jit::playback::context::dlopen_built_dso): When creating the gcc::jit::result, if GCC_JIT_BOOL_OPTION_DEBUGINFO is set, hand over ownership of the tempdir to it. * jit-result.c: Include "jit-tempdir.h". (gcc::jit::result::result): Add tempdir param, saving it as m_tempdir. (gcc::jit::result::~result): Delete m_tempdir. * jit-result.h (gcc::jit::result::result): Add tempdir param. (gcc::jit::result::m_tempdir): New field. * jit-tempdir.c (gcc::jit::tempdir::tempdir): Add logger param; add JIT_LOG_SCOPE. (gcc::jit::tempdir::create): Add JIT_LOG_SCOPE to log entry/exit, and log m_path_template and m_path_tempdir. (gcc::jit::tempdir::~tempdir): Add JIT_LOG_SCOPE to log entry/exit, and log the unlink and rmdir calls. * jit-tempdir.h: Include "jit-logging.h". (class gcc::jit::tempdir): Make this be a subclass of log_user. (gcc::jit::tempdir::tempdir): Add logger param. * notes.txt: Update to show the two possible places where the tempdir can be cleaned up. From-SVN: r219395 --- gcc/jit/ChangeLog | 33 +++++++++++++ gcc/jit/docs/_build/texinfo/libgccjit.texi | 27 +++++++++-- .../internals/test-hello-world.exe.log.txt | 16 ++++++- gcc/jit/jit-logging.h | 2 + gcc/jit/jit-playback.c | 48 +++++++++++++++++-- gcc/jit/jit-result.c | 13 ++++- gcc/jit/jit-result.h | 3 +- gcc/jit/jit-tempdir.c | 29 +++++++++-- gcc/jit/jit-tempdir.h | 6 ++- gcc/jit/notes.txt | 9 +++- 10 files changed, 165 insertions(+), 21 deletions(-) diff --git a/gcc/jit/ChangeLog b/gcc/jit/ChangeLog index 796c8a1c8ce..9dc133ed3ff 100644 --- a/gcc/jit/ChangeLog +++ b/gcc/jit/ChangeLog @@ -1,3 +1,36 @@ +2015-01-09 David Malcolm + + PR jit/64206 + * docs/internals/test-hello-world.exe.log.txt: Update, the log now + shows tempdir creation/cleanup. + * docs/_build/texinfo/libgccjit.texi: Regenerate. + * jit-logging.h (class gcc::jit::log_user): Add gcc::jit::tempdir + to the list of subclasses in the comment. + * jit-playback.c (gcc::jit::playback::context::context): Add a + comment clarifying when the tempdir gets cleaned up. + (gcc::jit::playback::context::compile): Pass the context's logger, + if any, to the tempdir. + (gcc::jit::playback::context::dlopen_built_dso): When creating the + gcc::jit::result, if GCC_JIT_BOOL_OPTION_DEBUGINFO is set, hand + over ownership of the tempdir to it. + * jit-result.c: Include "jit-tempdir.h". + (gcc::jit::result::result): Add tempdir param, saving it as + m_tempdir. + (gcc::jit::result::~result): Delete m_tempdir. + * jit-result.h (gcc::jit::result::result): Add tempdir param. + (gcc::jit::result::m_tempdir): New field. + * jit-tempdir.c (gcc::jit::tempdir::tempdir): Add logger param; + add JIT_LOG_SCOPE. + (gcc::jit::tempdir::create): Add JIT_LOG_SCOPE to log entry/exit, + and log m_path_template and m_path_tempdir. + (gcc::jit::tempdir::~tempdir): Add JIT_LOG_SCOPE to log + entry/exit, and log the unlink and rmdir calls. + * jit-tempdir.h: Include "jit-logging.h". + (class gcc::jit::tempdir): Make this be a subclass of log_user. + (gcc::jit::tempdir::tempdir): Add logger param. + * notes.txt: Update to show the two possible places where the + tempdir can be cleaned up. + 2015-01-08 David Malcolm * libgccjit.h (struct gcc_jit_context): Rewrite the descriptive diff --git a/gcc/jit/docs/_build/texinfo/libgccjit.texi b/gcc/jit/docs/_build/texinfo/libgccjit.texi index d489d46527a..4c9e2ba6e3f 100644 --- a/gcc/jit/docs/_build/texinfo/libgccjit.texi +++ b/gcc/jit/docs/_build/texinfo/libgccjit.texi @@ -19,7 +19,7 @@ @copying @quotation -libgccjit 5.0.0 (experimental 20150108), January 08, 2015 +libgccjit 5.0.0 (experimental 20150109), January 09, 2015 David Malcolm @@ -12468,9 +12468,12 @@ Client Code . Generated . libgccjit.so . . │ . . . . │ playback::context dtor . . ──> . . - . . │ Cleanup tempdir . + . . │ Normally we cleanup the tempdir here: . . │ ("fake.so" is unlinked from the . . │ filesystem at this point) + . . │ If the client code requested debuginfo, the + . . │ cleanup happens later (in gcc_jit_result_release) + . . │ to make it easier on the debugger (see PR jit/64206) . . <── . . . . │ . . . . │ end of recording::context::compile () @@ -12494,6 +12497,10 @@ etc│ . . . . ──────────────────────────> . . . . │ dlclose () the loaded DSO . . │ (code becomes uncallable) + . . │ . . + . . │ If the client code requested debuginfo, then + . . │ cleanup of the tempdir was delayed. + . . │ If that was the case, clean it up now. <─────────────────────────── . . │ . . . . @@ -12621,6 +12628,12 @@ JIT: exiting: void gcc::jit::recording::context::validate() JIT: entering: gcc::jit::playback::context::context(gcc::jit::recording::context*) JIT: exiting: gcc::jit::playback::context::context(gcc::jit::recording::context*) JIT: entering: gcc::jit::result* gcc::jit::playback::context::compile() +JIT: entering: gcc::jit::tempdir::tempdir(gcc::jit::logger*, int) +JIT: exiting: gcc::jit::tempdir::tempdir(gcc::jit::logger*, int) +JIT: entering: bool gcc::jit::tempdir::create() +JIT: m_path_template: /tmp/libgccjit-XXXXXX +JIT: m_path_tempdir: /tmp/libgccjit-CKq1M9 +JIT: exiting: bool gcc::jit::tempdir::create() JIT: entering: void gcc::jit::playback::context::make_fake_args(vec*, const char*, vec*) JIT: exiting: void gcc::jit::playback::context::make_fake_args(vec*, const char*, vec*) JIT: entering: void gcc::jit::playback::context::acquire_mutex() @@ -12671,8 +12684,9 @@ JIT: argv[5]: -fno-use-linker-plugin JIT: argv[6]: (null) JIT: exiting: void gcc::jit::playback::context::convert_to_dso(const char*) JIT: entering: gcc::jit::result* gcc::jit::playback::context::dlopen_built_dso() -JIT: entering: gcc::jit::result::result(gcc::jit::logger*, void*) -JIT: exiting: gcc::jit::result::result(gcc::jit::logger*, void*) +JIT: GCC_JIT_BOOL_OPTION_DEBUGINFO was set: handing over tempdir to jit::result +JIT: entering: gcc::jit::result::result(gcc::jit::logger*, void*, gcc::jit::tempdir*) +JIT: exiting: gcc::jit::result::result(gcc::jit::logger*, void*, gcc::jit::tempdir*) JIT: exiting: gcc::jit::result* gcc::jit::playback::context::dlopen_built_dso() JIT: entering: void gcc::jit::playback::context::release_mutex() JIT: exiting: void gcc::jit::playback::context::release_mutex() @@ -12696,6 +12710,11 @@ JIT: exiting: gcc_jit_context_release JIT: entering: gcc_jit_result_release JIT: deleting result: 0x12f75d0 JIT: entering: virtual gcc::jit::result::~result() +JIT: entering: gcc::jit::tempdir::~tempdir() +JIT: unlinking .s file: /tmp/libgccjit-CKq1M9/fake.s +JIT: unlinking .so file: /tmp/libgccjit-CKq1M9/fake.so +JIT: removing tempdir: /tmp/libgccjit-CKq1M9 +JIT: exiting: gcc::jit::tempdir::~tempdir() JIT: exiting: virtual gcc::jit::result::~result() JIT: exiting: gcc_jit_result_release JIT: gcc::jit::logger::~logger() diff --git a/gcc/jit/docs/internals/test-hello-world.exe.log.txt b/gcc/jit/docs/internals/test-hello-world.exe.log.txt index a96d80fc45e..113dc351297 100644 --- a/gcc/jit/docs/internals/test-hello-world.exe.log.txt +++ b/gcc/jit/docs/internals/test-hello-world.exe.log.txt @@ -46,6 +46,12 @@ JIT: exiting: void gcc::jit::recording::context::validate() JIT: entering: gcc::jit::playback::context::context(gcc::jit::recording::context*) JIT: exiting: gcc::jit::playback::context::context(gcc::jit::recording::context*) JIT: entering: gcc::jit::result* gcc::jit::playback::context::compile() +JIT: entering: gcc::jit::tempdir::tempdir(gcc::jit::logger*, int) +JIT: exiting: gcc::jit::tempdir::tempdir(gcc::jit::logger*, int) +JIT: entering: bool gcc::jit::tempdir::create() +JIT: m_path_template: /tmp/libgccjit-XXXXXX +JIT: m_path_tempdir: /tmp/libgccjit-CKq1M9 +JIT: exiting: bool gcc::jit::tempdir::create() JIT: entering: void gcc::jit::playback::context::make_fake_args(vec*, const char*, vec*) JIT: exiting: void gcc::jit::playback::context::make_fake_args(vec*, const char*, vec*) JIT: entering: void gcc::jit::playback::context::acquire_mutex() @@ -96,8 +102,9 @@ JIT: argv[5]: -fno-use-linker-plugin JIT: argv[6]: (null) JIT: exiting: void gcc::jit::playback::context::convert_to_dso(const char*) JIT: entering: gcc::jit::result* gcc::jit::playback::context::dlopen_built_dso() -JIT: entering: gcc::jit::result::result(gcc::jit::logger*, void*) -JIT: exiting: gcc::jit::result::result(gcc::jit::logger*, void*) +JIT: GCC_JIT_BOOL_OPTION_DEBUGINFO was set: handing over tempdir to jit::result +JIT: entering: gcc::jit::result::result(gcc::jit::logger*, void*, gcc::jit::tempdir*) +JIT: exiting: gcc::jit::result::result(gcc::jit::logger*, void*, gcc::jit::tempdir*) JIT: exiting: gcc::jit::result* gcc::jit::playback::context::dlopen_built_dso() JIT: entering: void gcc::jit::playback::context::release_mutex() JIT: exiting: void gcc::jit::playback::context::release_mutex() @@ -121,6 +128,11 @@ JIT: exiting: gcc_jit_context_release JIT: entering: gcc_jit_result_release JIT: deleting result: 0x12f75d0 JIT: entering: virtual gcc::jit::result::~result() +JIT: entering: gcc::jit::tempdir::~tempdir() +JIT: unlinking .s file: /tmp/libgccjit-CKq1M9/fake.s +JIT: unlinking .so file: /tmp/libgccjit-CKq1M9/fake.so +JIT: removing tempdir: /tmp/libgccjit-CKq1M9 +JIT: exiting: gcc::jit::tempdir::~tempdir() JIT: exiting: virtual gcc::jit::result::~result() JIT: exiting: gcc_jit_result_release JIT: gcc::jit::logger::~logger() diff --git a/gcc/jit/jit-logging.h b/gcc/jit/jit-logging.h index 581090c738a..48f223de981 100644 --- a/gcc/jit/jit-logging.h +++ b/gcc/jit/jit-logging.h @@ -112,6 +112,8 @@ log_scope::~log_scope () - class gcc::jit::playback::context + - class gcc::jit::tempdir + - class gcc::jit::result The log_user class keeps the reference-count of a logger up-to-date. */ diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c index a8a281a6684..401977827c4 100644 --- a/gcc/jit/jit-playback.c +++ b/gcc/jit/jit-playback.c @@ -104,8 +104,18 @@ playback::context::context (recording::context *ctxt) playback::context::~context () { JIT_LOG_SCOPE (get_logger ()); - if (m_tempdir) - delete m_tempdir; + + /* Normally the playback::context is responsible for cleaning up the + tempdir (including "fake.so" within the filesystem). + + In the normal case, clean it up now. + + However m_tempdir can be NULL if the context has handed over + responsibility for the tempdir cleanup to the jit::result object, so + that the cleanup can be delayed (see PR jit/64206). If that's the + case this "delete NULL;" is a no-op. */ + delete m_tempdir; + m_functions.release (); } @@ -1554,7 +1564,7 @@ compile () int keep_intermediates = get_bool_option (GCC_JIT_BOOL_OPTION_KEEP_INTERMEDIATES); - m_tempdir = new tempdir (keep_intermediates); + m_tempdir = new tempdir (get_logger (), keep_intermediates); if (!m_tempdir->create ()) return NULL; @@ -1935,7 +1945,37 @@ dlopen_built_dso () add_error (NULL, "%s", error); } if (handle) - result_obj = new result (get_logger (), handle); + { + /* We've successfully dlopened the result; create a + jit::result object to wrap it. + + We're done with the tempdir for now, but if the user + has requested debugging, the user's debugger might not + be capable of dealing with the .so file being unlinked + immediately, so keep it around until after the result + is released. We do this by handing over ownership of + the jit::tempdir to the result. See PR jit/64206. */ + tempdir *handover_tempdir; + if (get_bool_option (GCC_JIT_BOOL_OPTION_DEBUGINFO)) + { + handover_tempdir = m_tempdir; + m_tempdir = NULL; + /* The tempdir will eventually be cleaned up in the + jit::result's dtor. */ + log ("GCC_JIT_BOOL_OPTION_DEBUGINFO was set:" + " handing over tempdir to jit::result"); + } + else + { + handover_tempdir = NULL; + /* ... and retain ownership of m_tempdir so we clean it + up it the playback::context's dtor. */ + log ("GCC_JIT_BOOL_OPTION_DEBUGINFO was not set:" + " retaining ownership of tempdir"); + } + + result_obj = new result (get_logger (), handle, handover_tempdir); + } else result_obj = NULL; diff --git a/gcc/jit/jit-result.c b/gcc/jit/jit-result.c index 67699e0b198..a9330e591c5 100644 --- a/gcc/jit/jit-result.c +++ b/gcc/jit/jit-result.c @@ -25,6 +25,7 @@ along with GCC; see the file COPYING3. If not see #include "jit-common.h" #include "jit-logging.h" #include "jit-result.h" +#include "jit-tempdir.h" namespace gcc { namespace jit { @@ -32,9 +33,10 @@ namespace jit { /* Constructor for gcc::jit::result. */ result:: -result(logger *logger, void *dso_handle) : +result(logger *logger, void *dso_handle, tempdir *tempdir_) : log_user (logger), - m_dso_handle (dso_handle) + m_dso_handle (dso_handle), + m_tempdir (tempdir_) { JIT_LOG_SCOPE (get_logger ()); } @@ -48,6 +50,13 @@ result::~result() JIT_LOG_SCOPE (get_logger ()); dlclose (m_dso_handle); + + /* Responsibility for cleaning up the tempdir (including "fake.so" within + the filesystem) might have been handed to us by the playback::context, + so that the cleanup can be delayed (see PR jit/64206). + + If so, clean it up now. */ + delete m_tempdir; } /* Attempt to locate the given function by name within the diff --git a/gcc/jit/jit-result.h b/gcc/jit/jit-result.h index 59d28a1a364..d9073f26945 100644 --- a/gcc/jit/jit-result.h +++ b/gcc/jit/jit-result.h @@ -29,7 +29,7 @@ namespace jit { class result : public log_user { public: - result(logger *logger, void *dso_handle); + result(logger *logger, void *dso_handle, tempdir *tempdir_); virtual ~result(); @@ -38,6 +38,7 @@ public: private: void *m_dso_handle; + tempdir *m_tempdir; }; } // namespace gcc::jit diff --git a/gcc/jit/jit-tempdir.c b/gcc/jit/jit-tempdir.c index 14c7418469d..856b2469b4f 100644 --- a/gcc/jit/jit-tempdir.c +++ b/gcc/jit/jit-tempdir.c @@ -66,14 +66,16 @@ make_tempdir_path_template () /* The constructor for the jit::tempdir object. The real work is done by the jit::tempdir::create method. */ -gcc::jit::tempdir::tempdir (int keep_intermediates) - : m_keep_intermediates (keep_intermediates), +gcc::jit::tempdir::tempdir (logger *logger, int keep_intermediates) + : log_user (logger), + m_keep_intermediates (keep_intermediates), m_path_template (NULL), m_path_tempdir (NULL), m_path_c_file (NULL), m_path_s_file (NULL), m_path_so_file (NULL) { + JIT_LOG_SCOPE (get_logger ()); } /* Do the real work of creating the on-disk tempdir. @@ -83,16 +85,22 @@ gcc::jit::tempdir::tempdir (int keep_intermediates) bool gcc::jit::tempdir::create () { + JIT_LOG_SCOPE (get_logger ()); + m_path_template = make_tempdir_path_template (); if (!m_path_template) return false; + log ("m_path_template: %s", m_path_template); + /* Create tempdir using mkdtemp. This is created with 0700 perms and is unique. Hence no other (non-root) users should have access to the paths within it. */ m_path_tempdir = mkdtemp (m_path_template); if (!m_path_tempdir) return false; + log ("m_path_tempdir: %s", m_path_tempdir); + m_path_c_file = concat (m_path_tempdir, "/fake.c", NULL); m_path_s_file = concat (m_path_tempdir, "/fake.s", NULL); m_path_so_file = concat (m_path_tempdir, "/fake.so", NULL); @@ -107,17 +115,28 @@ gcc::jit::tempdir::create () gcc::jit::tempdir::~tempdir () { + JIT_LOG_SCOPE (get_logger ()); + if (m_keep_intermediates) fprintf (stderr, "intermediate files written to %s\n", m_path_tempdir); else { /* Clean up .s/.so and tempdir. */ if (m_path_s_file) - unlink (m_path_s_file); + { + log ("unlinking .s file: %s", m_path_s_file); + unlink (m_path_s_file); + } if (m_path_so_file) - unlink (m_path_so_file); + { + log ("unlinking .so file: %s", m_path_so_file); + unlink (m_path_so_file); + } if (m_path_tempdir) - rmdir (m_path_tempdir); + { + log ("removing tempdir: %s", m_path_tempdir); + rmdir (m_path_tempdir); + } } free (m_path_template); diff --git a/gcc/jit/jit-tempdir.h b/gcc/jit/jit-tempdir.h index 49f686cc2a8..2553d72f924 100644 --- a/gcc/jit/jit-tempdir.h +++ b/gcc/jit/jit-tempdir.h @@ -21,6 +21,8 @@ along with GCC; see the file COPYING3. If not see #ifndef JIT_TEMPDIR_H #define JIT_TEMPDIR_H +#include "jit-logging.h" + namespace gcc { namespace jit { @@ -43,10 +45,10 @@ namespace jit { It is normally deleted from the filesystem in the playback::context's dtor, unless GCC_JIT_BOOL_OPTION_KEEP_INTERMEDIATES was set. */ -class tempdir +class tempdir : public log_user { public: - tempdir (int keep_intermediates); + tempdir (logger *logger, int keep_intermediates); ~tempdir (); bool create (); diff --git a/gcc/jit/notes.txt b/gcc/jit/notes.txt index 26f381e4177..7df4a7bdc4e 100644 --- a/gcc/jit/notes.txt +++ b/gcc/jit/notes.txt @@ -84,9 +84,12 @@ Client Code . Generated . libgccjit.so . . │ . . . . │ playback::context dtor . . ──> . . - . . │ Cleanup tempdir . + . . │ Normally we cleanup the tempdir here: . . │ ("fake.so" is unlinked from the . . │ filesystem at this point) + . . │ If the client code requested debuginfo, the + . . │ cleanup happens later (in gcc_jit_result_release) + . . │ to make it easier on the debugger (see PR jit/64206) . . <── . . . . │ . . . . │ end of recording::context::compile () @@ -110,5 +113,9 @@ etc│ . . . . ──────────────────────────> . . . . │ dlclose () the loaded DSO . . │ (code becomes uncallable) + . . │ . . + . . │ If the client code requested debuginfo, then + . . │ cleanup of the tempdir was delayed. + . . │ If that was the case, clean it up now. <─────────────────────────── . . │ . . . .