[RFC] Make solib_add regex-free

Sergio Durigan Junior
2011-08-09T00:53:32Z

Hi,

While working on the lazy debuginfo reading patch, I started to dislike
the way `solib_add' handles the list of shared libraries to have their
symbols read. It basically takes a regex as a first argument, and
matches the current so_list against it.

Well, this is not elegant IMO, especially because I believe the only
place which uses this regex feature is the `sharedlibrary' command.

Now suppose you're not the `sharedlibrary' command, and you already have
a `struct so_list *' that you want to load symbols from (happened to
me). What you would do? Call `solib_add' with the so->name, right?
Well, I did that, and it worked, but Jan has pointed me to a problem
that might happen: in order to call `solib_add' correctly, you would
have to treat the so->name as a regex, i.e., escape symbols and
whatnot. This is a pain and, IMO, error-prone.

That is why I decided to make `solib_add' take a list of `struct so_list
*' (stored in a VEC). I believe this is much more elegant and will
probably help clarify the code.

As for the `sharedlibrary' command, I decided to make a new function
which compiles a regex, matches it against the current so_list, and
return a list containing all the shlibs which will have their symbols
loaded. In order to get an up-to-date list of shared libs, I had to
call `update_solib_list' inside this new function, which made me break
`solib_add' in two functions to avoid calling `update_solib_list'
twice.

I tested this patch on the compile farm, without regressions.

Is this OK to check-in?

2011-08-08 Sergio Durigan Junior <sergiodj@redhat.com>

* solib-irix.c (irix_solib_create_inferior_hook): Uncast `solib_add'
first argument.
* solib-osf.c (osf_solib_create_inferior_hook): Likewise.
* solib-sunos.c (sunos_solib_create_inferior_hook): Likewise.
* solib.c (solib_add_1): New function which does not call
`update_solib_list'.
(solib_add): Split. Call `update_solib_list' and then call
`solib_add_1' to do the hard work. Receive new argument `so_list'.
Delete `pattern' argument.
(solib_match_regex_solist): New function.
(sharedlibrary_command): Call `solib_match_regex_solist' to obtain
a list of shared libraries to have their symbols loaded.
* solib.h: Include "vec.h".
(so_list_p): New type.
(solib_add): Update declaration; receive `VEC(so_list_p) *' as first
argument.

diff --git a/gdb/solib-irix.c b/gdb/solib-irix.c
index 7bb528a..982cf51 100644
--- a/gdb/solib-irix.c
+++ b/gdb/solib-irix.c
@@ -495,7 +495,7 @@ irix_solib_create_inferior_hook (int from_tty)
and will put out an annoying warning.
Delaying the resetting of stop_soon until after symbol loading
suppresses the warning. */
- solib_add ((char *) 0, 0, (struct target_ops *) 0, auto_solib_add);
+ solib_add (NULL, 0, (struct target_ops *) 0, auto_solib_add);
inf->control.stop_soon = NO_STOP_QUIETLY;
}

diff --git a/gdb/solib-osf.c b/gdb/solib-osf.c
index 0905eb6..0258404 100644
--- a/gdb/solib-osf.c
+++ b/gdb/solib-osf.c
@@ -356,7 +356,7 @@ osf_solib_create_inferior_hook (int from_tty)
and will put out an annoying warning.
Delaying the resetting of stop_soon until after symbol loading
suppresses the warning. */
- solib_add ((char *) 0, 0, (struct target_ops *) 0, auto_solib_add);
+ solib_add (NULL, 0, (struct target_ops *) 0, auto_solib_add);
inf->control.stop_soon = NO_STOP_QUIETLY;
}

diff --git a/gdb/solib-sunos.c b/gdb/solib-sunos.c
index 9936038..41c95c6 100644
--- a/gdb/solib-sunos.c
+++ b/gdb/solib-sunos.c
@@ -805,7 +805,7 @@ sunos_solib_create_inferior_hook (int from_tty)
warning (_("shared library handler failed to disable breakpoint"));
}

- solib_add ((char *) 0, 0, (struct target_ops *) 0, auto_solib_add);
+ solib_add (NULL, 0, (struct target_ops *) 0, auto_solib_add);
}

static void
diff --git a/gdb/solib.c b/gdb/solib.c
index 3296ed4..5a1049d 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -888,91 +888,101 @@ libpthread_solib_p (struct so_list *so)
return libpthread_name_p (so->so_name);
}

-/* GLOBAL FUNCTION
+/* LOCAL FUNCTION

- solib_add -- read in symbol info for newly added shared libraries
+ solib_add_1 -- read in symbol info for newly added shared libraries,
+ without calling `update_solib_list'.

SYNOPSIS

- void solib_add (char *pattern, int from_tty, struct target_ops
- *TARGET, int readsyms)
+ static void solib_add_1 (VEC(so_list_p) *so_list,
+ int from_tty, int readsyms)

DESCRIPTION

- Read in symbolic information for any shared objects whose names
- match PATTERN. (If we've already read a shared object's symbol
- info, leave it alone.) If PATTERN is zero, read them all.
-
- If READSYMS is 0, defer reading symbolic information until later
- but still do any needed low level processing.
-
- FROM_TTY and TARGET are as described for update_solib_list, above. */
+ Helper function for `solib_add' below. This function does all the
+ hard job described in `solib_add' comments, but does not call
+ `update_solib_list'. */

-void
-solib_add (char *pattern, int from_tty,
- struct target_ops *target, int readsyms)
+static void
+solib_add_1 (VEC(so_list_p) *so_list, int from_tty, int readsyms)
{
struct so_list *gdb;
+ struct target_so_ops *ops = solib_ops (target_gdbarch);

current_program_space->solib_add_generation++;

- if (pattern)
- {
- char *re_err = re_comp (pattern);
-
- if (re_err)
- error (_("Invalid regexp: %s"), re_err);
- }
-
- update_solib_list (from_tty, target);
-
/* Walk the list of currently loaded shared libraries, and read
symbols for any that match the pattern --- or any whose symbols
aren't already loaded, if no pattern was given. */
{
- int any_matches = 0;
int loaded_any_symbols = 0;
const int flags =
SYMFILE_DEFER_BP_RESET | (from_tty ? SYMFILE_VERBOSE : 0);

for (gdb = so_list_head; gdb; gdb = gdb->next)
- if (! pattern || re_exec (gdb->so_name))
- {
- /* Normally, we would read the symbols from that library
- only if READSYMS is set. However, we're making a small
- exception for the pthread library, because we sometimes
- need the library symbols to be loaded in order to provide
- thread support (x86-linux for instance). */
- const int add_this_solib =
- (readsyms || libpthread_solib_p (gdb));
-
- any_matches = 1;
- if (add_this_solib)
- {
- if (gdb->symbols_loaded)
- {
- /* If no pattern was given, be quiet for shared
- libraries we have already loaded. */
- if (pattern && (from_tty || info_verbose))
- printf_unfiltered (_("Symbols already loaded for %s\n"),
- gdb->so_name);
- }
- else if (solib_read_symbols (gdb, flags))
- loaded_any_symbols = 1;
- }
- }
+ {
+ int add_this_solib;
+
+ if (so_list)
+ {
+ int iter, found = 0;
+ struct so_list *cur_so;
+
+ for (iter = 0;
+ VEC_iterate (so_list_p, so_list, iter, cur_so);
+ iter++)
+ {
+ if (ops->same)
+ {
+ if (ops->same (gdb, cur_so))
+ {
+ found = 1;
+ break;
+ }
+ }
+ else
+ {
+ if (!filename_cmp (gdb->so_original_name,
+ cur_so->so_original_name))
+ {
+ found = 1;
+ break;
+ }
+ }
+ }
+
+ if (!found)
+ /* We are not adding this shared library's symbols. */
+ continue;
+ }
+ /* Normally, we would read the symbols from that library
+ only if READSYMS is set. However, we're making a small
+ exception for the pthread library, because we sometimes
+ need the library symbols to be loaded in order to provide
+ thread support (x86-linux for instance). */
+ add_this_solib = (readsyms || libpthread_solib_p (gdb));
+
+ if (add_this_solib)
+ {
+ if (gdb->symbols_loaded)
+ {
+ /* If no so_list was given, be quiet for shared
+ libraries we have already loaded. */
+ if (so_list && (from_tty || info_verbose))
+ printf_unfiltered (_("Symbols already loaded for %s\n"),
+ gdb->so_name);
+ }
+ else if (solib_read_symbols (gdb, flags))
+ loaded_any_symbols = 1;
+ }
+ }

if (loaded_any_symbols)
breakpoint_re_set ();

- if (from_tty && pattern && ! any_matches)
- printf_unfiltered
- ("No loaded shared libraries match the pattern `%s'.\n", pattern);
-
if (loaded_any_symbols)
{
- struct target_so_ops *ops = solib_ops (target_gdbarch);
-
/* Getting new symbols may change our opinion about what is
frameless. */
reinit_frame_cache ();
@@ -982,6 +992,34 @@ solib_add (char *pattern, int from_tty,
}
}

+/* GLOBAL FUNCTION
+
+ solib_add -- read in symbol info for newly added shared libraries
+
+ SYNOPSIS
+
+ void solib_add (VEC(so_list_p) *so_list, int from_tty,
+ struct target_ops *target, int readsyms)
+
+ DESCRIPTION
+
+ Read in symbolic information for any shared objects listed in
+ SO_LIST. ((If we've already read a shared object's symbol info,
+ leave it alone.) If SO_LIST is NULL, read them all.
+
+ If READSYMS is 0, defer reading symbolic information until later
+ but still do any needed low level processing inside `solib_add_1'.
+
+ FROM_TTY and TARGET are as described for update_solib_list, above. */
+
+void
+solib_add (VEC(so_list_p) *so_list, int from_tty,
+ struct target_ops *target, int readsyms)
+{
+ update_solib_list (from_tty, target);
+ solib_add_1 (so_list, from_tty, readsyms);
+}
+

/*

@@ -1272,6 +1310,54 @@ in_solib_dynsym_resolve_code (CORE_ADDR pc)

LOCAL FUNCTION

+ solib_match_regex_solist -- compile a regex and match shared libraries
+ against it.
+
+ SYNOPSIS
+
+ static VEC(so_list_p) *solib_match_regex_solist (const char *pattern)
+
+ DESCRIPTION
+
+ This function compiles a regex represented by PATTERN, and returns a list
+ of shared libraries matching it. Returns NULL if PATTERN is NULL or if
+ no match was found.
+
+ This function calls `update_solib_list' in order to refresh shared the
+ shlib list. If you are going to call `solib_add' after calling this
+ function, call `solib_add_1' instead, because this version does not
+ call `update_solib_list', thus saving time.
+
+ */
+
+static VEC(so_list_p) *
+solib_match_regex_solist (const char *pattern,
+ struct target_ops *target, int from_tty)
+{
+ char *err;
+ struct so_list *iter;
+ VEC(so_list_p) *solist = NULL;
+
+ if (!pattern)
+ return NULL;
+
+ err = re_comp (pattern);
+
+ if (err)
+ error (_("Invalid regexp: `%s'"), err);
+
+ update_solib_list (from_tty, target);
+ for (iter = so_list_head; iter; iter = iter->next)
+ if (re_exec (iter->so_name))
+ VEC_safe_push (so_list_p, solist, iter);
+
+ return solist;
+}
+
+/*
+
+ LOCAL FUNCTION
+
sharedlibrary_command -- handle command to explicitly add library

SYNOPSIS
@@ -1285,8 +1371,19 @@ in_solib_dynsym_resolve_code (CORE_ADDR pc)
static void
sharedlibrary_command (char *args, int from_tty)
{
+ struct cleanup *c;
+ VEC(so_list_p) *solist;
+
dont_repeat ();
- solib_add (args, from_tty, (struct target_ops *) 0, 1);
+
+ solist = solib_match_regex_solist (args, (struct target_ops *) 0,
+ from_tty);
+ if (!solist)
+ error (_("No shared library matched the pattern `%s'."), args);
+
+ c = make_cleanup (VEC_cleanup (so_list_p), &solist);
+ solib_add_1 (solist, from_tty, 1);
+ do_cleanups (c);
}

/* LOCAL FUNCTION
diff --git a/gdb/solib.h b/gdb/solib.h
index c473d85..f2055e7 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -21,12 +21,17 @@
#ifndef SOLIB_H
#define SOLIB_H

+#include "vec.h"
+
/* Forward decl's for prototypes */
struct so_list;
struct target_ops;
struct target_so_ops;
struct program_space;

+typedef struct so_list *so_list_p;
+DEF_VEC_P(so_list_p);
+
/* Called when we free all symtabs, to free the shared library information
as well. */

@@ -34,7 +39,7 @@ extern void clear_solib (void);

/* Called to add symbols from a shared library to gdb's symbol table. */

-extern void solib_add (char *, int, struct target_ops *, int);
+extern void solib_add (VEC(so_list_p) *, int, struct target_ops *, int);
extern int solib_read_symbols (struct so_list *, int);

/* Function to be called when the inferior starts up, to discover the