[RFA,04/14] Introduce gdb_dlopen_up
Commit Message
This introduces gdb_dlopen_up, a unique_ptr that can close a dlopen'd
library. All the functions working with dlopen handles are updated to
use this new type.
I did not try to build this on Windows.
2017-04-07 Tom Tromey <tom@tromey.com>
* jit.c (struct jit_reader): Declare separately. Add constructor
and destructor. Change type of "handle".
(loaded_jit_reader): Define separately.
(jit_reader_load): Update. New "new".
(jit_reader_unload_command): Use "delete".
* gdb-dlfcn.h (struct dlclose_deleter): New.
(gdb_dlopen_up): New typedef.
(gdb_dlopen, gdb_dlsym): Update types.
(gdb_dlclose): Remove.
* gdb-dlfcn.c (gdb_dlopen): Return a gdb_dlopen_up.
(gdb_dlsym): Change type of "handle".
(make_cleanup_dlclose): Remove.
(dlclose_deleter::operator()): Rename from gdb_dlclose.
* compile/compile-c-support.c (load_libcc): Update.
---
gdb/ChangeLog | 17 ++++++++++++++++
gdb/compile/compile-c-support.c | 6 ++++--
gdb/gdb-dlfcn.c | 45 ++++++++++++-----------------------------
gdb/gdb-dlfcn.h | 24 ++++++++++++----------
gdb/jit.c | 42 +++++++++++++++++++++-----------------
5 files changed, 70 insertions(+), 64 deletions(-)
Comments
On 2017-04-08 16:11, Tom Tromey wrote:
> This introduces gdb_dlopen_up, a unique_ptr that can close a dlopen'd
> library. All the functions working with dlopen handles are updated to
> use this new type.
>
> I did not try to build this on Windows.
Well, apparently the build on Windows was broken by the guy that pushed
the ptid patch last week, can you believe it? With that fixed (pushing
a the fix in a moment) and your patch, it builds fine with
--host=x86_64-w64-mingw32.
Otherwise, LGTM.
Thanks,
Simon
Hi!
On 04/08/2017 09:11 PM, Tom Tromey wrote:
> This introduces gdb_dlopen_up, a unique_ptr that can close a dlopen'd
> library. All the functions working with dlopen handles are updated to
> use this new type.
>
gdb_dlopen_up would sound to me like something that owns a
pointer to dlopen? Since this is a unique pointer that instead
owns a _handle_ that is returned by dlopen, I'd suggest naming
it dlhandle_up or gdb_dlhandle_up instead.
Thanks,
Pedro Alves
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> gdb_dlopen_up would sound to me like something that owns a
Pedro> pointer to dlopen? Since this is a unique pointer that instead
Pedro> owns a _handle_ that is returned by dlopen, I'd suggest naming
Pedro> it dlhandle_up or gdb_dlhandle_up instead.
I made this change.
Tom
@@ -1,5 +1,22 @@
2017-04-07 Tom Tromey <tom@tromey.com>
+ * jit.c (struct jit_reader): Declare separately. Add constructor
+ and destructor. Change type of "handle".
+ (loaded_jit_reader): Define separately.
+ (jit_reader_load): Update. New "new".
+ (jit_reader_unload_command): Use "delete".
+ * gdb-dlfcn.h (struct dlclose_deleter): New.
+ (gdb_dlopen_up): New typedef.
+ (gdb_dlopen, gdb_dlsym): Update types.
+ (gdb_dlclose): Remove.
+ * gdb-dlfcn.c (gdb_dlopen): Return a gdb_dlopen_up.
+ (gdb_dlsym): Change type of "handle".
+ (make_cleanup_dlclose): Remove.
+ (dlclose_deleter::operator()): Rename from gdb_dlclose.
+ * compile/compile-c-support.c (load_libcc): Update.
+
+2017-04-07 Tom Tromey <tom@tromey.com>
+
* symtab.h (find_pcs_for_symtab_line): Change return type.
* symtab.c (find_pcs_for_symtab_line): Change return type.
* python/py-linetable.c (build_line_table_tuple_from_pcs): Change
@@ -75,12 +75,11 @@ c_get_range_decl_name (const struct dynamic_prop *prop)
static gcc_c_fe_context_function *
load_libcc (void)
{
- void *handle;
gcc_c_fe_context_function *func;
/* gdb_dlopen will call error () on an error, so no need to check
value. */
- handle = gdb_dlopen (STRINGIFY (GCC_C_FE_LIBCC));
+ gdb_dlopen_up handle = gdb_dlopen (STRINGIFY (GCC_C_FE_LIBCC));
func = (gcc_c_fe_context_function *) gdb_dlsym (handle,
STRINGIFY (GCC_C_FE_CONTEXT));
@@ -88,6 +87,9 @@ load_libcc (void)
error (_("could not find symbol %s in library %s"),
STRINGIFY (GCC_C_FE_CONTEXT),
STRINGIFY (GCC_C_FE_LIBCC));
+
+ /* Leave the library open. */
+ handle.release ();
return func;
}
@@ -31,27 +31,20 @@
#ifdef NO_SHARED_LIB
-void *
+gdb_dlopen_up
gdb_dlopen (const char *filename)
{
gdb_assert_not_reached ("gdb_dlopen should not be called on this platform.");
}
void *
-gdb_dlsym (void *handle, const char *symbol)
+gdb_dlsym (const gdb_dlopen_up &handle, const char *symbol)
{
gdb_assert_not_reached ("gdb_dlsym should not be called on this platform.");
}
-struct cleanup *
-make_cleanup_dlclose (void *handle)
-{
- gdb_assert_not_reached ("make_cleanup_dlclose should not be called on this "
- "platform.");
-}
-
-int
-gdb_dlclose (void *handle)
+void
+dlclose_deleter::operator() (void *handle) const
{
gdb_assert_not_reached ("gdb_dlclose should not be called on this platform.");
}
@@ -64,7 +57,7 @@ is_dl_available (void)
#else /* NO_SHARED_LIB */
-void *
+gdb_dlopen_up
gdb_dlopen (const char *filename)
{
void *result;
@@ -74,7 +67,7 @@ gdb_dlopen (const char *filename)
result = (void *) LoadLibrary (filename);
#endif
if (result != NULL)
- return result;
+ return gdb_dlopen_up (result);
#ifdef HAVE_DLFCN_H
error (_("Could not load %s: %s"), filename, dlerror());
@@ -97,37 +90,25 @@ gdb_dlopen (const char *filename)
}
void *
-gdb_dlsym (void *handle, const char *symbol)
+gdb_dlsym (const gdb_dlopen_up &handle, const char *symbol)
{
#ifdef HAVE_DLFCN_H
- return dlsym (handle, symbol);
+ return dlsym (handle.get (), symbol);
#elif __MINGW32__
- return (void *) GetProcAddress ((HMODULE) handle, symbol);
+ return (void *) GetProcAddress ((HMODULE) handle.get (), symbol);
#endif
}
-int
-gdb_dlclose (void *handle)
+void
+dlclose_deleter::operator() (void *handle) const
{
#ifdef HAVE_DLFCN_H
- return dlclose (handle);
+ dlclose (handle);
#elif __MINGW32__
- return !((int) FreeLibrary ((HMODULE) handle));
+ FreeLibrary ((HMODULE) handle);
#endif
}
-static void
-do_dlclose_cleanup (void *handle)
-{
- gdb_dlclose (handle);
-}
-
-struct cleanup *
-make_cleanup_dlclose (void *handle)
-{
- return make_cleanup (do_dlclose_cleanup, handle);
-}
-
int
is_dl_available (void)
{
@@ -20,26 +20,28 @@
#ifndef GDB_DLFCN_H
#define GDB_DLFCN_H
+/* A deleter that closes an open dynamic library. */
+
+struct dlclose_deleter
+{
+ void operator() (void *handle) const;
+};
+
+/* A unique pointer that points to a dynamic library. */
+
+typedef std::unique_ptr<void, dlclose_deleter> gdb_dlopen_up;
+
/* Load the dynamic library file named FILENAME, and return a handle
for that dynamic library. Return NULL if the loading fails for any
reason. */
-void *gdb_dlopen (const char *filename);
+gdb_dlopen_up gdb_dlopen (const char *filename);
/* Return the address of the symbol named SYMBOL inside the shared
library whose handle is HANDLE. Return NULL when the symbol could
not be found. */
-void *gdb_dlsym (void *handle, const char *symbol);
-
-/* Install a cleanup routine which closes the handle HANDLE. */
-
-struct cleanup *make_cleanup_dlclose (void *handle);
-
-/* Cleanup the shared object pointed to by HANDLE. Return 0 on success
- and nonzero on failure. */
-
-int gdb_dlclose (void *handle);
+void *gdb_dlsym (const gdb_dlopen_up &handle, const char *symbol);
/* Return non-zero if the dynamic library functions are available on
this platform. */
@@ -151,14 +151,29 @@ bfd_open_from_target_memory (CORE_ADDR addr, ULONGEST size, char *target)
mem_bfd_iovec_stat);
}
+struct jit_reader
+{
+ jit_reader (struct gdb_reader_funcs *f, gdb_dlopen_up &&h)
+ : functions (f), handle (std::move (h))
+ {
+ }
+
+ ~jit_reader ()
+ {
+ functions->destroy (functions);
+ }
+
+ jit_reader (const jit_reader &) = delete;
+ jit_reader &operator= (const jit_reader &) = delete;
+
+ struct gdb_reader_funcs *functions;
+ gdb_dlopen_up handle;
+};
+
/* One reader that has been loaded successfully, and can potentially be used to
parse debug info. */
-static struct jit_reader
-{
- struct gdb_reader_funcs *functions;
- void *handle;
-} *loaded_jit_reader = NULL;
+static struct jit_reader *loaded_jit_reader = NULL;
typedef struct gdb_reader_funcs * (reader_init_fn_type) (void);
static const char *reader_init_fn_sym = "gdb_init_reader";
@@ -168,17 +183,13 @@ static const char *reader_init_fn_sym = "gdb_init_reader";
static struct jit_reader *
jit_reader_load (const char *file_name)
{
- void *so;
reader_init_fn_type *init_fn;
- struct jit_reader *new_reader = NULL;
struct gdb_reader_funcs *funcs = NULL;
- struct cleanup *old_cleanups;
if (jit_debug)
fprintf_unfiltered (gdb_stdlog, _("Opening shared object %s.\n"),
file_name);
- so = gdb_dlopen (file_name);
- old_cleanups = make_cleanup_dlclose (so);
+ gdb_dlopen_up so = gdb_dlopen (file_name);
init_fn = (reader_init_fn_type *) gdb_dlsym (so, reader_init_fn_sym);
if (!init_fn)
@@ -192,12 +203,7 @@ jit_reader_load (const char *file_name)
if (funcs->reader_version != GDB_READER_INTERFACE_VERSION)
error (_("Reader version does not match GDB version."));
- new_reader = XCNEW (struct jit_reader);
- new_reader->functions = funcs;
- new_reader->handle = so;
-
- discard_cleanups (old_cleanups);
- return new_reader;
+ return new jit_reader (funcs, std::move (so));
}
/* Provides the jit-reader-load command. */
@@ -240,10 +246,8 @@ jit_reader_unload_command (char *args, int from_tty)
reinit_frame_cache ();
jit_inferior_exit_hook (current_inferior ());
- loaded_jit_reader->functions->destroy (loaded_jit_reader->functions);
- gdb_dlclose (loaded_jit_reader->handle);
- xfree (loaded_jit_reader);
+ delete loaded_jit_reader;
loaded_jit_reader = NULL;
}