From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1875 invoked by alias); 9 Jan 2015 17:04:50 -0000 Mailing-List: contact jit-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Subscribe: Sender: jit-owner@gcc.gnu.org Received: (qmail 1850 invoked by uid 89); 9 Jan 2015 17:04:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.98.5 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=2.0 required=5.0 tests=AWL,BAYES_00,BODY_8BITS,GARBLED_BODY,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-Spam-Status: No, score=2.0 required=5.0 tests=AWL,BAYES_00,BODY_8BITS,GARBLED_BODY,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on sourceware.org X-Spam-Level: ** X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com From: David Malcolm To: jit@gcc.gnu.org, gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [PATCH, committed] PR jit/64206: delay cleanup of tempdir if the user has requested debuginfo Date: Thu, 01 Jan 2015 00:00:00 -0000 Message-Id: <1420823607-27794-1-git-send-email-dmalcolm@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-SW-Source: 2015-q1/txt/msg00010.txt.bz2 Committed to trunk as r219395. 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. --- .../docs/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 +++- 8 files changed, 109 insertions(+), 17 deletions(-) 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 a96d80f..113dc35 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 581090c..48f223d 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 a8a281a..4019778 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 67699e0..a9330e5 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 59d28a1..d9073f2 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 14c7418..856b246 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 49f686c..2553d72 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 26f381e..7df4a7b 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. <─────────────────────────── . . │ . . . . -- 1.8.5.3