From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12557 invoked by alias); 9 Aug 2011 00:53:32 -0000 Received: (qmail 12549 invoked by uid 22791); 9 Aug 2011 00:53:30 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 09 Aug 2011 00:53:10 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p790r98f006544 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 8 Aug 2011 20:53:09 -0400 Received: from psique ([10.3.112.5]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p790r52v013653 for ; Mon, 8 Aug 2011 20:53:07 -0400 From: Sergio Durigan Junior To: gdb-patches@sourceware.org Subject: [RFC] Make solib_add regex-free Date: Tue, 09 Aug 2011 00:53:00 -0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-08/txt/msg00165.txt.bz2 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 * 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