* [RFA 0/2] C++-ify some breakpoint subclasses a bit more @ 2017-06-04 22:54 Tom Tromey 2017-06-04 22:54 ` [RFA 2/2] C++-ify break-catch-throw Tom Tromey 2017-06-04 22:54 ` [RFA 1/2] C++-ify break-catch-sig Tom Tromey 0 siblings, 2 replies; 17+ messages in thread From: Tom Tromey @ 2017-06-04 22:54 UTC (permalink / raw) To: gdb-patches This short series builds on Simon Marchi's recent C++-ification patch. It updates a couple of breakpoint subclasses to be more C++-ish, using string and vector rather than manual memory management and VEC. Regression tested on the buildbot. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFA 2/2] C++-ify break-catch-throw 2017-06-04 22:54 [RFA 0/2] C++-ify some breakpoint subclasses a bit more Tom Tromey @ 2017-06-04 22:54 ` Tom Tromey 2017-06-05 8:54 ` Simon Marchi 2017-06-05 10:21 ` Pedro Alves 2017-06-04 22:54 ` [RFA 1/2] C++-ify break-catch-sig Tom Tromey 1 sibling, 2 replies; 17+ messages in thread From: Tom Tromey @ 2017-06-04 22:54 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This changes exception_catchpoint to be more of a C++ class, using std::string and std::vector, and updating the users. gdb/ChangeLog 2017-06-04 Tom Tromey <tom@tromey.com> * break-catch-throw.c (struct exception_catchpoint) <exception_rx>: Now a std::string. <pattern>: Now a unique_ptr. (~exception_catchpoint, check_status_exception_catchpoint) (print_one_detail_exception_catchpoint): Update. (handle_gnu_v3_exceptions): Change type of except_rx. (extract_exception_regexp): Return a std::string. (catch_exception_command_1): Update. --- gdb/ChangeLog | 11 +++++++++++ gdb/break-catch-throw.c | 49 +++++++++++++++++++++---------------------------- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index b89b47d..7be1267 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,16 @@ 2017-06-04 Tom Tromey <tom@tromey.com> + * break-catch-throw.c (struct exception_catchpoint) + <exception_rx>: Now a std::string. + <pattern>: Now a unique_ptr. + (~exception_catchpoint, check_status_exception_catchpoint) + (print_one_detail_exception_catchpoint): Update. + (handle_gnu_v3_exceptions): Change type of except_rx. + (extract_exception_regexp): Return a std::string. + (catch_exception_command_1): Update. + +2017-06-04 Tom Tromey <tom@tromey.com> + * break-catch-sig.c (gdb_signal_type): Remove typedef. (struct signal_catchpoint) <signals_to_be_caught>: Now a std::vector. diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c index 7731c5e..5e88ad5 100644 --- a/gdb/break-catch-throw.c +++ b/gdb/break-catch-throw.c @@ -82,15 +82,15 @@ struct exception_catchpoint : public breakpoint enum exception_event_kind kind; - /* If non-NULL, an xmalloc'd string holding the source form of the - regular expression to match against. */ + /* If not empty, a string holding the source form of the regular + expression to match against. */ - char *exception_rx; + std::string exception_rx; - /* If non-NULL, an xmalloc'd, compiled regular expression which is + /* If non-NULL, a compiled regular expression which is used to determine which exceptions to stop on. */ - regex_t *pattern; + std::unique_ptr<regex_t> pattern; }; \f @@ -144,9 +144,8 @@ classify_exception_breakpoint (struct breakpoint *b) exception_catchpoint::~exception_catchpoint () { - xfree (this->exception_rx); if (this->pattern != NULL) - regfree (this->pattern); + regfree (this->pattern.get ()); } /* Implement the 'check_status' method. */ @@ -185,7 +184,7 @@ check_status_exception_catchpoint (struct bpstats *bs) if (!type_name.empty ()) { - if (regexec (self->pattern, type_name.c_str (), 0, NULL, 0) != 0) + if (regexec (self->pattern.get (), type_name.c_str (), 0, NULL, 0) != 0) bs->stop = 0; } } @@ -321,10 +320,10 @@ print_one_detail_exception_catchpoint (const struct breakpoint *b, const struct exception_catchpoint *cp = (const struct exception_catchpoint *) b; - if (cp->exception_rx != NULL) + if (!cp->exception_rx.empty ()) { uiout->text (_("\tmatching: ")); - uiout->field_string ("regexp", cp->exception_rx); + uiout->field_string ("regexp", cp->exception_rx.c_str ()); uiout->text ("\n"); } } @@ -373,18 +372,17 @@ print_recreate_exception_catchpoint (struct breakpoint *b, } static void -handle_gnu_v3_exceptions (int tempflag, char *except_rx, +handle_gnu_v3_exceptions (int tempflag, std::string &&except_rx, const char *cond_string, enum exception_event_kind ex_event, int from_tty) { - regex_t *pattern = NULL; + std::unique_ptr<regex_t> pattern; - if (except_rx != NULL) + if (!except_rx.empty ()) { - pattern = XNEW (regex_t); - make_cleanup (xfree, pattern); + pattern.reset (new regex_t); - compile_rx_or_error (pattern, except_rx, + compile_rx_or_error (pattern.get (), except_rx.c_str (), _("invalid type-matching regexp")); } @@ -396,8 +394,8 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx, the right thing. */ cp->type = bp_breakpoint; cp->kind = ex_event; - cp->exception_rx = except_rx; - cp->pattern = pattern; + cp->exception_rx = std::move (except_rx); + cp->pattern = std::move (pattern); re_set_exception_catchpoint (cp.get ()); @@ -415,7 +413,7 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx, STRING is updated to point to the "if" token, if it exists, or to the end of the string. */ -static char * +static std::string extract_exception_regexp (const char **string) { const char *start; @@ -440,8 +438,8 @@ extract_exception_regexp (const char **string) *string = last; if (last_space > start) - return savestring (start, last_space - start); - return NULL; + return std::string (start, last_space - start); + return std::string (); } /* Deal with "catch catch", "catch throw", and "catch rethrow" @@ -452,17 +450,14 @@ catch_exception_command_1 (enum exception_event_kind ex_event, char *arg_entry, int tempflag, int from_tty) { - char *except_rx; const char *cond_string = NULL; - struct cleanup *cleanup; const char *arg = arg_entry; if (!arg) arg = ""; arg = skip_spaces_const (arg); - except_rx = extract_exception_regexp (&arg); - cleanup = make_cleanup (xfree, except_rx); + std::string except_rx = extract_exception_regexp (&arg); cond_string = ep_parse_optional_if_clause (&arg); @@ -474,10 +469,8 @@ catch_exception_command_1 (enum exception_event_kind ex_event, && ex_event != EX_EVENT_RETHROW) error (_("Unsupported or unknown exception event; cannot catch it")); - handle_gnu_v3_exceptions (tempflag, except_rx, cond_string, + handle_gnu_v3_exceptions (tempflag, std::move (except_rx), cond_string, ex_event, from_tty); - - discard_cleanups (cleanup); } /* Implementation of "catch catch" command. */ -- 2.9.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 2/2] C++-ify break-catch-throw 2017-06-04 22:54 ` [RFA 2/2] C++-ify break-catch-throw Tom Tromey @ 2017-06-05 8:54 ` Simon Marchi 2017-06-05 19:10 ` Tom Tromey 2017-06-05 10:21 ` Pedro Alves 1 sibling, 1 reply; 17+ messages in thread From: Simon Marchi @ 2017-06-05 8:54 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches > --- a/gdb/break-catch-throw.c > +++ b/gdb/break-catch-throw.c > @@ -82,15 +82,15 @@ struct exception_catchpoint : public breakpoint > > enum exception_event_kind kind; > > - /* If non-NULL, an xmalloc'd string holding the source form of the > - regular expression to match against. */ > + /* If not empty, a string holding the source form of the regular > + expression to match against. */ > > - char *exception_rx; > + std::string exception_rx; > > - /* If non-NULL, an xmalloc'd, compiled regular expression which is > + /* If non-NULL, a compiled regular expression which is > used to determine which exceptions to stop on. */ > > - regex_t *pattern; > + std::unique_ptr<regex_t> pattern; From what I understand, we were freeing the regex with regfree, but we weren't freeing the regex_t object, which is allocated separately, is that right? Or what is freed in handle_gnu_v3_exceptions by the cleanup, and then referenced later? Either way your code LGTM. Simon ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 2/2] C++-ify break-catch-throw 2017-06-05 8:54 ` Simon Marchi @ 2017-06-05 19:10 ` Tom Tromey 0 siblings, 0 replies; 17+ messages in thread From: Tom Tromey @ 2017-06-05 19:10 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches >>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes: Simon> From what I understand, we were freeing the regex with regfree, but we Simon> weren't freeing the regex_t object, which is allocated separately, is Simon> that right? Yes, I think that's a leak in the current code. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 2/2] C++-ify break-catch-throw 2017-06-04 22:54 ` [RFA 2/2] C++-ify break-catch-throw Tom Tromey 2017-06-05 8:54 ` Simon Marchi @ 2017-06-05 10:21 ` Pedro Alves 2017-06-05 10:33 ` Pedro Alves 1 sibling, 1 reply; 17+ messages in thread From: Pedro Alves @ 2017-06-05 10:21 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 06/04/2017 11:53 PM, Tom Tromey wrote: > @@ -452,17 +450,14 @@ catch_exception_command_1 (enum exception_event_kind ex_event, > char *arg_entry, > int tempflag, int from_tty) > { > - char *except_rx; > const char *cond_string = NULL; > - struct cleanup *cleanup; > const char *arg = arg_entry; > > if (!arg) > arg = ""; > arg = skip_spaces_const (arg); > > - except_rx = extract_exception_regexp (&arg); > - cleanup = make_cleanup (xfree, except_rx); > + std::string except_rx = extract_exception_regexp (&arg); > > cond_string = ep_parse_optional_if_clause (&arg); > > @@ -474,10 +469,8 @@ catch_exception_command_1 (enum exception_event_kind ex_event, > && ex_event != EX_EVENT_RETHROW) > error (_("Unsupported or unknown exception event; cannot catch it")); > > - handle_gnu_v3_exceptions (tempflag, except_rx, cond_string, > + handle_gnu_v3_exceptions (tempflag, std::move (except_rx), cond_string, > ex_event, from_tty); > - > - discard_cleanups (cleanup); > } Something looks suspicious to me -- compile_rx_or_error returns with an installed cleanup that calls regfree, and handle_gnu_v3_exceptions leaves it installed too. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 2/2] C++-ify break-catch-throw 2017-06-05 10:21 ` Pedro Alves @ 2017-06-05 10:33 ` Pedro Alves 2017-06-05 10:36 ` Pedro Alves 0 siblings, 1 reply; 17+ messages in thread From: Pedro Alves @ 2017-06-05 10:33 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 06/05/2017 11:21 AM, Pedro Alves wrote: > On 06/04/2017 11:53 PM, Tom Tromey wrote: >> @@ -452,17 +450,14 @@ catch_exception_command_1 (enum exception_event_kind ex_event, >> char *arg_entry, >> int tempflag, int from_tty) >> { >> - char *except_rx; >> const char *cond_string = NULL; >> - struct cleanup *cleanup; >> const char *arg = arg_entry; >> >> if (!arg) >> arg = ""; >> arg = skip_spaces_const (arg); >> >> - except_rx = extract_exception_regexp (&arg); >> - cleanup = make_cleanup (xfree, except_rx); >> + std::string except_rx = extract_exception_regexp (&arg); >> >> cond_string = ep_parse_optional_if_clause (&arg); >> >> @@ -474,10 +469,8 @@ catch_exception_command_1 (enum exception_event_kind ex_event, >> && ex_event != EX_EVENT_RETHROW) >> error (_("Unsupported or unknown exception event; cannot catch it")); >> >> - handle_gnu_v3_exceptions (tempflag, except_rx, cond_string, >> + handle_gnu_v3_exceptions (tempflag, std::move (except_rx), cond_string, >> ex_event, from_tty); >> - >> - discard_cleanups (cleanup); >> } > > Something looks suspicious to me -- compile_rx_or_error returns with > an installed cleanup that calls regfree, and handle_gnu_v3_exceptions > leaves it installed too. Maybe the cleanest would be to add a wrapper around regex_t, and replace compile_rx_or_error with a ctor that throws, like: class gdb_regex { public: // replaces old compile_rx_or_error gdb_regex (const char *rx, const char *message) { gdb_assert (rx != NULL); int code = regcomp (&m_pattern, rx, REG_NOSUB); if (code != 0) { gdb::unique_xmalloc_ptr<char> err = get_regcomp_error (code, &m_pattern); error (("%s: %s"), message, err.get ()); } } ~gdb_regex () { regfree (&m_pattern); } // add convenience methods as needed. private: regex_t m_pattern; }; and use it throughout. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 2/2] C++-ify break-catch-throw 2017-06-05 10:33 ` Pedro Alves @ 2017-06-05 10:36 ` Pedro Alves 2017-06-05 12:29 ` Pedro Alves 0 siblings, 1 reply; 17+ messages in thread From: Pedro Alves @ 2017-06-05 10:36 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 06/05/2017 11:33 AM, Pedro Alves wrote: > class gdb_regex > { > public: > // replaces old compile_rx_or_error > gdb_regex (const char *rx, const char *message) > { Maybe call it "compiled_regex" instead, since you wouldn't be able to end up with a not-compiled-yet regex with this. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 2/2] C++-ify break-catch-throw 2017-06-05 10:36 ` Pedro Alves @ 2017-06-05 12:29 ` Pedro Alves 2017-06-05 12:32 ` Pedro Alves 2017-06-05 19:27 ` Tom Tromey 0 siblings, 2 replies; 17+ messages in thread From: Pedro Alves @ 2017-06-05 12:29 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 06/05/2017 11:36 AM, Pedro Alves wrote: > On 06/05/2017 11:33 AM, Pedro Alves wrote: >> class gdb_regex >> { >> public: >> // replaces old compile_rx_or_error >> gdb_regex (const char *rx, const char *message) >> { > > Maybe call it "compiled_regex" instead, since you wouldn't > be able to end up with a not-compiled-yet regex with this. > So something like this. Builds and gdb starts, but I have not regtested it. In a couple places, this either forces moving the regex object to the heap, or to wrap it in gdb::optional. In the cases where we already have to keep the regex string around, it ends up being not maximally efficient memory-wise, but I don't think it really matters. We're considering std::string for those same strings, which grows the structs more than that, anyway (for size/capacity). For linux-tdep.cmapping_is_anonymous_p, I was first considering std::aligned_storage, but switched back to gdb::optional before posting, because std::aligned_storage complicates the code a little bit (I mean, forces readers to learn about std::aligned_storage while here the redundant "has value" variable that gdb::optional has internally doesn't really matter. We could move the mapping_regexes struct to the heap too, since is a one-shot initialization. (I see now that I left the "tristate" comment behind.) I didn't use "compiled_regex" for struct name, simply because "gdb_regex.h" already existed. But I'd still consider renaming it. WDYT? From 26a9200f3b8f94f24b4c026413ffd7968fd3ea01 Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Mon, 5 Jun 2017 12:48:46 +0100 Subject: [PATCH] gdb_regex --- gdb/Makefile.in | 2 ++ gdb/ada-lang.c | 30 ++++++---------- gdb/break-catch-throw.c | 21 +++++------ gdb/breakpoint.c | 19 +++------- gdb/cli/cli-cmds.c | 21 ++--------- gdb/cli/cli-decode.c | 11 +++--- gdb/cli/cli-decode.h | 5 ++- gdb/gdb_regex.h | 26 +++++++++++--- gdb/linux-tdep.c | 93 +++++++++++++++++++++++++------------------------ gdb/probe.c | 19 +++++----- gdb/skip.c | 24 +++---------- gdb/symtab.c | 42 ++++++++-------------- gdb/utils.c | 52 --------------------------- 13 files changed, 135 insertions(+), 230 deletions(-) diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 8be73ba..2156438 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -1104,6 +1104,7 @@ SFILES = \ gdb_bfd.c \ gdb-dlfcn.c \ gdb_obstack.c \ + gdb_regex.c \ gdb_usleep.c \ gdbarch.c \ gdbarch-selftests.c \ @@ -1717,6 +1718,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \ gdb_bfd.o \ gdb-dlfcn.o \ gdb_obstack.o \ + gdb_regex.o \ gdb_usleep.o \ gdb_vecs.o \ gdbarch.o \ diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index f90907a..e338cd1 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -13219,14 +13219,14 @@ sort_remove_dups_ada_exceptions_list (VEC(ada_exc_info) **exceptions, gets pushed. */ static void -ada_add_standard_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) +ada_add_standard_exceptions (gdb_regex *preg, VEC(ada_exc_info) **exceptions) { int i; for (i = 0; i < ARRAY_SIZE (standard_exc); i++) { if (preg == NULL - || regexec (preg, standard_exc[i], 0, NULL, 0) == 0) + || preg->exec (standard_exc[i], 0, NULL, 0) == 0) { struct bound_minimal_symbol msymbol = ada_lookup_simple_minsym (standard_exc[i]); @@ -13253,7 +13253,7 @@ ada_add_standard_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) gets pushed. */ static void -ada_add_exceptions_from_frame (regex_t *preg, struct frame_info *frame, +ada_add_exceptions_from_frame (gdb_regex *preg, struct frame_info *frame, VEC(ada_exc_info) **exceptions) { const struct block *block = get_frame_block (frame, 0); @@ -13290,10 +13290,10 @@ ada_add_exceptions_from_frame (regex_t *preg, struct frame_info *frame, /* Return true if NAME matches PREG or if PREG is NULL. */ static bool -name_matches_regex (const char *name, regex_t *preg) +name_matches_regex (const char *name, gdb_regex *preg) { return (preg == NULL - || regexec (preg, ada_decode (name), 0, NULL, 0) == 0); + || preg->exec (ada_decode (name), 0, NULL, 0) == 0); } /* Add all exceptions defined globally whose name name match @@ -13316,7 +13316,7 @@ name_matches_regex (const char *name, regex_t *preg) gets pushed. */ static void -ada_add_global_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) +ada_add_global_exceptions (gdb_regex *preg, VEC(ada_exc_info) **exceptions) { struct objfile *objfile; struct compunit_symtab *s; @@ -13364,7 +13364,7 @@ ada_add_global_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) do not match. Otherwise, all exceptions are listed. */ static VEC(ada_exc_info) * -ada_exceptions_list_1 (regex_t *preg) +ada_exceptions_list_1 (gdb_regex *preg) { VEC(ada_exc_info) *result = NULL; struct cleanup *old_chain @@ -13417,19 +13417,11 @@ ada_exceptions_list_1 (regex_t *preg) VEC(ada_exc_info) * ada_exceptions_list (const char *regexp) { - VEC(ada_exc_info) *result = NULL; - struct cleanup *old_chain = NULL; - regex_t reg; - - if (regexp != NULL) - old_chain = compile_rx_or_error (®, regexp, - _("invalid regular expression")); + if (regexp == NULL) + return ada_exceptions_list_1 (NULL); - result = ada_exceptions_list_1 (regexp != NULL ? ® : NULL); - - if (old_chain != NULL) - do_cleanups (old_chain); - return result; + gdb_regex reg (regexp, REG_NOSUB, _("invalid regular expression")); + return ada_exceptions_list_1 (®); } /* Implement the "info exceptions" command. */ diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c index 7731c5e..49d5180 100644 --- a/gdb/break-catch-throw.c +++ b/gdb/break-catch-throw.c @@ -87,10 +87,10 @@ struct exception_catchpoint : public breakpoint char *exception_rx; - /* If non-NULL, an xmalloc'd, compiled regular expression which is - used to determine which exceptions to stop on. */ + /* If non-NULL, a compiled regular expression which is used to + determine which exceptions to stop on. */ - regex_t *pattern; + std::unique_ptr<gdb_regex> pattern; }; \f @@ -145,8 +145,6 @@ classify_exception_breakpoint (struct breakpoint *b) exception_catchpoint::~exception_catchpoint () { xfree (this->exception_rx); - if (this->pattern != NULL) - regfree (this->pattern); } /* Implement the 'check_status' method. */ @@ -185,7 +183,7 @@ check_status_exception_catchpoint (struct bpstats *bs) if (!type_name.empty ()) { - if (regexec (self->pattern, type_name.c_str (), 0, NULL, 0) != 0) + if (self->pattern->exec (type_name.c_str (), 0, NULL, 0) != 0) bs->stop = 0; } } @@ -377,15 +375,12 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx, const char *cond_string, enum exception_event_kind ex_event, int from_tty) { - regex_t *pattern = NULL; + std::unique_ptr<gdb_regex> pattern; if (except_rx != NULL) { - pattern = XNEW (regex_t); - make_cleanup (xfree, pattern); - - compile_rx_or_error (pattern, except_rx, - _("invalid type-matching regexp")); + pattern.reset (new gdb_regex (except_rx, REG_NOSUB, + _("invalid type-matching regexp"))); } std::unique_ptr<exception_catchpoint> cp (new exception_catchpoint ()); @@ -397,7 +392,7 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx, cp->type = bp_breakpoint; cp->kind = ex_event; cp->exception_rx = except_rx; - cp->pattern = pattern; + cp->pattern = std::move (pattern); re_set_exception_catchpoint (cp.get ()); diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 0dc9841..b78cf2b 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -8282,13 +8282,11 @@ struct solib_catchpoint : public breakpoint /* Regular expression to match, if any. COMPILED is only valid when REGEX is non-NULL. */ char *regex; - regex_t compiled; + std::unique_ptr<gdb_regex> compiled; }; solib_catchpoint::~solib_catchpoint () { - if (this->regex) - regfree (&this->compiled); xfree (this->regex); } @@ -8356,7 +8354,7 @@ check_status_catch_solib (struct bpstats *bs) ++ix) { if (!self->regex - || regexec (&self->compiled, iter->so_name, 0, NULL, 0) == 0) + || self->compiled->exec (iter->so_name, 0, NULL, 0) == 0) return; } } @@ -8370,7 +8368,7 @@ check_status_catch_solib (struct bpstats *bs) ++ix) { if (!self->regex - || regexec (&self->compiled, iter, 0, NULL, 0) == 0) + || self->compiled->exec (iter, 0, NULL, 0) == 0) return; } } @@ -8486,16 +8484,7 @@ add_solib_catchpoint (const char *arg, int is_load, int is_temp, int enabled) if (*arg != '\0') { - int errcode; - - errcode = regcomp (&c->compiled, arg, REG_NOSUB); - if (errcode != 0) - { - char *err = get_regcomp_error (errcode, &c->compiled); - - make_cleanup (xfree, err); - error (_("Invalid regexp (%s): %s"), err, arg); - } + c->compiled.reset (new gdb_regex (arg, REG_NOSUB, _("Invalid regexp"))); c->regex = xstrdup (arg); } diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 2a5b128..3aedecd 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -1336,28 +1336,13 @@ show_user (char *args, int from_tty) static void apropos_command (char *searchstr, int from_tty) { - regex_t pattern; - int code; - if (searchstr == NULL) error (_("REGEXP string is empty")); - code = regcomp (&pattern, searchstr, REG_ICASE); - if (code == 0) - { - struct cleanup *cleanups; + gdb_regex pattern (searchstr, REG_ICASE, + _("Error in regular expression")); - cleanups = make_regfree_cleanup (&pattern); - apropos_cmd (gdb_stdout, cmdlist, &pattern, ""); - do_cleanups (cleanups); - } - else - { - char *err = get_regcomp_error (code, &pattern); - - make_cleanup (xfree, err); - error (_("Error in regular expression: %s"), err); - } + apropos_cmd (gdb_stdout, cmdlist, &pattern, ""); } /* Subroutine of alias_command to simplify it. diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index d386d02..87ac0d5 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -917,7 +917,7 @@ add_com_suppress_notification (const char *name, enum command_class theclass, void apropos_cmd (struct ui_file *stream, struct cmd_list_element *commandlist, - struct re_pattern_buffer *regex, const char *prefix) + gdb_regex *regex, const char *prefix) { struct cmd_list_element *c; int returnvalue; @@ -928,9 +928,10 @@ apropos_cmd (struct ui_file *stream, returnvalue = -1; /* Needed to avoid double printing. */ if (c->name != NULL) { + size_t name_len = strlen (c->name); + /* Try to match against the name. */ - returnvalue = re_search (regex, c->name, strlen(c->name), - 0, strlen (c->name), NULL); + returnvalue = regex->search (c->name, name_len, 0, name_len, NULL); if (returnvalue >= 0) { print_help_for_command (c, prefix, @@ -939,8 +940,10 @@ apropos_cmd (struct ui_file *stream, } if (c->doc != NULL && returnvalue < 0) { + size_t doc_len = strlen (c->doc); + /* Try to match against documentation. */ - if (re_search(regex,c->doc,strlen(c->doc),0,strlen(c->doc),NULL) >=0) + if (regex->search (c->doc, doc_len, 0, doc_len, NULL) >= 0) { print_help_for_command (c, prefix, 0 /* don't recurse */, stream); diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h index 66159fd..b8f2f7c 100644 --- a/gdb/cli/cli-decode.h +++ b/gdb/cli/cli-decode.h @@ -23,8 +23,7 @@ /* Include the public interfaces. */ #include "command.h" - -struct re_pattern_buffer; +#include "gdb_regex.h" #if 0 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum @@ -234,7 +233,7 @@ extern void help_cmd_list (struct cmd_list_element *, enum command_class, extern void help_cmd (const char *, struct ui_file *); extern void apropos_cmd (struct ui_file *, struct cmd_list_element *, - struct re_pattern_buffer *, const char *); + gdb_regex *, const char *); /* Used to mark commands that don't do anything. If we just leave the function field NULL, the command is interpreted as a help topic, or diff --git a/gdb/gdb_regex.h b/gdb/gdb_regex.h index 0be26ef..2b0a5a0 100644 --- a/gdb/gdb_regex.h +++ b/gdb/gdb_regex.h @@ -27,10 +27,26 @@ # include <regex.h> #endif -/* From utils.c. */ -struct cleanup *make_regfree_cleanup (regex_t *); -char *get_regcomp_error (int, regex_t *); -struct cleanup *compile_rx_or_error (regex_t *pattern, const char *rx, - const char *message); +/* A compiled regex. */ + +class gdb_regex +{ +public: + /* Compile a regexp and throw an exception on error, including + MESSAGE. REGEX and MESSAGE must not be NULL. */ + gdb_regex (const char *regex, int cflags, + const char *message); + ~gdb_regex (); + + /* Wrapper around ::regexec. */ + int exec (const char *string, size_t nmatch, regmatch_t pmatch[], int eflags); + + /* Wrapper around ::re_search. */ + int search (const char *string, int size, + int startpos, int range, struct re_registers *regs); + +private: + regex_t m_pattern; +}; #endif /* not GDB_REGEX_H */ diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c index 016aadf..f424947 100644 --- a/gdb/linux-tdep.c +++ b/gdb/linux-tdep.c @@ -38,6 +38,7 @@ #include "gdbcmd.h" #include "gdb_regex.h" #include "common/enum-flags.h" +#include "common/gdb_optional.h" #include <ctype.h> @@ -493,6 +494,47 @@ decode_vmflags (char *p, struct smaps_vmflags *v) } } +struct mapping_regexes +{ + mapping_regexes () + : dev_zero + ("^/dev/zero\\( (deleted)\\)\\?$", REG_NOSUB, + _("Could not compile regex to match /dev/zero filename")), + shmem_file + ("^/\\?SYSV[0-9a-fA-F]\\{8\\}\\( (deleted)\\)\\?$", REG_NOSUB, + _("Could not compile regex to match shmem filenames")), + file_deleted + (" (deleted)$", REG_NOSUB, + _("Could not compile regex to match '<file> (deleted)'")) + {} + + /* Matches "/dev/zero" filenames (with or without the "(deleted)" + string in the end). We know for sure, based on the Linux kernel + code, that memory mappings whose associated filename is + "/dev/zero" are guaranteed to be MAP_ANONYMOUS. */ + gdb_regex dev_zero; + + /* Matches "/SYSV%08x" filenames (with or without the "(deleted)" + string in the end). These filenames refer to shared memory + (shmem), and memory mappings associated with them are + MAP_ANONYMOUS as well. */ + gdb_regex shmem_file; + + /* A heuristic we use to try to mimic the Linux kernel's 'n_link == + 0' code, which is responsible to decide if it is dealing with a + 'MAP_SHARED | MAP_ANONYMOUS' mapping. In other words, if + FILE_DELETED matches, it does not necessarily mean that we are + dealing with an anonymous shared mapping. However, there is no + easy way to detect this currently, so this is the best + approximation we have. + + As a result, GDB will dump readonly pages of deleted executables + when using the default value of coredump_filter (0x33), while the + Linux kernel will not dump those pages. But we can live with + that. */ + gdb_regex file_deleted; +}; + /* Return 1 if the memory mapping is anonymous, 0 otherwise. FILENAME is the name of the file present in the first line of the @@ -506,52 +548,13 @@ decode_vmflags (char *p, struct smaps_vmflags *v) static int mapping_is_anonymous_p (const char *filename) { - static regex_t dev_zero_regex, shmem_file_regex, file_deleted_regex; + /* Like gdb::optional, but tristate. */ static int init_regex_p = 0; + static gdb::optional<mapping_regexes> regexes; if (!init_regex_p) { - struct cleanup *c = make_cleanup (null_cleanup, NULL); - - /* Let's be pessimistic and assume there will be an error while - compiling the regex'es. */ - init_regex_p = -1; - - /* DEV_ZERO_REGEX matches "/dev/zero" filenames (with or - without the "(deleted)" string in the end). We know for - sure, based on the Linux kernel code, that memory mappings - whose associated filename is "/dev/zero" are guaranteed to be - MAP_ANONYMOUS. */ - compile_rx_or_error (&dev_zero_regex, "^/dev/zero\\( (deleted)\\)\\?$", - _("Could not compile regex to match /dev/zero " - "filename")); - /* SHMEM_FILE_REGEX matches "/SYSV%08x" filenames (with or - without the "(deleted)" string in the end). These filenames - refer to shared memory (shmem), and memory mappings - associated with them are MAP_ANONYMOUS as well. */ - compile_rx_or_error (&shmem_file_regex, - "^/\\?SYSV[0-9a-fA-F]\\{8\\}\\( (deleted)\\)\\?$", - _("Could not compile regex to match shmem " - "filenames")); - /* FILE_DELETED_REGEX is a heuristic we use to try to mimic the - Linux kernel's 'n_link == 0' code, which is responsible to - decide if it is dealing with a 'MAP_SHARED | MAP_ANONYMOUS' - mapping. In other words, if FILE_DELETED_REGEX matches, it - does not necessarily mean that we are dealing with an - anonymous shared mapping. However, there is no easy way to - detect this currently, so this is the best approximation we - have. - - As a result, GDB will dump readonly pages of deleted - executables when using the default value of coredump_filter - (0x33), while the Linux kernel will not dump those pages. - But we can live with that. */ - compile_rx_or_error (&file_deleted_regex, " (deleted)$", - _("Could not compile regex to match " - "'<file> (deleted)'")); - /* We will never release these regexes, so just discard the - cleanups. */ - discard_cleanups (c); + regexes.emplace (); /* If we reached this point, then everything succeeded. */ init_regex_p = 1; @@ -573,9 +576,9 @@ mapping_is_anonymous_p (const char *filename) } if (*filename == '\0' - || regexec (&dev_zero_regex, filename, 0, NULL, 0) == 0 - || regexec (&shmem_file_regex, filename, 0, NULL, 0) == 0 - || regexec (&file_deleted_regex, filename, 0, NULL, 0) == 0) + || regexes->dev_zero.exec (filename, 0, NULL, 0) == 0 + || regexes->shmem_file.exec (filename, 0, NULL, 0) == 0 + || regexes->file_deleted.exec (filename, 0, NULL, 0) == 0) return 1; return 0; diff --git a/gdb/probe.c b/gdb/probe.c index e65e031..c8b92f3 100644 --- a/gdb/probe.c +++ b/gdb/probe.c @@ -36,6 +36,7 @@ #include "location.h" #include <ctype.h> #include <algorithm> +#include "common/gdb_optional.h" typedef struct bound_probe bound_probe_s; DEF_VEC_O (bound_probe_s); @@ -288,18 +289,17 @@ collect_probes (char *objname, char *provider, char *probe_name, { struct objfile *objfile; VEC (bound_probe_s) *result = NULL; - struct cleanup *cleanup, *cleanup_temps; - regex_t obj_pat, prov_pat, probe_pat; + struct cleanup *cleanup; + gdb::optional<gdb_regex> obj_pat, prov_pat, probe_pat; cleanup = make_cleanup (VEC_cleanup (bound_probe_s), &result); - cleanup_temps = make_cleanup (null_cleanup, NULL); if (provider != NULL) - compile_rx_or_error (&prov_pat, provider, _("Invalid provider regexp")); + prov_pat.emplace (provider, REG_NOSUB, _("Invalid provider regexp")); if (probe_name != NULL) - compile_rx_or_error (&probe_pat, probe_name, _("Invalid probe regexp")); + probe_pat.emplace (probe_name, REG_NOSUB, _("Invalid probe regexp")); if (objname != NULL) - compile_rx_or_error (&obj_pat, objname, _("Invalid object file regexp")); + obj_pat.emplace (objname, REG_NOSUB, _("Invalid object file regexp")); ALL_OBJFILES (objfile) { @@ -312,7 +312,7 @@ collect_probes (char *objname, char *provider, char *probe_name, if (objname) { - if (regexec (&obj_pat, objfile_name (objfile), 0, NULL, 0) != 0) + if (obj_pat->exec (objfile_name (objfile), 0, NULL, 0) != 0) continue; } @@ -326,11 +326,11 @@ collect_probes (char *objname, char *provider, char *probe_name, continue; if (provider - && regexec (&prov_pat, probe->provider, 0, NULL, 0) != 0) + && prov_pat->exec (probe->provider, 0, NULL, 0) != 0) continue; if (probe_name - && regexec (&probe_pat, probe->name, 0, NULL, 0) != 0) + && probe_pat->exec (probe->name, 0, NULL, 0) != 0) continue; bound.objfile = objfile; @@ -339,7 +339,6 @@ collect_probes (char *objname, char *provider, char *probe_name, } } - do_cleanups (cleanup_temps); discard_cleanups (cleanup); return result; } diff --git a/gdb/skip.c b/gdb/skip.c index 4bd8a9e..65583d8 100644 --- a/gdb/skip.c +++ b/gdb/skip.c @@ -34,6 +34,7 @@ #include "filenames.h" #include "fnmatch.h" #include "gdb_regex.h" +#include "common/gdb_optional.h" struct skiplist_entry { @@ -57,10 +58,7 @@ struct skiplist_entry char *function; /* If this is a function regexp, the compiled form. */ - regex_t compiled_function_regexp; - - /* Non-zero if the function regexp has been compiled. */ - int compiled_function_regexp_is_valid; + gdb::optional<gdb_regex> compiled_function_regexp; int enabled; @@ -112,8 +110,6 @@ free_skiplist_entry (struct skiplist_entry *e) { xfree (e->file); xfree (e->function); - if (e->function_is_regexp && e->compiled_function_regexp_is_valid) - regfree (&e->compiled_function_regexp); xfree (e); } @@ -202,7 +198,6 @@ skip_function_command (char *arg, int from_tty) static void compile_skip_regexp (struct skiplist_entry *e, const char *message) { - int code; int flags = REG_NOSUB; #ifdef REG_EXTENDED @@ -210,16 +205,7 @@ compile_skip_regexp (struct skiplist_entry *e, const char *message) #endif gdb_assert (e->function_is_regexp && e->function != NULL); - - code = regcomp (&e->compiled_function_regexp, e->function, flags); - if (code != 0) - { - char *err = get_regcomp_error (code, &e->compiled_function_regexp); - - make_cleanup (xfree, err); - error (_("%s: %s"), message, err); - } - e->compiled_function_regexp_is_valid = 1; + e->compiled_function_regexp.emplace (e->function, flags, message); } /* Process "skip ..." that does not match "skip file" or "skip function". */ @@ -601,8 +587,8 @@ static int skip_rfunction_p (struct skiplist_entry *e, const char *function_name) { gdb_assert (e->function != NULL && e->function_is_regexp - && e->compiled_function_regexp_is_valid); - return (regexec (&e->compiled_function_regexp, function_name, 0, NULL, 0) + && e->compiled_function_regexp); + return (e->compiled_function_regexp->exec (function_name, 0, NULL, 0) == 0); } diff --git a/gdb/symtab.c b/gdb/symtab.c index 22d81fa..1c1eb6c 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -62,6 +62,7 @@ #include "parser-defs.h" #include "completer.h" #include "progspace-and-thread.h" +#include "common/gdb_optional.h" /* Forward declarations for local functions. */ @@ -4299,9 +4300,7 @@ search_symbols (const char *regexp, enum search_domain kind, struct symbol_search *found; struct symbol_search *tail; int nfound; - /* This is true if PREG contains valid data, false otherwise. */ - bool preg_p; - regex_t preg; + gdb::optional<gdb_regex> preg; /* OLD_CHAIN .. RETVAL_CHAIN is always freed, RETVAL_CHAIN .. current CLEANUP_CHAIN is freed only in the case of an error. */ @@ -4316,7 +4315,6 @@ search_symbols (const char *regexp, enum search_domain kind, ourtype4 = types4[kind]; *matches = NULL; - preg_p = false; if (regexp != NULL) { @@ -4355,18 +4353,9 @@ search_symbols (const char *regexp, enum search_domain kind, } } - errcode = regcomp (&preg, regexp, - REG_NOSUB | (case_sensitivity == case_sensitive_off - ? REG_ICASE : 0)); - if (errcode != 0) - { - char *err = get_regcomp_error (errcode, &preg); - - make_cleanup (xfree, err); - error (_("Invalid regexp (%s): %s"), err, regexp); - } - preg_p = true; - make_regfree_cleanup (&preg); + int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off + ? REG_ICASE : 0); + preg.emplace (regexp, cflags, _("Invalid regexp")); } /* Search through the partial symtabs *first* for all symbols @@ -4379,8 +4368,8 @@ search_symbols (const char *regexp, enum search_domain kind, }, [&] (const char *symname) { - return (!preg_p || regexec (&preg, symname, - 0, NULL, 0) == 0); + return (!preg || preg->exec (symname, + 0, NULL, 0) == 0); }, NULL, kind); @@ -4415,9 +4404,9 @@ search_symbols (const char *regexp, enum search_domain kind, || MSYMBOL_TYPE (msymbol) == ourtype3 || MSYMBOL_TYPE (msymbol) == ourtype4) { - if (!preg_p - || regexec (&preg, MSYMBOL_NATURAL_NAME (msymbol), 0, - NULL, 0) == 0) + if (!preg + || preg->exec (MSYMBOL_NATURAL_NAME (msymbol), 0, + NULL, 0) == 0) { /* Note: An important side-effect of these lookup functions is to expand the symbol table if msymbol is found, for the @@ -4459,9 +4448,9 @@ search_symbols (const char *regexp, enum search_domain kind, files, nfiles, 1)) && file_matches (symtab_to_fullname (real_symtab), files, nfiles, 0))) - && ((!preg_p - || regexec (&preg, SYMBOL_NATURAL_NAME (sym), 0, - NULL, 0) == 0) + && ((!preg + || preg->exec (SYMBOL_NATURAL_NAME (sym), 0, + NULL, 0) == 0) && ((kind == VARIABLES_DOMAIN && SYMBOL_CLASS (sym) != LOC_TYPEDEF && SYMBOL_CLASS (sym) != LOC_UNRESOLVED @@ -4517,9 +4506,8 @@ search_symbols (const char *regexp, enum search_domain kind, || MSYMBOL_TYPE (msymbol) == ourtype3 || MSYMBOL_TYPE (msymbol) == ourtype4) { - if (!preg_p - || regexec (&preg, MSYMBOL_NATURAL_NAME (msymbol), 0, - NULL, 0) == 0) + if (!preg || preg->exec (MSYMBOL_NATURAL_NAME (msymbol), 0, + NULL, 0) == 0) { /* For functions we can do a quick check of whether the symbol might be found via find_pc_symtab. */ diff --git a/gdb/utils.c b/gdb/utils.c index b4332f8..88a1789 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1038,58 +1038,6 @@ make_hex_string (const gdb_byte *data, size_t length) \f -/* A cleanup function that calls regfree. */ - -static void -do_regfree_cleanup (void *r) -{ - regfree ((regex_t *) r); -} - -/* Create a new cleanup that frees the compiled regular expression R. */ - -struct cleanup * -make_regfree_cleanup (regex_t *r) -{ - return make_cleanup (do_regfree_cleanup, r); -} - -/* Return an xmalloc'd error message resulting from a regular - expression compilation failure. */ - -char * -get_regcomp_error (int code, regex_t *rx) -{ - size_t length = regerror (code, rx, NULL, 0); - char *result = (char *) xmalloc (length); - - regerror (code, rx, result, length); - return result; -} - -/* Compile a regexp and throw an exception on error. This returns a - cleanup to free the resulting pattern on success. RX must not be - NULL. */ - -struct cleanup * -compile_rx_or_error (regex_t *pattern, const char *rx, const char *message) -{ - int code; - - gdb_assert (rx != NULL); - - code = regcomp (pattern, rx, REG_NOSUB); - if (code != 0) - { - char *err = get_regcomp_error (code, pattern); - - make_cleanup (xfree, err); - error (("%s: %s"), message, err); - } - - return make_regfree_cleanup (pattern); -} - /* A cleanup that simply calls ui_unregister_input_event_handler. */ static void -- 2.5.5 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 2/2] C++-ify break-catch-throw 2017-06-05 12:29 ` Pedro Alves @ 2017-06-05 12:32 ` Pedro Alves 2017-06-05 19:27 ` Tom Tromey 1 sibling, 0 replies; 17+ messages in thread From: Pedro Alves @ 2017-06-05 12:32 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 06/05/2017 01:29 PM, Pedro Alves wrote: > On 06/05/2017 11:36 AM, Pedro Alves wrote: >> On 06/05/2017 11:33 AM, Pedro Alves wrote: >>> class gdb_regex >>> { >>> public: >>> // replaces old compile_rx_or_error >>> gdb_regex (const char *rx, const char *message) >>> { >> >> Maybe call it "compiled_regex" instead, since you wouldn't >> be able to end up with a not-compiled-yet regex with this. >> > > So something like this. Builds and gdb starts, but I have > not regtested it. > > In a couple places, this either forces moving the regex object > to the heap, or to wrap it in gdb::optional. In the cases > where we already have to keep the regex string around, > it ends up being not maximally efficient memory-wise, but I don't > think it really matters. We're considering std::string for > those same strings, which grows the structs more than that, anyway > (for size/capacity). > > For linux-tdep.cmapping_is_anonymous_p, I was first considering > std::aligned_storage, but switched back to gdb::optional before posting, > because std::aligned_storage complicates the code a little bit (I mean, forces > readers to learn about std::aligned_storage while here the redundant > "has value" variable that gdb::optional has internally doesn't really > matter. We could move the mapping_regexes struct to the heap too, since is > a one-shot initialization. (I see now that I left the "tristate" comment behind.) > > I didn't use "compiled_regex" for struct name, simply because "gdb_regex.h" > already existed. But I'd still consider renaming it. > > WDYT? > Forgot to "git add" the new file... Trying again. From 529e26f36ae5ed1f7ee7d375d54843d0b435cf83 Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Mon, 5 Jun 2017 12:48:46 +0100 Subject: [PATCH] gdb_regex --- gdb/Makefile.in | 2 ++ gdb/ada-lang.c | 30 ++++++---------- gdb/break-catch-throw.c | 21 +++++------ gdb/breakpoint.c | 19 +++------- gdb/cli/cli-cmds.c | 21 ++--------- gdb/cli/cli-decode.c | 11 +++--- gdb/cli/cli-decode.h | 5 ++- gdb/gdb_regex.c | 59 +++++++++++++++++++++++++++++++ gdb/gdb_regex.h | 26 +++++++++++--- gdb/linux-tdep.c | 93 +++++++++++++++++++++++++------------------------ gdb/probe.c | 19 +++++----- gdb/skip.c | 24 +++---------- gdb/symtab.c | 42 ++++++++-------------- gdb/utils.c | 52 --------------------------- 14 files changed, 194 insertions(+), 230 deletions(-) create mode 100644 gdb/gdb_regex.c diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 8be73ba..2156438 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -1104,6 +1104,7 @@ SFILES = \ gdb_bfd.c \ gdb-dlfcn.c \ gdb_obstack.c \ + gdb_regex.c \ gdb_usleep.c \ gdbarch.c \ gdbarch-selftests.c \ @@ -1717,6 +1718,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \ gdb_bfd.o \ gdb-dlfcn.o \ gdb_obstack.o \ + gdb_regex.o \ gdb_usleep.o \ gdb_vecs.o \ gdbarch.o \ diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index f90907a..e338cd1 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -13219,14 +13219,14 @@ sort_remove_dups_ada_exceptions_list (VEC(ada_exc_info) **exceptions, gets pushed. */ static void -ada_add_standard_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) +ada_add_standard_exceptions (gdb_regex *preg, VEC(ada_exc_info) **exceptions) { int i; for (i = 0; i < ARRAY_SIZE (standard_exc); i++) { if (preg == NULL - || regexec (preg, standard_exc[i], 0, NULL, 0) == 0) + || preg->exec (standard_exc[i], 0, NULL, 0) == 0) { struct bound_minimal_symbol msymbol = ada_lookup_simple_minsym (standard_exc[i]); @@ -13253,7 +13253,7 @@ ada_add_standard_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) gets pushed. */ static void -ada_add_exceptions_from_frame (regex_t *preg, struct frame_info *frame, +ada_add_exceptions_from_frame (gdb_regex *preg, struct frame_info *frame, VEC(ada_exc_info) **exceptions) { const struct block *block = get_frame_block (frame, 0); @@ -13290,10 +13290,10 @@ ada_add_exceptions_from_frame (regex_t *preg, struct frame_info *frame, /* Return true if NAME matches PREG or if PREG is NULL. */ static bool -name_matches_regex (const char *name, regex_t *preg) +name_matches_regex (const char *name, gdb_regex *preg) { return (preg == NULL - || regexec (preg, ada_decode (name), 0, NULL, 0) == 0); + || preg->exec (ada_decode (name), 0, NULL, 0) == 0); } /* Add all exceptions defined globally whose name name match @@ -13316,7 +13316,7 @@ name_matches_regex (const char *name, regex_t *preg) gets pushed. */ static void -ada_add_global_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) +ada_add_global_exceptions (gdb_regex *preg, VEC(ada_exc_info) **exceptions) { struct objfile *objfile; struct compunit_symtab *s; @@ -13364,7 +13364,7 @@ ada_add_global_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) do not match. Otherwise, all exceptions are listed. */ static VEC(ada_exc_info) * -ada_exceptions_list_1 (regex_t *preg) +ada_exceptions_list_1 (gdb_regex *preg) { VEC(ada_exc_info) *result = NULL; struct cleanup *old_chain @@ -13417,19 +13417,11 @@ ada_exceptions_list_1 (regex_t *preg) VEC(ada_exc_info) * ada_exceptions_list (const char *regexp) { - VEC(ada_exc_info) *result = NULL; - struct cleanup *old_chain = NULL; - regex_t reg; - - if (regexp != NULL) - old_chain = compile_rx_or_error (®, regexp, - _("invalid regular expression")); + if (regexp == NULL) + return ada_exceptions_list_1 (NULL); - result = ada_exceptions_list_1 (regexp != NULL ? ® : NULL); - - if (old_chain != NULL) - do_cleanups (old_chain); - return result; + gdb_regex reg (regexp, REG_NOSUB, _("invalid regular expression")); + return ada_exceptions_list_1 (®); } /* Implement the "info exceptions" command. */ diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c index 7731c5e..49d5180 100644 --- a/gdb/break-catch-throw.c +++ b/gdb/break-catch-throw.c @@ -87,10 +87,10 @@ struct exception_catchpoint : public breakpoint char *exception_rx; - /* If non-NULL, an xmalloc'd, compiled regular expression which is - used to determine which exceptions to stop on. */ + /* If non-NULL, a compiled regular expression which is used to + determine which exceptions to stop on. */ - regex_t *pattern; + std::unique_ptr<gdb_regex> pattern; }; \f @@ -145,8 +145,6 @@ classify_exception_breakpoint (struct breakpoint *b) exception_catchpoint::~exception_catchpoint () { xfree (this->exception_rx); - if (this->pattern != NULL) - regfree (this->pattern); } /* Implement the 'check_status' method. */ @@ -185,7 +183,7 @@ check_status_exception_catchpoint (struct bpstats *bs) if (!type_name.empty ()) { - if (regexec (self->pattern, type_name.c_str (), 0, NULL, 0) != 0) + if (self->pattern->exec (type_name.c_str (), 0, NULL, 0) != 0) bs->stop = 0; } } @@ -377,15 +375,12 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx, const char *cond_string, enum exception_event_kind ex_event, int from_tty) { - regex_t *pattern = NULL; + std::unique_ptr<gdb_regex> pattern; if (except_rx != NULL) { - pattern = XNEW (regex_t); - make_cleanup (xfree, pattern); - - compile_rx_or_error (pattern, except_rx, - _("invalid type-matching regexp")); + pattern.reset (new gdb_regex (except_rx, REG_NOSUB, + _("invalid type-matching regexp"))); } std::unique_ptr<exception_catchpoint> cp (new exception_catchpoint ()); @@ -397,7 +392,7 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx, cp->type = bp_breakpoint; cp->kind = ex_event; cp->exception_rx = except_rx; - cp->pattern = pattern; + cp->pattern = std::move (pattern); re_set_exception_catchpoint (cp.get ()); diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 0dc9841..b78cf2b 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -8282,13 +8282,11 @@ struct solib_catchpoint : public breakpoint /* Regular expression to match, if any. COMPILED is only valid when REGEX is non-NULL. */ char *regex; - regex_t compiled; + std::unique_ptr<gdb_regex> compiled; }; solib_catchpoint::~solib_catchpoint () { - if (this->regex) - regfree (&this->compiled); xfree (this->regex); } @@ -8356,7 +8354,7 @@ check_status_catch_solib (struct bpstats *bs) ++ix) { if (!self->regex - || regexec (&self->compiled, iter->so_name, 0, NULL, 0) == 0) + || self->compiled->exec (iter->so_name, 0, NULL, 0) == 0) return; } } @@ -8370,7 +8368,7 @@ check_status_catch_solib (struct bpstats *bs) ++ix) { if (!self->regex - || regexec (&self->compiled, iter, 0, NULL, 0) == 0) + || self->compiled->exec (iter, 0, NULL, 0) == 0) return; } } @@ -8486,16 +8484,7 @@ add_solib_catchpoint (const char *arg, int is_load, int is_temp, int enabled) if (*arg != '\0') { - int errcode; - - errcode = regcomp (&c->compiled, arg, REG_NOSUB); - if (errcode != 0) - { - char *err = get_regcomp_error (errcode, &c->compiled); - - make_cleanup (xfree, err); - error (_("Invalid regexp (%s): %s"), err, arg); - } + c->compiled.reset (new gdb_regex (arg, REG_NOSUB, _("Invalid regexp"))); c->regex = xstrdup (arg); } diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 2a5b128..3aedecd 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -1336,28 +1336,13 @@ show_user (char *args, int from_tty) static void apropos_command (char *searchstr, int from_tty) { - regex_t pattern; - int code; - if (searchstr == NULL) error (_("REGEXP string is empty")); - code = regcomp (&pattern, searchstr, REG_ICASE); - if (code == 0) - { - struct cleanup *cleanups; + gdb_regex pattern (searchstr, REG_ICASE, + _("Error in regular expression")); - cleanups = make_regfree_cleanup (&pattern); - apropos_cmd (gdb_stdout, cmdlist, &pattern, ""); - do_cleanups (cleanups); - } - else - { - char *err = get_regcomp_error (code, &pattern); - - make_cleanup (xfree, err); - error (_("Error in regular expression: %s"), err); - } + apropos_cmd (gdb_stdout, cmdlist, &pattern, ""); } /* Subroutine of alias_command to simplify it. diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index d386d02..87ac0d5 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -917,7 +917,7 @@ add_com_suppress_notification (const char *name, enum command_class theclass, void apropos_cmd (struct ui_file *stream, struct cmd_list_element *commandlist, - struct re_pattern_buffer *regex, const char *prefix) + gdb_regex *regex, const char *prefix) { struct cmd_list_element *c; int returnvalue; @@ -928,9 +928,10 @@ apropos_cmd (struct ui_file *stream, returnvalue = -1; /* Needed to avoid double printing. */ if (c->name != NULL) { + size_t name_len = strlen (c->name); + /* Try to match against the name. */ - returnvalue = re_search (regex, c->name, strlen(c->name), - 0, strlen (c->name), NULL); + returnvalue = regex->search (c->name, name_len, 0, name_len, NULL); if (returnvalue >= 0) { print_help_for_command (c, prefix, @@ -939,8 +940,10 @@ apropos_cmd (struct ui_file *stream, } if (c->doc != NULL && returnvalue < 0) { + size_t doc_len = strlen (c->doc); + /* Try to match against documentation. */ - if (re_search(regex,c->doc,strlen(c->doc),0,strlen(c->doc),NULL) >=0) + if (regex->search (c->doc, doc_len, 0, doc_len, NULL) >= 0) { print_help_for_command (c, prefix, 0 /* don't recurse */, stream); diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h index 66159fd..b8f2f7c 100644 --- a/gdb/cli/cli-decode.h +++ b/gdb/cli/cli-decode.h @@ -23,8 +23,7 @@ /* Include the public interfaces. */ #include "command.h" - -struct re_pattern_buffer; +#include "gdb_regex.h" #if 0 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum @@ -234,7 +233,7 @@ extern void help_cmd_list (struct cmd_list_element *, enum command_class, extern void help_cmd (const char *, struct ui_file *); extern void apropos_cmd (struct ui_file *, struct cmd_list_element *, - struct re_pattern_buffer *, const char *); + gdb_regex *, const char *); /* Used to mark commands that don't do anything. If we just leave the function field NULL, the command is interpreted as a help topic, or diff --git a/gdb/gdb_regex.c b/gdb/gdb_regex.c new file mode 100644 index 0000000..06c4666 --- /dev/null +++ b/gdb/gdb_regex.c @@ -0,0 +1,59 @@ +/* Portable <regex.h>. + + Copyright (C) 1986-2017 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#include "defs.h" +#include "gdb_regex.h" + +/* See gdb_regex.h. */ + +gdb_regex::gdb_regex (const char *regex, int cflags, + const char *message) +{ + gdb_assert (regex != NULL); + + int code = regcomp (&m_pattern, regex, cflags); + if (code != 0) + { + size_t length = regerror (code, &m_pattern, NULL, 0); + std::unique_ptr<char[]> err (new char[length]); + + regerror (code, &m_pattern, err.get (), length); + error (("%s: %s"), message, err.get ()); + } +} + +gdb_regex::~gdb_regex () +{ + regfree (&m_pattern); +} + +int +gdb_regex::exec (const char *string, size_t nmatch, + regmatch_t pmatch[], int eflags) +{ + return regexec (&m_pattern, string, nmatch, pmatch, eflags); +} + +int +gdb_regex::search (const char *string, + int length, int start, int range, + struct re_registers *regs) +{ + return re_search (&m_pattern, string, length, start, range, regs); +} diff --git a/gdb/gdb_regex.h b/gdb/gdb_regex.h index 0be26ef..2b0a5a0 100644 --- a/gdb/gdb_regex.h +++ b/gdb/gdb_regex.h @@ -27,10 +27,26 @@ # include <regex.h> #endif -/* From utils.c. */ -struct cleanup *make_regfree_cleanup (regex_t *); -char *get_regcomp_error (int, regex_t *); -struct cleanup *compile_rx_or_error (regex_t *pattern, const char *rx, - const char *message); +/* A compiled regex. */ + +class gdb_regex +{ +public: + /* Compile a regexp and throw an exception on error, including + MESSAGE. REGEX and MESSAGE must not be NULL. */ + gdb_regex (const char *regex, int cflags, + const char *message); + ~gdb_regex (); + + /* Wrapper around ::regexec. */ + int exec (const char *string, size_t nmatch, regmatch_t pmatch[], int eflags); + + /* Wrapper around ::re_search. */ + int search (const char *string, int size, + int startpos, int range, struct re_registers *regs); + +private: + regex_t m_pattern; +}; #endif /* not GDB_REGEX_H */ diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c index 016aadf..f424947 100644 --- a/gdb/linux-tdep.c +++ b/gdb/linux-tdep.c @@ -38,6 +38,7 @@ #include "gdbcmd.h" #include "gdb_regex.h" #include "common/enum-flags.h" +#include "common/gdb_optional.h" #include <ctype.h> @@ -493,6 +494,47 @@ decode_vmflags (char *p, struct smaps_vmflags *v) } } +struct mapping_regexes +{ + mapping_regexes () + : dev_zero + ("^/dev/zero\\( (deleted)\\)\\?$", REG_NOSUB, + _("Could not compile regex to match /dev/zero filename")), + shmem_file + ("^/\\?SYSV[0-9a-fA-F]\\{8\\}\\( (deleted)\\)\\?$", REG_NOSUB, + _("Could not compile regex to match shmem filenames")), + file_deleted + (" (deleted)$", REG_NOSUB, + _("Could not compile regex to match '<file> (deleted)'")) + {} + + /* Matches "/dev/zero" filenames (with or without the "(deleted)" + string in the end). We know for sure, based on the Linux kernel + code, that memory mappings whose associated filename is + "/dev/zero" are guaranteed to be MAP_ANONYMOUS. */ + gdb_regex dev_zero; + + /* Matches "/SYSV%08x" filenames (with or without the "(deleted)" + string in the end). These filenames refer to shared memory + (shmem), and memory mappings associated with them are + MAP_ANONYMOUS as well. */ + gdb_regex shmem_file; + + /* A heuristic we use to try to mimic the Linux kernel's 'n_link == + 0' code, which is responsible to decide if it is dealing with a + 'MAP_SHARED | MAP_ANONYMOUS' mapping. In other words, if + FILE_DELETED matches, it does not necessarily mean that we are + dealing with an anonymous shared mapping. However, there is no + easy way to detect this currently, so this is the best + approximation we have. + + As a result, GDB will dump readonly pages of deleted executables + when using the default value of coredump_filter (0x33), while the + Linux kernel will not dump those pages. But we can live with + that. */ + gdb_regex file_deleted; +}; + /* Return 1 if the memory mapping is anonymous, 0 otherwise. FILENAME is the name of the file present in the first line of the @@ -506,52 +548,13 @@ decode_vmflags (char *p, struct smaps_vmflags *v) static int mapping_is_anonymous_p (const char *filename) { - static regex_t dev_zero_regex, shmem_file_regex, file_deleted_regex; + /* Like gdb::optional, but tristate. */ static int init_regex_p = 0; + static gdb::optional<mapping_regexes> regexes; if (!init_regex_p) { - struct cleanup *c = make_cleanup (null_cleanup, NULL); - - /* Let's be pessimistic and assume there will be an error while - compiling the regex'es. */ - init_regex_p = -1; - - /* DEV_ZERO_REGEX matches "/dev/zero" filenames (with or - without the "(deleted)" string in the end). We know for - sure, based on the Linux kernel code, that memory mappings - whose associated filename is "/dev/zero" are guaranteed to be - MAP_ANONYMOUS. */ - compile_rx_or_error (&dev_zero_regex, "^/dev/zero\\( (deleted)\\)\\?$", - _("Could not compile regex to match /dev/zero " - "filename")); - /* SHMEM_FILE_REGEX matches "/SYSV%08x" filenames (with or - without the "(deleted)" string in the end). These filenames - refer to shared memory (shmem), and memory mappings - associated with them are MAP_ANONYMOUS as well. */ - compile_rx_or_error (&shmem_file_regex, - "^/\\?SYSV[0-9a-fA-F]\\{8\\}\\( (deleted)\\)\\?$", - _("Could not compile regex to match shmem " - "filenames")); - /* FILE_DELETED_REGEX is a heuristic we use to try to mimic the - Linux kernel's 'n_link == 0' code, which is responsible to - decide if it is dealing with a 'MAP_SHARED | MAP_ANONYMOUS' - mapping. In other words, if FILE_DELETED_REGEX matches, it - does not necessarily mean that we are dealing with an - anonymous shared mapping. However, there is no easy way to - detect this currently, so this is the best approximation we - have. - - As a result, GDB will dump readonly pages of deleted - executables when using the default value of coredump_filter - (0x33), while the Linux kernel will not dump those pages. - But we can live with that. */ - compile_rx_or_error (&file_deleted_regex, " (deleted)$", - _("Could not compile regex to match " - "'<file> (deleted)'")); - /* We will never release these regexes, so just discard the - cleanups. */ - discard_cleanups (c); + regexes.emplace (); /* If we reached this point, then everything succeeded. */ init_regex_p = 1; @@ -573,9 +576,9 @@ mapping_is_anonymous_p (const char *filename) } if (*filename == '\0' - || regexec (&dev_zero_regex, filename, 0, NULL, 0) == 0 - || regexec (&shmem_file_regex, filename, 0, NULL, 0) == 0 - || regexec (&file_deleted_regex, filename, 0, NULL, 0) == 0) + || regexes->dev_zero.exec (filename, 0, NULL, 0) == 0 + || regexes->shmem_file.exec (filename, 0, NULL, 0) == 0 + || regexes->file_deleted.exec (filename, 0, NULL, 0) == 0) return 1; return 0; diff --git a/gdb/probe.c b/gdb/probe.c index e65e031..c8b92f3 100644 --- a/gdb/probe.c +++ b/gdb/probe.c @@ -36,6 +36,7 @@ #include "location.h" #include <ctype.h> #include <algorithm> +#include "common/gdb_optional.h" typedef struct bound_probe bound_probe_s; DEF_VEC_O (bound_probe_s); @@ -288,18 +289,17 @@ collect_probes (char *objname, char *provider, char *probe_name, { struct objfile *objfile; VEC (bound_probe_s) *result = NULL; - struct cleanup *cleanup, *cleanup_temps; - regex_t obj_pat, prov_pat, probe_pat; + struct cleanup *cleanup; + gdb::optional<gdb_regex> obj_pat, prov_pat, probe_pat; cleanup = make_cleanup (VEC_cleanup (bound_probe_s), &result); - cleanup_temps = make_cleanup (null_cleanup, NULL); if (provider != NULL) - compile_rx_or_error (&prov_pat, provider, _("Invalid provider regexp")); + prov_pat.emplace (provider, REG_NOSUB, _("Invalid provider regexp")); if (probe_name != NULL) - compile_rx_or_error (&probe_pat, probe_name, _("Invalid probe regexp")); + probe_pat.emplace (probe_name, REG_NOSUB, _("Invalid probe regexp")); if (objname != NULL) - compile_rx_or_error (&obj_pat, objname, _("Invalid object file regexp")); + obj_pat.emplace (objname, REG_NOSUB, _("Invalid object file regexp")); ALL_OBJFILES (objfile) { @@ -312,7 +312,7 @@ collect_probes (char *objname, char *provider, char *probe_name, if (objname) { - if (regexec (&obj_pat, objfile_name (objfile), 0, NULL, 0) != 0) + if (obj_pat->exec (objfile_name (objfile), 0, NULL, 0) != 0) continue; } @@ -326,11 +326,11 @@ collect_probes (char *objname, char *provider, char *probe_name, continue; if (provider - && regexec (&prov_pat, probe->provider, 0, NULL, 0) != 0) + && prov_pat->exec (probe->provider, 0, NULL, 0) != 0) continue; if (probe_name - && regexec (&probe_pat, probe->name, 0, NULL, 0) != 0) + && probe_pat->exec (probe->name, 0, NULL, 0) != 0) continue; bound.objfile = objfile; @@ -339,7 +339,6 @@ collect_probes (char *objname, char *provider, char *probe_name, } } - do_cleanups (cleanup_temps); discard_cleanups (cleanup); return result; } diff --git a/gdb/skip.c b/gdb/skip.c index 4bd8a9e..65583d8 100644 --- a/gdb/skip.c +++ b/gdb/skip.c @@ -34,6 +34,7 @@ #include "filenames.h" #include "fnmatch.h" #include "gdb_regex.h" +#include "common/gdb_optional.h" struct skiplist_entry { @@ -57,10 +58,7 @@ struct skiplist_entry char *function; /* If this is a function regexp, the compiled form. */ - regex_t compiled_function_regexp; - - /* Non-zero if the function regexp has been compiled. */ - int compiled_function_regexp_is_valid; + gdb::optional<gdb_regex> compiled_function_regexp; int enabled; @@ -112,8 +110,6 @@ free_skiplist_entry (struct skiplist_entry *e) { xfree (e->file); xfree (e->function); - if (e->function_is_regexp && e->compiled_function_regexp_is_valid) - regfree (&e->compiled_function_regexp); xfree (e); } @@ -202,7 +198,6 @@ skip_function_command (char *arg, int from_tty) static void compile_skip_regexp (struct skiplist_entry *e, const char *message) { - int code; int flags = REG_NOSUB; #ifdef REG_EXTENDED @@ -210,16 +205,7 @@ compile_skip_regexp (struct skiplist_entry *e, const char *message) #endif gdb_assert (e->function_is_regexp && e->function != NULL); - - code = regcomp (&e->compiled_function_regexp, e->function, flags); - if (code != 0) - { - char *err = get_regcomp_error (code, &e->compiled_function_regexp); - - make_cleanup (xfree, err); - error (_("%s: %s"), message, err); - } - e->compiled_function_regexp_is_valid = 1; + e->compiled_function_regexp.emplace (e->function, flags, message); } /* Process "skip ..." that does not match "skip file" or "skip function". */ @@ -601,8 +587,8 @@ static int skip_rfunction_p (struct skiplist_entry *e, const char *function_name) { gdb_assert (e->function != NULL && e->function_is_regexp - && e->compiled_function_regexp_is_valid); - return (regexec (&e->compiled_function_regexp, function_name, 0, NULL, 0) + && e->compiled_function_regexp); + return (e->compiled_function_regexp->exec (function_name, 0, NULL, 0) == 0); } diff --git a/gdb/symtab.c b/gdb/symtab.c index 22d81fa..1c1eb6c 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -62,6 +62,7 @@ #include "parser-defs.h" #include "completer.h" #include "progspace-and-thread.h" +#include "common/gdb_optional.h" /* Forward declarations for local functions. */ @@ -4299,9 +4300,7 @@ search_symbols (const char *regexp, enum search_domain kind, struct symbol_search *found; struct symbol_search *tail; int nfound; - /* This is true if PREG contains valid data, false otherwise. */ - bool preg_p; - regex_t preg; + gdb::optional<gdb_regex> preg; /* OLD_CHAIN .. RETVAL_CHAIN is always freed, RETVAL_CHAIN .. current CLEANUP_CHAIN is freed only in the case of an error. */ @@ -4316,7 +4315,6 @@ search_symbols (const char *regexp, enum search_domain kind, ourtype4 = types4[kind]; *matches = NULL; - preg_p = false; if (regexp != NULL) { @@ -4355,18 +4353,9 @@ search_symbols (const char *regexp, enum search_domain kind, } } - errcode = regcomp (&preg, regexp, - REG_NOSUB | (case_sensitivity == case_sensitive_off - ? REG_ICASE : 0)); - if (errcode != 0) - { - char *err = get_regcomp_error (errcode, &preg); - - make_cleanup (xfree, err); - error (_("Invalid regexp (%s): %s"), err, regexp); - } - preg_p = true; - make_regfree_cleanup (&preg); + int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off + ? REG_ICASE : 0); + preg.emplace (regexp, cflags, _("Invalid regexp")); } /* Search through the partial symtabs *first* for all symbols @@ -4379,8 +4368,8 @@ search_symbols (const char *regexp, enum search_domain kind, }, [&] (const char *symname) { - return (!preg_p || regexec (&preg, symname, - 0, NULL, 0) == 0); + return (!preg || preg->exec (symname, + 0, NULL, 0) == 0); }, NULL, kind); @@ -4415,9 +4404,9 @@ search_symbols (const char *regexp, enum search_domain kind, || MSYMBOL_TYPE (msymbol) == ourtype3 || MSYMBOL_TYPE (msymbol) == ourtype4) { - if (!preg_p - || regexec (&preg, MSYMBOL_NATURAL_NAME (msymbol), 0, - NULL, 0) == 0) + if (!preg + || preg->exec (MSYMBOL_NATURAL_NAME (msymbol), 0, + NULL, 0) == 0) { /* Note: An important side-effect of these lookup functions is to expand the symbol table if msymbol is found, for the @@ -4459,9 +4448,9 @@ search_symbols (const char *regexp, enum search_domain kind, files, nfiles, 1)) && file_matches (symtab_to_fullname (real_symtab), files, nfiles, 0))) - && ((!preg_p - || regexec (&preg, SYMBOL_NATURAL_NAME (sym), 0, - NULL, 0) == 0) + && ((!preg + || preg->exec (SYMBOL_NATURAL_NAME (sym), 0, + NULL, 0) == 0) && ((kind == VARIABLES_DOMAIN && SYMBOL_CLASS (sym) != LOC_TYPEDEF && SYMBOL_CLASS (sym) != LOC_UNRESOLVED @@ -4517,9 +4506,8 @@ search_symbols (const char *regexp, enum search_domain kind, || MSYMBOL_TYPE (msymbol) == ourtype3 || MSYMBOL_TYPE (msymbol) == ourtype4) { - if (!preg_p - || regexec (&preg, MSYMBOL_NATURAL_NAME (msymbol), 0, - NULL, 0) == 0) + if (!preg || preg->exec (MSYMBOL_NATURAL_NAME (msymbol), 0, + NULL, 0) == 0) { /* For functions we can do a quick check of whether the symbol might be found via find_pc_symtab. */ diff --git a/gdb/utils.c b/gdb/utils.c index b4332f8..88a1789 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1038,58 +1038,6 @@ make_hex_string (const gdb_byte *data, size_t length) \f -/* A cleanup function that calls regfree. */ - -static void -do_regfree_cleanup (void *r) -{ - regfree ((regex_t *) r); -} - -/* Create a new cleanup that frees the compiled regular expression R. */ - -struct cleanup * -make_regfree_cleanup (regex_t *r) -{ - return make_cleanup (do_regfree_cleanup, r); -} - -/* Return an xmalloc'd error message resulting from a regular - expression compilation failure. */ - -char * -get_regcomp_error (int code, regex_t *rx) -{ - size_t length = regerror (code, rx, NULL, 0); - char *result = (char *) xmalloc (length); - - regerror (code, rx, result, length); - return result; -} - -/* Compile a regexp and throw an exception on error. This returns a - cleanup to free the resulting pattern on success. RX must not be - NULL. */ - -struct cleanup * -compile_rx_or_error (regex_t *pattern, const char *rx, const char *message) -{ - int code; - - gdb_assert (rx != NULL); - - code = regcomp (pattern, rx, REG_NOSUB); - if (code != 0) - { - char *err = get_regcomp_error (code, pattern); - - make_cleanup (xfree, err); - error (("%s: %s"), message, err); - } - - return make_regfree_cleanup (pattern); -} - /* A cleanup that simply calls ui_unregister_input_event_handler. */ static void -- 2.5.5 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 2/2] C++-ify break-catch-throw 2017-06-05 12:29 ` Pedro Alves 2017-06-05 12:32 ` Pedro Alves @ 2017-06-05 19:27 ` Tom Tromey 2017-06-06 16:22 ` Pedro Alves 1 sibling, 1 reply; 17+ messages in thread From: Tom Tromey @ 2017-06-05 19:27 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, gdb-patches >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: Pedro> So something like this. Builds and gdb starts, but I have Pedro> not regtested it. Thanks for doing this. I should have noticed the compile_rx_or_error problem in my patch -- IIRC I wrote compile_rx_or_error and even violated my own rule about cleanup naming (that cleanup-returning functions should start "make_cleanup_") at the time. Double ouch. Pedro> In a couple places, this either forces moving the regex object Pedro> to the heap, or to wrap it in gdb::optional. In the cases Pedro> where we already have to keep the regex string around, Pedro> it ends up being not maximally efficient memory-wise, but I don't Pedro> think it really matters. We're considering std::string for Pedro> those same strings, which grows the structs more than that, anyway Pedro> (for size/capacity). I was thinking that perhaps the regexp object should just have its own optionality. But, I think your way is also fine, maybe more principled in a way. I didn't look at all the cases, but at least for "catch throw" it doesn't seem to matter much. For a few bytes in that structure to be a problem, someone would have to use thousands or millions of catchpoints, which seems absurd. Pedro> WDYT? Looks great, I will wait for this to land before round 2. Pedro> +ada_add_standard_exceptions (gdb_regex *preg, VEC(ada_exc_info) **exceptions) The various callees could use const if the exec method was const. Pedro> + /* Compile a regexp and throw an exception on error, including Pedro> + MESSAGE. REGEX and MESSAGE must not be NULL. */ Pedro> + gdb_regex (const char *regex, int cflags, Pedro> + const char *message); Use ATTRIBUTE_NONNULL? Pedro> + /* Wrapper around ::regexec. */ Pedro> + int exec (const char *string, size_t nmatch, regmatch_t pmatch[], int eflags); Pedro> + Pedro> + /* Wrapper around ::re_search. */ Pedro> + int search (const char *string, int size, Pedro> + int startpos, int range, struct re_registers *regs); Could both be 'const', I think. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 2/2] C++-ify break-catch-throw 2017-06-05 19:27 ` Tom Tromey @ 2017-06-06 16:22 ` Pedro Alves 2017-06-07 12:38 ` Tom Tromey 0 siblings, 1 reply; 17+ messages in thread From: Pedro Alves @ 2017-06-06 16:22 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 06/05/2017 08:27 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: > > Pedro> So something like this. Builds and gdb starts, but I have > Pedro> not regtested it. > > Thanks for doing this. I should have noticed the compile_rx_or_error > problem in my patch -- IIRC I wrote compile_rx_or_error and even > violated my own rule about cleanup naming (that cleanup-returning > functions should start "make_cleanup_") at the time. Double ouch. > > Pedro> In a couple places, this either forces moving the regex object > Pedro> to the heap, or to wrap it in gdb::optional. In the cases > Pedro> where we already have to keep the regex string around, > Pedro> it ends up being not maximally efficient memory-wise, but I don't > Pedro> think it really matters. We're considering std::string for > Pedro> those same strings, which grows the structs more than that, anyway > Pedro> (for size/capacity). > > I was thinking that perhaps the regexp object should just have its own > optionality. But, I think your way is also fine, maybe more principled > in a way. Yeah. Today I tried adding an optional_regex type that was a variant of the gdb_regex from the previous patch, except that it also kept a pointer to the source string, and then optionality was inferred from NULL-nesss of that string pointer. It looked neat, until I stumbled on the fact that some callers would like to pass an non-owning source string while others want to pass in an owning string. I didn't feel like 3 regex types (compiled_regex, optional_regex and owning_optional_regex) would really clarify things, so I kept it simple and rolled back to what I had before. I renamed gdb_regex to compiled_regex in the process though, and also realized that linux-tdep.c:mapping_regexes would look a bit clearer with in-class initialization. > > I didn't look at all the cases, but at least for "catch throw" it > doesn't seem to matter much. For a few bytes in that structure to be a > problem, someone would have to use thousands or millions of catchpoints, > which seems absurd. > > Pedro> WDYT? > > Looks great, I will wait for this to land before round 2. > > Pedro> +ada_add_standard_exceptions (gdb_regex *preg, VEC(ada_exc_info) **exceptions) > > The various callees could use const if the exec method was const. > Done. > Pedro> + /* Compile a regexp and throw an exception on error, including > Pedro> + MESSAGE. REGEX and MESSAGE must not be NULL. */ > Pedro> + gdb_regex (const char *regex, int cflags, > Pedro> + const char *message); > > Use ATTRIBUTE_NONNULL? Done. Oddly, ATTRIBUTE_NONNULL takes only one argument, while GCC's attribute nonnull takes a comma separated list of indexes. That's why I'm specifying the attribute more than once (I checked that it warned as expected). > > Pedro> + /* Wrapper around ::regexec. */ > Pedro> + int exec (const char *string, size_t nmatch, regmatch_t pmatch[], int eflags); > Pedro> + > Pedro> + /* Wrapper around ::re_search. */ > Pedro> + int search (const char *string, int size, > Pedro> + int startpos, int range, struct re_registers *regs); > > Could both be 'const', I think. That requires a const_cast in re_search, though. I've done that, assuming that's just a const-correctness issue with the C API, and that the buffer is not truly mutable at search time. Thanks for the review. I've addressed all the above, I think, polished it a bit, and ran regression tests. WDYT? From 625efedb75e4f1c25abc670109895b88de9edf9d Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Tue, 6 Jun 2017 17:16:26 +0100 Subject: [PATCH] Introduce compiled_regex, eliminate make_regfree_cleanup This patch replaces compile_rx_or_error and make_regfree_cleanup with a class that wraps a regex_t. gdb/ChangeLog: 2017-06-06 Pedro Alves <palves@redhat.com> * Makefile.in (SFILES): Add gdb_regex.c. (COMMON_OBS): Add gdb_regex.o. * ada-lang.c (ada_add_standard_exceptions) (ada_add_exceptions_from_frame, name_matches_regex) (ada_add_global_exceptions, ada_exceptions_list_1): Change regex parameter type to compiled_regex. Adjust. (ada_exceptions_list): Use compiled_regex. * break-catch-throw.c (exception_catchpoint::pattern): Now a std::unique_ptr<compiled_regex>. (exception_catchpoint::~exception_catchpoint): Remove regfree call. (check_status_exception_catchpoint): Adjust to use compiled_regex. (handle_gnu_v3_exceptions): Adjust to use compiled_regex. * breakpoint.c (solib_catchpoint::compiled): Now a std::unique_ptr<compiled_regex>. (solib_catchpoint::~solib_catchpoint): Remove regfree call. (check_status_catch_solib): Adjust to use compiled_regex. (add_solib_catchpoint): Adjust to use compiled_regex. * cli/cli-cmds.c (apropos_command): Use compiled_regex. * cli/cli-decode.c (apropos_cmd): Change regex parameter to const compiled_regex reference. Adjust to use it. * cli/cli-decode.h: Remove struct re_pattern_buffer forward declaration. Include "gdb_regex.h". (apropos_cmd): Change regex parameter to const compiled_regex reference. * gdb_regex.c: New file. * gdb_regex.h (make_regfree_cleanup, get_regcomp_error): Delete declarations. (class compiled_regex): New. * linux-tdep.c: Include "common/gdb_optional.h". (struct mapping_regexes): New, factored out from mapping_is_anonymous_p, and adjusted to use compiled_regex. (mapping_is_anonymous_p): Use mapping_regexes wrapped in a gdb::optional and and remove cleanups. Adjust to compiled_regex. * probe.c: Include "common/gdb_optional.h". (collect_probes): Use compiled_regex and gdb::optional and remove cleanups. * skip.c: Include "common/gdb_optional.h". (skiplist_entry::compiled_function_regexp): Now a gdb::optional<compiled_regex>. (skiplist_entry::compiled_function_regexp_is_valid): Delete field. (free_skiplist_entry): Remove regfree call. (compile_skip_regexp, skip_rfunction_p): Adjust to use compiled_regex and gdb::optional. * symtab.c: Include "common/gdb_optional.h". (search_symbols): Use compiled_regex and gdb::optional. * utils.c (do_regfree_cleanup, make_regfree_cleanup) (get_regcomp_error, compile_rx_or_error): Delete. Some bits moved to gdb_regex.c. --- gdb/Makefile.in | 2 ++ gdb/ada-lang.c | 33 ++++++++----------- gdb/break-catch-throw.c | 21 +++++------- gdb/breakpoint.c | 20 +++--------- gdb/cli/cli-cmds.c | 21 ++---------- gdb/cli/cli-decode.c | 11 ++++--- gdb/cli/cli-decode.h | 5 ++- gdb/gdb_regex.c | 57 +++++++++++++++++++++++++++++++++ gdb/gdb_regex.h | 38 +++++++++++++++++++--- gdb/linux-tdep.c | 85 +++++++++++++++++++++++++------------------------ gdb/probe.c | 19 ++++++----- gdb/skip.c | 24 +++----------- gdb/symtab.c | 42 +++++++++--------------- gdb/utils.c | 52 ------------------------------ 14 files changed, 204 insertions(+), 226 deletions(-) create mode 100644 gdb/gdb_regex.c diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 8be73ba..2156438 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -1104,6 +1104,7 @@ SFILES = \ gdb_bfd.c \ gdb-dlfcn.c \ gdb_obstack.c \ + gdb_regex.c \ gdb_usleep.c \ gdbarch.c \ gdbarch-selftests.c \ @@ -1717,6 +1718,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \ gdb_bfd.o \ gdb-dlfcn.o \ gdb_obstack.o \ + gdb_regex.o \ gdb_usleep.o \ gdb_vecs.o \ gdbarch.o \ diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index f90907a..d7c3023 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -13219,14 +13219,15 @@ sort_remove_dups_ada_exceptions_list (VEC(ada_exc_info) **exceptions, gets pushed. */ static void -ada_add_standard_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) +ada_add_standard_exceptions (const compiled_regex *preg, + VEC(ada_exc_info) **exceptions) { int i; for (i = 0; i < ARRAY_SIZE (standard_exc); i++) { if (preg == NULL - || regexec (preg, standard_exc[i], 0, NULL, 0) == 0) + || preg->exec (standard_exc[i], 0, NULL, 0) == 0) { struct bound_minimal_symbol msymbol = ada_lookup_simple_minsym (standard_exc[i]); @@ -13253,7 +13254,8 @@ ada_add_standard_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) gets pushed. */ static void -ada_add_exceptions_from_frame (regex_t *preg, struct frame_info *frame, +ada_add_exceptions_from_frame (const compiled_regex *preg, + struct frame_info *frame, VEC(ada_exc_info) **exceptions) { const struct block *block = get_frame_block (frame, 0); @@ -13290,10 +13292,10 @@ ada_add_exceptions_from_frame (regex_t *preg, struct frame_info *frame, /* Return true if NAME matches PREG or if PREG is NULL. */ static bool -name_matches_regex (const char *name, regex_t *preg) +name_matches_regex (const char *name, const compiled_regex *preg) { return (preg == NULL - || regexec (preg, ada_decode (name), 0, NULL, 0) == 0); + || preg->exec (ada_decode (name), 0, NULL, 0) == 0); } /* Add all exceptions defined globally whose name name match @@ -13316,7 +13318,8 @@ name_matches_regex (const char *name, regex_t *preg) gets pushed. */ static void -ada_add_global_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) +ada_add_global_exceptions (const compiled_regex *preg, + VEC(ada_exc_info) **exceptions) { struct objfile *objfile; struct compunit_symtab *s; @@ -13364,7 +13367,7 @@ ada_add_global_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) do not match. Otherwise, all exceptions are listed. */ static VEC(ada_exc_info) * -ada_exceptions_list_1 (regex_t *preg) +ada_exceptions_list_1 (const compiled_regex *preg) { VEC(ada_exc_info) *result = NULL; struct cleanup *old_chain @@ -13417,19 +13420,11 @@ ada_exceptions_list_1 (regex_t *preg) VEC(ada_exc_info) * ada_exceptions_list (const char *regexp) { - VEC(ada_exc_info) *result = NULL; - struct cleanup *old_chain = NULL; - regex_t reg; - - if (regexp != NULL) - old_chain = compile_rx_or_error (®, regexp, - _("invalid regular expression")); + if (regexp == NULL) + return ada_exceptions_list_1 (NULL); - result = ada_exceptions_list_1 (regexp != NULL ? ® : NULL); - - if (old_chain != NULL) - do_cleanups (old_chain); - return result; + compiled_regex reg (regexp, REG_NOSUB, _("invalid regular expression")); + return ada_exceptions_list_1 (®); } /* Implement the "info exceptions" command. */ diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c index 7731c5e..0074d06 100644 --- a/gdb/break-catch-throw.c +++ b/gdb/break-catch-throw.c @@ -87,10 +87,10 @@ struct exception_catchpoint : public breakpoint char *exception_rx; - /* If non-NULL, an xmalloc'd, compiled regular expression which is - used to determine which exceptions to stop on. */ + /* If non-NULL, a compiled regular expression which is used to + determine which exceptions to stop on. */ - regex_t *pattern; + std::unique_ptr<compiled_regex> pattern; }; \f @@ -145,8 +145,6 @@ classify_exception_breakpoint (struct breakpoint *b) exception_catchpoint::~exception_catchpoint () { xfree (this->exception_rx); - if (this->pattern != NULL) - regfree (this->pattern); } /* Implement the 'check_status' method. */ @@ -185,7 +183,7 @@ check_status_exception_catchpoint (struct bpstats *bs) if (!type_name.empty ()) { - if (regexec (self->pattern, type_name.c_str (), 0, NULL, 0) != 0) + if (self->pattern->exec (type_name.c_str (), 0, NULL, 0) != 0) bs->stop = 0; } } @@ -377,15 +375,12 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx, const char *cond_string, enum exception_event_kind ex_event, int from_tty) { - regex_t *pattern = NULL; + std::unique_ptr<compiled_regex> pattern; if (except_rx != NULL) { - pattern = XNEW (regex_t); - make_cleanup (xfree, pattern); - - compile_rx_or_error (pattern, except_rx, - _("invalid type-matching regexp")); + pattern.reset (new compiled_regex (except_rx, REG_NOSUB, + _("invalid type-matching regexp"))); } std::unique_ptr<exception_catchpoint> cp (new exception_catchpoint ()); @@ -397,7 +392,7 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx, cp->type = bp_breakpoint; cp->kind = ex_event; cp->exception_rx = except_rx; - cp->pattern = pattern; + cp->pattern = std::move (pattern); re_set_exception_catchpoint (cp.get ()); diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index e84d164..053ccef 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -8284,13 +8284,11 @@ struct solib_catchpoint : public breakpoint /* Regular expression to match, if any. COMPILED is only valid when REGEX is non-NULL. */ char *regex; - regex_t compiled; + std::unique_ptr<compiled_regex> compiled; }; solib_catchpoint::~solib_catchpoint () { - if (this->regex) - regfree (&this->compiled); xfree (this->regex); } @@ -8358,7 +8356,7 @@ check_status_catch_solib (struct bpstats *bs) ++ix) { if (!self->regex - || regexec (&self->compiled, iter->so_name, 0, NULL, 0) == 0) + || self->compiled->exec (iter->so_name, 0, NULL, 0) == 0) return; } } @@ -8372,7 +8370,7 @@ check_status_catch_solib (struct bpstats *bs) ++ix) { if (!self->regex - || regexec (&self->compiled, iter, 0, NULL, 0) == 0) + || self->compiled->exec (iter, 0, NULL, 0) == 0) return; } } @@ -8488,16 +8486,8 @@ add_solib_catchpoint (const char *arg, int is_load, int is_temp, int enabled) if (*arg != '\0') { - int errcode; - - errcode = regcomp (&c->compiled, arg, REG_NOSUB); - if (errcode != 0) - { - char *err = get_regcomp_error (errcode, &c->compiled); - - make_cleanup (xfree, err); - error (_("Invalid regexp (%s): %s"), err, arg); - } + c->compiled.reset (new compiled_regex (arg, REG_NOSUB, + _("Invalid regexp"))); c->regex = xstrdup (arg); } diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 2a5b128..0930342 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -1336,28 +1336,13 @@ show_user (char *args, int from_tty) static void apropos_command (char *searchstr, int from_tty) { - regex_t pattern; - int code; - if (searchstr == NULL) error (_("REGEXP string is empty")); - code = regcomp (&pattern, searchstr, REG_ICASE); - if (code == 0) - { - struct cleanup *cleanups; + compiled_regex pattern (searchstr, REG_ICASE, + _("Error in regular expression")); - cleanups = make_regfree_cleanup (&pattern); - apropos_cmd (gdb_stdout, cmdlist, &pattern, ""); - do_cleanups (cleanups); - } - else - { - char *err = get_regcomp_error (code, &pattern); - - make_cleanup (xfree, err); - error (_("Error in regular expression: %s"), err); - } + apropos_cmd (gdb_stdout, cmdlist, pattern, ""); } /* Subroutine of alias_command to simplify it. diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index d386d02..81d8165 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -917,7 +917,7 @@ add_com_suppress_notification (const char *name, enum command_class theclass, void apropos_cmd (struct ui_file *stream, struct cmd_list_element *commandlist, - struct re_pattern_buffer *regex, const char *prefix) + const compiled_regex ®ex, const char *prefix) { struct cmd_list_element *c; int returnvalue; @@ -928,9 +928,10 @@ apropos_cmd (struct ui_file *stream, returnvalue = -1; /* Needed to avoid double printing. */ if (c->name != NULL) { + size_t name_len = strlen (c->name); + /* Try to match against the name. */ - returnvalue = re_search (regex, c->name, strlen(c->name), - 0, strlen (c->name), NULL); + returnvalue = regex.search (c->name, name_len, 0, name_len, NULL); if (returnvalue >= 0) { print_help_for_command (c, prefix, @@ -939,8 +940,10 @@ apropos_cmd (struct ui_file *stream, } if (c->doc != NULL && returnvalue < 0) { + size_t doc_len = strlen (c->doc); + /* Try to match against documentation. */ - if (re_search(regex,c->doc,strlen(c->doc),0,strlen(c->doc),NULL) >=0) + if (regex.search (c->doc, doc_len, 0, doc_len, NULL) >= 0) { print_help_for_command (c, prefix, 0 /* don't recurse */, stream); diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h index 66159fd..bf92344 100644 --- a/gdb/cli/cli-decode.h +++ b/gdb/cli/cli-decode.h @@ -23,8 +23,7 @@ /* Include the public interfaces. */ #include "command.h" - -struct re_pattern_buffer; +#include "gdb_regex.h" #if 0 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum @@ -234,7 +233,7 @@ extern void help_cmd_list (struct cmd_list_element *, enum command_class, extern void help_cmd (const char *, struct ui_file *); extern void apropos_cmd (struct ui_file *, struct cmd_list_element *, - struct re_pattern_buffer *, const char *); + const compiled_regex &, const char *); /* Used to mark commands that don't do anything. If we just leave the function field NULL, the command is interpreted as a help topic, or diff --git a/gdb/gdb_regex.c b/gdb/gdb_regex.c new file mode 100644 index 0000000..2d00d92 --- /dev/null +++ b/gdb/gdb_regex.c @@ -0,0 +1,57 @@ +/* Copyright (C) 2011-2017 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#include "defs.h" +#include "gdb_regex.h" + +compiled_regex::compiled_regex (const char *regex, int cflags, + const char *message) +{ + gdb_assert (regex != NULL); + gdb_assert (message != NULL); + + int code = regcomp (&m_pattern, regex, cflags); + if (code != 0) + { + size_t length = regerror (code, &m_pattern, NULL, 0); + std::unique_ptr<char[]> err (new char[length]); + + regerror (code, &m_pattern, err.get (), length); + error (("%s: %s"), message, err.get ()); + } +} + +compiled_regex::~compiled_regex () +{ + regfree (&m_pattern); +} + +int +compiled_regex::exec (const char *string, size_t nmatch, + regmatch_t pmatch[], int eflags) const +{ + return regexec (&m_pattern, string, nmatch, pmatch, eflags); +} + +int +compiled_regex::search (const char *string, + int length, int start, int range, + struct re_registers *regs) const +{ + return re_search (const_cast<regex_t *> (&m_pattern), + string, length, start, range, regs); +} diff --git a/gdb/gdb_regex.h b/gdb/gdb_regex.h index 0be26ef..7873231 100644 --- a/gdb/gdb_regex.h +++ b/gdb/gdb_regex.h @@ -27,10 +27,38 @@ # include <regex.h> #endif -/* From utils.c. */ -struct cleanup *make_regfree_cleanup (regex_t *); -char *get_regcomp_error (int, regex_t *); -struct cleanup *compile_rx_or_error (regex_t *pattern, const char *rx, - const char *message); +/* A compiled regex. This is mainly a wrapper around regex_t. The + the constructor throws on regcomp error and the destructor is + responsible for calling regfree. The former means that it's not + possible to create an instance of compiled_regex that isn't + compiled, hence the name. */ +class compiled_regex +{ +public: + /* Compile a regexp and throw an exception on error, including + MESSAGE. REGEX and MESSAGE must not be NULL. */ + compiled_regex (const char *regex, int cflags, + const char *message) + ATTRIBUTE_NONNULL (2) ATTRIBUTE_NONNULL (4); + + ~compiled_regex (); + + /* Disable copy. */ + compiled_regex (const compiled_regex&) = delete; + void operator= (const compiled_regex&) = delete; + + /* Wrapper around ::regexec. */ + int exec (const char *string, + size_t nmatch, regmatch_t pmatch[], + int eflags) const; + + /* Wrapper around ::re_search. */ + int search (const char *string, int size, int startpos, + int range, struct re_registers *regs) const; + +private: + /* The compiled pattern. */ + regex_t m_pattern; +}; #endif /* not GDB_REGEX_H */ diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c index 016aadf..1afa8d7 100644 --- a/gdb/linux-tdep.c +++ b/gdb/linux-tdep.c @@ -38,6 +38,7 @@ #include "gdbcmd.h" #include "gdb_regex.h" #include "common/enum-flags.h" +#include "common/gdb_optional.h" #include <ctype.h> @@ -493,6 +494,44 @@ decode_vmflags (char *p, struct smaps_vmflags *v) } } +/* Regexes used by mapping_is_anonymous_p. Put in a structure because + they're initialized lazily. */ + +struct mapping_regexes +{ + /* Matches "/dev/zero" filenames (with or without the "(deleted)" + string in the end). We know for sure, based on the Linux kernel + code, that memory mappings whose associated filename is + "/dev/zero" are guaranteed to be MAP_ANONYMOUS. */ + compiled_regex dev_zero + {"^/dev/zero\\( (deleted)\\)\\?$", REG_NOSUB, + _("Could not compile regex to match /dev/zero filename")}; + + /* Matches "/SYSV%08x" filenames (with or without the "(deleted)" + string in the end). These filenames refer to shared memory + (shmem), and memory mappings associated with them are + MAP_ANONYMOUS as well. */ + compiled_regex shmem_file + {"^/\\?SYSV[0-9a-fA-F]\\{8\\}\\( (deleted)\\)\\?$", REG_NOSUB, + _("Could not compile regex to match shmem filenames")}; + + /* A heuristic we use to try to mimic the Linux kernel's 'n_link == + 0' code, which is responsible to decide if it is dealing with a + 'MAP_SHARED | MAP_ANONYMOUS' mapping. In other words, if + FILE_DELETED matches, it does not necessarily mean that we are + dealing with an anonymous shared mapping. However, there is no + easy way to detect this currently, so this is the best + approximation we have. + + As a result, GDB will dump readonly pages of deleted executables + when using the default value of coredump_filter (0x33), while the + Linux kernel will not dump those pages. But we can live with + that. */ + compiled_regex file_deleted + {" (deleted)$", REG_NOSUB, + _("Could not compile regex to match '<file> (deleted)'")}; +}; + /* Return 1 if the memory mapping is anonymous, 0 otherwise. FILENAME is the name of the file present in the first line of the @@ -506,52 +545,16 @@ decode_vmflags (char *p, struct smaps_vmflags *v) static int mapping_is_anonymous_p (const char *filename) { - static regex_t dev_zero_regex, shmem_file_regex, file_deleted_regex; + static gdb::optional<mapping_regexes> regexes; static int init_regex_p = 0; if (!init_regex_p) { - struct cleanup *c = make_cleanup (null_cleanup, NULL); - /* Let's be pessimistic and assume there will be an error while compiling the regex'es. */ init_regex_p = -1; - /* DEV_ZERO_REGEX matches "/dev/zero" filenames (with or - without the "(deleted)" string in the end). We know for - sure, based on the Linux kernel code, that memory mappings - whose associated filename is "/dev/zero" are guaranteed to be - MAP_ANONYMOUS. */ - compile_rx_or_error (&dev_zero_regex, "^/dev/zero\\( (deleted)\\)\\?$", - _("Could not compile regex to match /dev/zero " - "filename")); - /* SHMEM_FILE_REGEX matches "/SYSV%08x" filenames (with or - without the "(deleted)" string in the end). These filenames - refer to shared memory (shmem), and memory mappings - associated with them are MAP_ANONYMOUS as well. */ - compile_rx_or_error (&shmem_file_regex, - "^/\\?SYSV[0-9a-fA-F]\\{8\\}\\( (deleted)\\)\\?$", - _("Could not compile regex to match shmem " - "filenames")); - /* FILE_DELETED_REGEX is a heuristic we use to try to mimic the - Linux kernel's 'n_link == 0' code, which is responsible to - decide if it is dealing with a 'MAP_SHARED | MAP_ANONYMOUS' - mapping. In other words, if FILE_DELETED_REGEX matches, it - does not necessarily mean that we are dealing with an - anonymous shared mapping. However, there is no easy way to - detect this currently, so this is the best approximation we - have. - - As a result, GDB will dump readonly pages of deleted - executables when using the default value of coredump_filter - (0x33), while the Linux kernel will not dump those pages. - But we can live with that. */ - compile_rx_or_error (&file_deleted_regex, " (deleted)$", - _("Could not compile regex to match " - "'<file> (deleted)'")); - /* We will never release these regexes, so just discard the - cleanups. */ - discard_cleanups (c); + regexes.emplace (); /* If we reached this point, then everything succeeded. */ init_regex_p = 1; @@ -573,9 +576,9 @@ mapping_is_anonymous_p (const char *filename) } if (*filename == '\0' - || regexec (&dev_zero_regex, filename, 0, NULL, 0) == 0 - || regexec (&shmem_file_regex, filename, 0, NULL, 0) == 0 - || regexec (&file_deleted_regex, filename, 0, NULL, 0) == 0) + || regexes->dev_zero.exec (filename, 0, NULL, 0) == 0 + || regexes->shmem_file.exec (filename, 0, NULL, 0) == 0 + || regexes->file_deleted.exec (filename, 0, NULL, 0) == 0) return 1; return 0; diff --git a/gdb/probe.c b/gdb/probe.c index e65e031..cb2dcdb 100644 --- a/gdb/probe.c +++ b/gdb/probe.c @@ -36,6 +36,7 @@ #include "location.h" #include <ctype.h> #include <algorithm> +#include "common/gdb_optional.h" typedef struct bound_probe bound_probe_s; DEF_VEC_O (bound_probe_s); @@ -288,18 +289,17 @@ collect_probes (char *objname, char *provider, char *probe_name, { struct objfile *objfile; VEC (bound_probe_s) *result = NULL; - struct cleanup *cleanup, *cleanup_temps; - regex_t obj_pat, prov_pat, probe_pat; + struct cleanup *cleanup; + gdb::optional<compiled_regex> obj_pat, prov_pat, probe_pat; cleanup = make_cleanup (VEC_cleanup (bound_probe_s), &result); - cleanup_temps = make_cleanup (null_cleanup, NULL); if (provider != NULL) - compile_rx_or_error (&prov_pat, provider, _("Invalid provider regexp")); + prov_pat.emplace (provider, REG_NOSUB, _("Invalid provider regexp")); if (probe_name != NULL) - compile_rx_or_error (&probe_pat, probe_name, _("Invalid probe regexp")); + probe_pat.emplace (probe_name, REG_NOSUB, _("Invalid probe regexp")); if (objname != NULL) - compile_rx_or_error (&obj_pat, objname, _("Invalid object file regexp")); + obj_pat.emplace (objname, REG_NOSUB, _("Invalid object file regexp")); ALL_OBJFILES (objfile) { @@ -312,7 +312,7 @@ collect_probes (char *objname, char *provider, char *probe_name, if (objname) { - if (regexec (&obj_pat, objfile_name (objfile), 0, NULL, 0) != 0) + if (obj_pat->exec (objfile_name (objfile), 0, NULL, 0) != 0) continue; } @@ -326,11 +326,11 @@ collect_probes (char *objname, char *provider, char *probe_name, continue; if (provider - && regexec (&prov_pat, probe->provider, 0, NULL, 0) != 0) + && prov_pat->exec (probe->provider, 0, NULL, 0) != 0) continue; if (probe_name - && regexec (&probe_pat, probe->name, 0, NULL, 0) != 0) + && probe_pat->exec (probe->name, 0, NULL, 0) != 0) continue; bound.objfile = objfile; @@ -339,7 +339,6 @@ collect_probes (char *objname, char *provider, char *probe_name, } } - do_cleanups (cleanup_temps); discard_cleanups (cleanup); return result; } diff --git a/gdb/skip.c b/gdb/skip.c index 4bd8a9e..e767f83 100644 --- a/gdb/skip.c +++ b/gdb/skip.c @@ -34,6 +34,7 @@ #include "filenames.h" #include "fnmatch.h" #include "gdb_regex.h" +#include "common/gdb_optional.h" struct skiplist_entry { @@ -57,10 +58,7 @@ struct skiplist_entry char *function; /* If this is a function regexp, the compiled form. */ - regex_t compiled_function_regexp; - - /* Non-zero if the function regexp has been compiled. */ - int compiled_function_regexp_is_valid; + gdb::optional<compiled_regex> compiled_function_regexp; int enabled; @@ -112,8 +110,6 @@ free_skiplist_entry (struct skiplist_entry *e) { xfree (e->file); xfree (e->function); - if (e->function_is_regexp && e->compiled_function_regexp_is_valid) - regfree (&e->compiled_function_regexp); xfree (e); } @@ -202,7 +198,6 @@ skip_function_command (char *arg, int from_tty) static void compile_skip_regexp (struct skiplist_entry *e, const char *message) { - int code; int flags = REG_NOSUB; #ifdef REG_EXTENDED @@ -210,16 +205,7 @@ compile_skip_regexp (struct skiplist_entry *e, const char *message) #endif gdb_assert (e->function_is_regexp && e->function != NULL); - - code = regcomp (&e->compiled_function_regexp, e->function, flags); - if (code != 0) - { - char *err = get_regcomp_error (code, &e->compiled_function_regexp); - - make_cleanup (xfree, err); - error (_("%s: %s"), message, err); - } - e->compiled_function_regexp_is_valid = 1; + e->compiled_function_regexp.emplace (e->function, flags, message); } /* Process "skip ..." that does not match "skip file" or "skip function". */ @@ -601,8 +587,8 @@ static int skip_rfunction_p (struct skiplist_entry *e, const char *function_name) { gdb_assert (e->function != NULL && e->function_is_regexp - && e->compiled_function_regexp_is_valid); - return (regexec (&e->compiled_function_regexp, function_name, 0, NULL, 0) + && e->compiled_function_regexp); + return (e->compiled_function_regexp->exec (function_name, 0, NULL, 0) == 0); } diff --git a/gdb/symtab.c b/gdb/symtab.c index 22d81fa..f1daf8c 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -62,6 +62,7 @@ #include "parser-defs.h" #include "completer.h" #include "progspace-and-thread.h" +#include "common/gdb_optional.h" /* Forward declarations for local functions. */ @@ -4299,9 +4300,7 @@ search_symbols (const char *regexp, enum search_domain kind, struct symbol_search *found; struct symbol_search *tail; int nfound; - /* This is true if PREG contains valid data, false otherwise. */ - bool preg_p; - regex_t preg; + gdb::optional<compiled_regex> preg; /* OLD_CHAIN .. RETVAL_CHAIN is always freed, RETVAL_CHAIN .. current CLEANUP_CHAIN is freed only in the case of an error. */ @@ -4316,7 +4315,6 @@ search_symbols (const char *regexp, enum search_domain kind, ourtype4 = types4[kind]; *matches = NULL; - preg_p = false; if (regexp != NULL) { @@ -4355,18 +4353,9 @@ search_symbols (const char *regexp, enum search_domain kind, } } - errcode = regcomp (&preg, regexp, - REG_NOSUB | (case_sensitivity == case_sensitive_off - ? REG_ICASE : 0)); - if (errcode != 0) - { - char *err = get_regcomp_error (errcode, &preg); - - make_cleanup (xfree, err); - error (_("Invalid regexp (%s): %s"), err, regexp); - } - preg_p = true; - make_regfree_cleanup (&preg); + int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off + ? REG_ICASE : 0); + preg.emplace (regexp, cflags, _("Invalid regexp")); } /* Search through the partial symtabs *first* for all symbols @@ -4379,8 +4368,8 @@ search_symbols (const char *regexp, enum search_domain kind, }, [&] (const char *symname) { - return (!preg_p || regexec (&preg, symname, - 0, NULL, 0) == 0); + return (!preg || preg->exec (symname, + 0, NULL, 0) == 0); }, NULL, kind); @@ -4415,9 +4404,9 @@ search_symbols (const char *regexp, enum search_domain kind, || MSYMBOL_TYPE (msymbol) == ourtype3 || MSYMBOL_TYPE (msymbol) == ourtype4) { - if (!preg_p - || regexec (&preg, MSYMBOL_NATURAL_NAME (msymbol), 0, - NULL, 0) == 0) + if (!preg + || preg->exec (MSYMBOL_NATURAL_NAME (msymbol), 0, + NULL, 0) == 0) { /* Note: An important side-effect of these lookup functions is to expand the symbol table if msymbol is found, for the @@ -4459,9 +4448,9 @@ search_symbols (const char *regexp, enum search_domain kind, files, nfiles, 1)) && file_matches (symtab_to_fullname (real_symtab), files, nfiles, 0))) - && ((!preg_p - || regexec (&preg, SYMBOL_NATURAL_NAME (sym), 0, - NULL, 0) == 0) + && ((!preg + || preg->exec (SYMBOL_NATURAL_NAME (sym), 0, + NULL, 0) == 0) && ((kind == VARIABLES_DOMAIN && SYMBOL_CLASS (sym) != LOC_TYPEDEF && SYMBOL_CLASS (sym) != LOC_UNRESOLVED @@ -4517,9 +4506,8 @@ search_symbols (const char *regexp, enum search_domain kind, || MSYMBOL_TYPE (msymbol) == ourtype3 || MSYMBOL_TYPE (msymbol) == ourtype4) { - if (!preg_p - || regexec (&preg, MSYMBOL_NATURAL_NAME (msymbol), 0, - NULL, 0) == 0) + if (!preg || preg->exec (MSYMBOL_NATURAL_NAME (msymbol), 0, + NULL, 0) == 0) { /* For functions we can do a quick check of whether the symbol might be found via find_pc_symtab. */ diff --git a/gdb/utils.c b/gdb/utils.c index b4332f8..88a1789 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1038,58 +1038,6 @@ make_hex_string (const gdb_byte *data, size_t length) \f -/* A cleanup function that calls regfree. */ - -static void -do_regfree_cleanup (void *r) -{ - regfree ((regex_t *) r); -} - -/* Create a new cleanup that frees the compiled regular expression R. */ - -struct cleanup * -make_regfree_cleanup (regex_t *r) -{ - return make_cleanup (do_regfree_cleanup, r); -} - -/* Return an xmalloc'd error message resulting from a regular - expression compilation failure. */ - -char * -get_regcomp_error (int code, regex_t *rx) -{ - size_t length = regerror (code, rx, NULL, 0); - char *result = (char *) xmalloc (length); - - regerror (code, rx, result, length); - return result; -} - -/* Compile a regexp and throw an exception on error. This returns a - cleanup to free the resulting pattern on success. RX must not be - NULL. */ - -struct cleanup * -compile_rx_or_error (regex_t *pattern, const char *rx, const char *message) -{ - int code; - - gdb_assert (rx != NULL); - - code = regcomp (pattern, rx, REG_NOSUB); - if (code != 0) - { - char *err = get_regcomp_error (code, pattern); - - make_cleanup (xfree, err); - error (("%s: %s"), message, err); - } - - return make_regfree_cleanup (pattern); -} - /* A cleanup that simply calls ui_unregister_input_event_handler. */ static void -- 2.5.5 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 2/2] C++-ify break-catch-throw 2017-06-06 16:22 ` Pedro Alves @ 2017-06-07 12:38 ` Tom Tromey 2017-06-07 13:40 ` [pushed] Introduce compiled_regex, eliminate make_regfree_cleanup (Re: [RFA 2/2] C++-ify break-catch-throw) Pedro Alves 0 siblings, 1 reply; 17+ messages in thread From: Tom Tromey @ 2017-06-07 12:38 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, gdb-patches >>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: Pedro> That requires a const_cast in re_search, though. I've done that, Pedro> assuming that's just a const-correctness issue with the C API, Pedro> and that the buffer is not truly mutable at search time. Now I am not sure, given libiberty/regex.c:re_compile_fastmap. Pedro> WDYT? Looks great. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* [pushed] Introduce compiled_regex, eliminate make_regfree_cleanup (Re: [RFA 2/2] C++-ify break-catch-throw) 2017-06-07 12:38 ` Tom Tromey @ 2017-06-07 13:40 ` Pedro Alves 0 siblings, 0 replies; 17+ messages in thread From: Pedro Alves @ 2017-06-07 13:40 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 06/07/2017 01:38 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes: > > Pedro> That requires a const_cast in re_search, though. I've done that, > Pedro> assuming that's just a const-correctness issue with the C API, > Pedro> and that the buffer is not truly mutable at search time. > > Now I am not sure, given libiberty/regex.c:re_compile_fastmap. Indeed, thanks for checking! I've left the const in compiled_regex::exec, but removed it from compiled_regex::search and elsewhere, even when not strictly necessary, assuming a callee might reasonably want to call search at some point. > > Pedro> WDYT? > > Looks great. > Here's what I pushed, then. From 2d7cc5c7973b6d1bdd9205288863bedadeaf8b41 Mon Sep 17 00:00:00 2001 From: Pedro Alves <palves@redhat.com> Date: Wed, 7 Jun 2017 14:21:40 +0100 Subject: [PATCH] Introduce compiled_regex, eliminate make_regfree_cleanup This patch replaces compile_rx_or_error and make_regfree_cleanup with a class that wraps a regex_t. gdb/ChangeLog: 2017-06-07 Pedro Alves <palves@redhat.com> * Makefile.in (SFILES): Add gdb_regex.c. (COMMON_OBS): Add gdb_regex.o. * ada-lang.c (ada_add_standard_exceptions) (ada_add_exceptions_from_frame, name_matches_regex) (ada_add_global_exceptions, ada_exceptions_list_1): Change regex parameter type to compiled_regex. Adjust. (ada_exceptions_list): Use compiled_regex. * break-catch-throw.c (exception_catchpoint::pattern): Now a std::unique_ptr<compiled_regex>. (exception_catchpoint::~exception_catchpoint): Remove regfree call. (check_status_exception_catchpoint): Adjust to use compiled_regex. (handle_gnu_v3_exceptions): Adjust to use compiled_regex. * breakpoint.c (solib_catchpoint::compiled): Now a std::unique_ptr<compiled_regex>. (solib_catchpoint::~solib_catchpoint): Remove regfree call. (check_status_catch_solib): Adjust to use compiled_regex. (add_solib_catchpoint): Adjust to use compiled_regex. * cli/cli-cmds.c (apropos_command): Use compiled_regex. * cli/cli-decode.c (apropos_cmd): Change regex parameter to compiled_regex reference. Adjust to use it. * cli/cli-decode.h: Remove struct re_pattern_buffer forward declaration. Include "gdb_regex.h". (apropos_cmd): Change regex parameter to compiled_regex reference. * gdb_regex.c: New file. * gdb_regex.h (make_regfree_cleanup, get_regcomp_error): Delete declarations. (class compiled_regex): New. * linux-tdep.c: Include "common/gdb_optional.h". (struct mapping_regexes): New, factored out from mapping_is_anonymous_p, and adjusted to use compiled_regex. (mapping_is_anonymous_p): Use mapping_regexes wrapped in a gdb::optional and remove cleanups. Adjust to compiled_regex. * probe.c: Include "common/gdb_optional.h". (collect_probes): Use compiled_regex and gdb::optional and remove cleanups. * skip.c: Include "common/gdb_optional.h". (skiplist_entry::compiled_function_regexp): Now a gdb::optional<compiled_regex>. (skiplist_entry::compiled_function_regexp_is_valid): Delete field. (free_skiplist_entry): Remove regfree call. (compile_skip_regexp, skip_rfunction_p): Adjust to use compiled_regex and gdb::optional. * symtab.c: Include "common/gdb_optional.h". (search_symbols): Use compiled_regex and gdb::optional. * utils.c (do_regfree_cleanup, make_regfree_cleanup) (get_regcomp_error, compile_rx_or_error): Delete. Some bits moved to gdb_regex.c. --- gdb/ChangeLog | 51 +++++++++++++++++++++++++++++ gdb/Makefile.in | 2 ++ gdb/ada-lang.c | 33 ++++++++----------- gdb/break-catch-throw.c | 21 +++++------- gdb/breakpoint.c | 20 +++--------- gdb/cli/cli-cmds.c | 21 ++---------- gdb/cli/cli-decode.c | 11 ++++--- gdb/cli/cli-decode.h | 5 ++- gdb/gdb_regex.c | 56 ++++++++++++++++++++++++++++++++ gdb/gdb_regex.h | 39 ++++++++++++++++++++--- gdb/linux-tdep.c | 85 +++++++++++++++++++++++++------------------------ gdb/probe.c | 19 ++++++----- gdb/skip.c | 24 +++----------- gdb/symtab.c | 42 +++++++++--------------- gdb/utils.c | 52 ------------------------------ 15 files changed, 255 insertions(+), 226 deletions(-) create mode 100644 gdb/gdb_regex.c diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 3190410..6424388 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,54 @@ +2017-06-07 Pedro Alves <palves@redhat.com> + + * Makefile.in (SFILES): Add gdb_regex.c. + (COMMON_OBS): Add gdb_regex.o. + * ada-lang.c (ada_add_standard_exceptions) + (ada_add_exceptions_from_frame, name_matches_regex) + (ada_add_global_exceptions, ada_exceptions_list_1): Change regex + parameter type to compiled_regex. Adjust. + (ada_exceptions_list): Use compiled_regex. + * break-catch-throw.c (exception_catchpoint::pattern): Now a + std::unique_ptr<compiled_regex>. + (exception_catchpoint::~exception_catchpoint): Remove regfree + call. + (check_status_exception_catchpoint): Adjust to use compiled_regex. + (handle_gnu_v3_exceptions): Adjust to use compiled_regex. + * breakpoint.c (solib_catchpoint::compiled): Now a + std::unique_ptr<compiled_regex>. + (solib_catchpoint::~solib_catchpoint): Remove regfree call. + (check_status_catch_solib): Adjust to use compiled_regex. + (add_solib_catchpoint): Adjust to use compiled_regex. + * cli/cli-cmds.c (apropos_command): Use compiled_regex. + * cli/cli-decode.c (apropos_cmd): Change regex parameter to + compiled_regex reference. Adjust to use it. + * cli/cli-decode.h: Remove struct re_pattern_buffer forward + declaration. Include "gdb_regex.h". + (apropos_cmd): Change regex parameter to compiled_regex reference. + * gdb_regex.c: New file. + * gdb_regex.h (make_regfree_cleanup, get_regcomp_error): Delete + declarations. + (class compiled_regex): New. + * linux-tdep.c: Include "common/gdb_optional.h". + (struct mapping_regexes): New, factored out from + mapping_is_anonymous_p, and adjusted to use compiled_regex. + (mapping_is_anonymous_p): Use mapping_regexes wrapped in a + gdb::optional and remove cleanups. Adjust to compiled_regex. + * probe.c: Include "common/gdb_optional.h". + (collect_probes): Use compiled_regex and gdb::optional and remove + cleanups. + * skip.c: Include "common/gdb_optional.h". + (skiplist_entry::compiled_function_regexp): Now a + gdb::optional<compiled_regex>. + (skiplist_entry::compiled_function_regexp_is_valid): Delete field. + (free_skiplist_entry): Remove regfree call. + (compile_skip_regexp, skip_rfunction_p): Adjust to use + compiled_regex and gdb::optional. + * symtab.c: Include "common/gdb_optional.h". + (search_symbols): Use compiled_regex and gdb::optional. + * utils.c (do_regfree_cleanup, make_regfree_cleanup) + (get_regcomp_error, compile_rx_or_error): Delete. Some bits moved + to gdb_regex.c. + 2017-06-07 Alan Hayward <alan.hayward@arm.com> * regcache.c (regcache::save): Avoid buffer use. diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 8be73ba..2156438 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -1104,6 +1104,7 @@ SFILES = \ gdb_bfd.c \ gdb-dlfcn.c \ gdb_obstack.c \ + gdb_regex.c \ gdb_usleep.c \ gdbarch.c \ gdbarch-selftests.c \ @@ -1717,6 +1718,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \ gdb_bfd.o \ gdb-dlfcn.o \ gdb_obstack.o \ + gdb_regex.o \ gdb_usleep.o \ gdb_vecs.o \ gdbarch.o \ diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index f90907a..57c670e 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -13219,14 +13219,15 @@ sort_remove_dups_ada_exceptions_list (VEC(ada_exc_info) **exceptions, gets pushed. */ static void -ada_add_standard_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) +ada_add_standard_exceptions (compiled_regex *preg, + VEC(ada_exc_info) **exceptions) { int i; for (i = 0; i < ARRAY_SIZE (standard_exc); i++) { if (preg == NULL - || regexec (preg, standard_exc[i], 0, NULL, 0) == 0) + || preg->exec (standard_exc[i], 0, NULL, 0) == 0) { struct bound_minimal_symbol msymbol = ada_lookup_simple_minsym (standard_exc[i]); @@ -13253,7 +13254,8 @@ ada_add_standard_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) gets pushed. */ static void -ada_add_exceptions_from_frame (regex_t *preg, struct frame_info *frame, +ada_add_exceptions_from_frame (compiled_regex *preg, + struct frame_info *frame, VEC(ada_exc_info) **exceptions) { const struct block *block = get_frame_block (frame, 0); @@ -13290,10 +13292,10 @@ ada_add_exceptions_from_frame (regex_t *preg, struct frame_info *frame, /* Return true if NAME matches PREG or if PREG is NULL. */ static bool -name_matches_regex (const char *name, regex_t *preg) +name_matches_regex (const char *name, compiled_regex *preg) { return (preg == NULL - || regexec (preg, ada_decode (name), 0, NULL, 0) == 0); + || preg->exec (ada_decode (name), 0, NULL, 0) == 0); } /* Add all exceptions defined globally whose name name match @@ -13316,7 +13318,8 @@ name_matches_regex (const char *name, regex_t *preg) gets pushed. */ static void -ada_add_global_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) +ada_add_global_exceptions (compiled_regex *preg, + VEC(ada_exc_info) **exceptions) { struct objfile *objfile; struct compunit_symtab *s; @@ -13364,7 +13367,7 @@ ada_add_global_exceptions (regex_t *preg, VEC(ada_exc_info) **exceptions) do not match. Otherwise, all exceptions are listed. */ static VEC(ada_exc_info) * -ada_exceptions_list_1 (regex_t *preg) +ada_exceptions_list_1 (compiled_regex *preg) { VEC(ada_exc_info) *result = NULL; struct cleanup *old_chain @@ -13417,19 +13420,11 @@ ada_exceptions_list_1 (regex_t *preg) VEC(ada_exc_info) * ada_exceptions_list (const char *regexp) { - VEC(ada_exc_info) *result = NULL; - struct cleanup *old_chain = NULL; - regex_t reg; - - if (regexp != NULL) - old_chain = compile_rx_or_error (®, regexp, - _("invalid regular expression")); + if (regexp == NULL) + return ada_exceptions_list_1 (NULL); - result = ada_exceptions_list_1 (regexp != NULL ? ® : NULL); - - if (old_chain != NULL) - do_cleanups (old_chain); - return result; + compiled_regex reg (regexp, REG_NOSUB, _("invalid regular expression")); + return ada_exceptions_list_1 (®); } /* Implement the "info exceptions" command. */ diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c index 7731c5e..0074d06 100644 --- a/gdb/break-catch-throw.c +++ b/gdb/break-catch-throw.c @@ -87,10 +87,10 @@ struct exception_catchpoint : public breakpoint char *exception_rx; - /* If non-NULL, an xmalloc'd, compiled regular expression which is - used to determine which exceptions to stop on. */ + /* If non-NULL, a compiled regular expression which is used to + determine which exceptions to stop on. */ - regex_t *pattern; + std::unique_ptr<compiled_regex> pattern; }; \f @@ -145,8 +145,6 @@ classify_exception_breakpoint (struct breakpoint *b) exception_catchpoint::~exception_catchpoint () { xfree (this->exception_rx); - if (this->pattern != NULL) - regfree (this->pattern); } /* Implement the 'check_status' method. */ @@ -185,7 +183,7 @@ check_status_exception_catchpoint (struct bpstats *bs) if (!type_name.empty ()) { - if (regexec (self->pattern, type_name.c_str (), 0, NULL, 0) != 0) + if (self->pattern->exec (type_name.c_str (), 0, NULL, 0) != 0) bs->stop = 0; } } @@ -377,15 +375,12 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx, const char *cond_string, enum exception_event_kind ex_event, int from_tty) { - regex_t *pattern = NULL; + std::unique_ptr<compiled_regex> pattern; if (except_rx != NULL) { - pattern = XNEW (regex_t); - make_cleanup (xfree, pattern); - - compile_rx_or_error (pattern, except_rx, - _("invalid type-matching regexp")); + pattern.reset (new compiled_regex (except_rx, REG_NOSUB, + _("invalid type-matching regexp"))); } std::unique_ptr<exception_catchpoint> cp (new exception_catchpoint ()); @@ -397,7 +392,7 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx, cp->type = bp_breakpoint; cp->kind = ex_event; cp->exception_rx = except_rx; - cp->pattern = pattern; + cp->pattern = std::move (pattern); re_set_exception_catchpoint (cp.get ()); diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index e84d164..053ccef 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -8284,13 +8284,11 @@ struct solib_catchpoint : public breakpoint /* Regular expression to match, if any. COMPILED is only valid when REGEX is non-NULL. */ char *regex; - regex_t compiled; + std::unique_ptr<compiled_regex> compiled; }; solib_catchpoint::~solib_catchpoint () { - if (this->regex) - regfree (&this->compiled); xfree (this->regex); } @@ -8358,7 +8356,7 @@ check_status_catch_solib (struct bpstats *bs) ++ix) { if (!self->regex - || regexec (&self->compiled, iter->so_name, 0, NULL, 0) == 0) + || self->compiled->exec (iter->so_name, 0, NULL, 0) == 0) return; } } @@ -8372,7 +8370,7 @@ check_status_catch_solib (struct bpstats *bs) ++ix) { if (!self->regex - || regexec (&self->compiled, iter, 0, NULL, 0) == 0) + || self->compiled->exec (iter, 0, NULL, 0) == 0) return; } } @@ -8488,16 +8486,8 @@ add_solib_catchpoint (const char *arg, int is_load, int is_temp, int enabled) if (*arg != '\0') { - int errcode; - - errcode = regcomp (&c->compiled, arg, REG_NOSUB); - if (errcode != 0) - { - char *err = get_regcomp_error (errcode, &c->compiled); - - make_cleanup (xfree, err); - error (_("Invalid regexp (%s): %s"), err, arg); - } + c->compiled.reset (new compiled_regex (arg, REG_NOSUB, + _("Invalid regexp"))); c->regex = xstrdup (arg); } diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index 2a5b128..0930342 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -1336,28 +1336,13 @@ show_user (char *args, int from_tty) static void apropos_command (char *searchstr, int from_tty) { - regex_t pattern; - int code; - if (searchstr == NULL) error (_("REGEXP string is empty")); - code = regcomp (&pattern, searchstr, REG_ICASE); - if (code == 0) - { - struct cleanup *cleanups; + compiled_regex pattern (searchstr, REG_ICASE, + _("Error in regular expression")); - cleanups = make_regfree_cleanup (&pattern); - apropos_cmd (gdb_stdout, cmdlist, &pattern, ""); - do_cleanups (cleanups); - } - else - { - char *err = get_regcomp_error (code, &pattern); - - make_cleanup (xfree, err); - error (_("Error in regular expression: %s"), err); - } + apropos_cmd (gdb_stdout, cmdlist, pattern, ""); } /* Subroutine of alias_command to simplify it. diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index d386d02..383adf8 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -917,7 +917,7 @@ add_com_suppress_notification (const char *name, enum command_class theclass, void apropos_cmd (struct ui_file *stream, struct cmd_list_element *commandlist, - struct re_pattern_buffer *regex, const char *prefix) + compiled_regex ®ex, const char *prefix) { struct cmd_list_element *c; int returnvalue; @@ -928,9 +928,10 @@ apropos_cmd (struct ui_file *stream, returnvalue = -1; /* Needed to avoid double printing. */ if (c->name != NULL) { + size_t name_len = strlen (c->name); + /* Try to match against the name. */ - returnvalue = re_search (regex, c->name, strlen(c->name), - 0, strlen (c->name), NULL); + returnvalue = regex.search (c->name, name_len, 0, name_len, NULL); if (returnvalue >= 0) { print_help_for_command (c, prefix, @@ -939,8 +940,10 @@ apropos_cmd (struct ui_file *stream, } if (c->doc != NULL && returnvalue < 0) { + size_t doc_len = strlen (c->doc); + /* Try to match against documentation. */ - if (re_search(regex,c->doc,strlen(c->doc),0,strlen(c->doc),NULL) >=0) + if (regex.search (c->doc, doc_len, 0, doc_len, NULL) >= 0) { print_help_for_command (c, prefix, 0 /* don't recurse */, stream); diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h index 66159fd..11248ba 100644 --- a/gdb/cli/cli-decode.h +++ b/gdb/cli/cli-decode.h @@ -23,8 +23,7 @@ /* Include the public interfaces. */ #include "command.h" - -struct re_pattern_buffer; +#include "gdb_regex.h" #if 0 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum @@ -234,7 +233,7 @@ extern void help_cmd_list (struct cmd_list_element *, enum command_class, extern void help_cmd (const char *, struct ui_file *); extern void apropos_cmd (struct ui_file *, struct cmd_list_element *, - struct re_pattern_buffer *, const char *); + compiled_regex &, const char *); /* Used to mark commands that don't do anything. If we just leave the function field NULL, the command is interpreted as a help topic, or diff --git a/gdb/gdb_regex.c b/gdb/gdb_regex.c new file mode 100644 index 0000000..2e376e3 --- /dev/null +++ b/gdb/gdb_regex.c @@ -0,0 +1,56 @@ +/* Copyright (C) 2011-2017 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +#include "defs.h" +#include "gdb_regex.h" + +compiled_regex::compiled_regex (const char *regex, int cflags, + const char *message) +{ + gdb_assert (regex != NULL); + gdb_assert (message != NULL); + + int code = regcomp (&m_pattern, regex, cflags); + if (code != 0) + { + size_t length = regerror (code, &m_pattern, NULL, 0); + std::unique_ptr<char[]> err (new char[length]); + + regerror (code, &m_pattern, err.get (), length); + error (("%s: %s"), message, err.get ()); + } +} + +compiled_regex::~compiled_regex () +{ + regfree (&m_pattern); +} + +int +compiled_regex::exec (const char *string, size_t nmatch, + regmatch_t pmatch[], int eflags) const +{ + return regexec (&m_pattern, string, nmatch, pmatch, eflags); +} + +int +compiled_regex::search (const char *string, + int length, int start, int range, + struct re_registers *regs) +{ + return re_search (&m_pattern, string, length, start, range, regs); +} diff --git a/gdb/gdb_regex.h b/gdb/gdb_regex.h index 0be26ef..f62f81d 100644 --- a/gdb/gdb_regex.h +++ b/gdb/gdb_regex.h @@ -27,10 +27,39 @@ # include <regex.h> #endif -/* From utils.c. */ -struct cleanup *make_regfree_cleanup (regex_t *); -char *get_regcomp_error (int, regex_t *); -struct cleanup *compile_rx_or_error (regex_t *pattern, const char *rx, - const char *message); +/* A compiled regex. This is mainly a wrapper around regex_t. The + the constructor throws on regcomp error and the destructor is + responsible for calling regfree. The former means that it's not + possible to create an instance of compiled_regex that isn't + compiled, hence the name. */ +class compiled_regex +{ +public: + /* Compile a regexp and throw an exception on error, including + MESSAGE. REGEX and MESSAGE must not be NULL. */ + compiled_regex (const char *regex, int cflags, + const char *message) + ATTRIBUTE_NONNULL (2) ATTRIBUTE_NONNULL (4); + + ~compiled_regex (); + + /* Disable copy. */ + compiled_regex (const compiled_regex&) = delete; + void operator= (const compiled_regex&) = delete; + + /* Wrapper around ::regexec. */ + int exec (const char *string, + size_t nmatch, regmatch_t pmatch[], + int eflags) const; + + /* Wrapper around ::re_search. (Not const because re_search's + regex_t parameter isn't either.) */ + int search (const char *string, int size, int startpos, + int range, struct re_registers *regs); + +private: + /* The compiled pattern. */ + regex_t m_pattern; +}; #endif /* not GDB_REGEX_H */ diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c index 016aadf..1afa8d7 100644 --- a/gdb/linux-tdep.c +++ b/gdb/linux-tdep.c @@ -38,6 +38,7 @@ #include "gdbcmd.h" #include "gdb_regex.h" #include "common/enum-flags.h" +#include "common/gdb_optional.h" #include <ctype.h> @@ -493,6 +494,44 @@ decode_vmflags (char *p, struct smaps_vmflags *v) } } +/* Regexes used by mapping_is_anonymous_p. Put in a structure because + they're initialized lazily. */ + +struct mapping_regexes +{ + /* Matches "/dev/zero" filenames (with or without the "(deleted)" + string in the end). We know for sure, based on the Linux kernel + code, that memory mappings whose associated filename is + "/dev/zero" are guaranteed to be MAP_ANONYMOUS. */ + compiled_regex dev_zero + {"^/dev/zero\\( (deleted)\\)\\?$", REG_NOSUB, + _("Could not compile regex to match /dev/zero filename")}; + + /* Matches "/SYSV%08x" filenames (with or without the "(deleted)" + string in the end). These filenames refer to shared memory + (shmem), and memory mappings associated with them are + MAP_ANONYMOUS as well. */ + compiled_regex shmem_file + {"^/\\?SYSV[0-9a-fA-F]\\{8\\}\\( (deleted)\\)\\?$", REG_NOSUB, + _("Could not compile regex to match shmem filenames")}; + + /* A heuristic we use to try to mimic the Linux kernel's 'n_link == + 0' code, which is responsible to decide if it is dealing with a + 'MAP_SHARED | MAP_ANONYMOUS' mapping. In other words, if + FILE_DELETED matches, it does not necessarily mean that we are + dealing with an anonymous shared mapping. However, there is no + easy way to detect this currently, so this is the best + approximation we have. + + As a result, GDB will dump readonly pages of deleted executables + when using the default value of coredump_filter (0x33), while the + Linux kernel will not dump those pages. But we can live with + that. */ + compiled_regex file_deleted + {" (deleted)$", REG_NOSUB, + _("Could not compile regex to match '<file> (deleted)'")}; +}; + /* Return 1 if the memory mapping is anonymous, 0 otherwise. FILENAME is the name of the file present in the first line of the @@ -506,52 +545,16 @@ decode_vmflags (char *p, struct smaps_vmflags *v) static int mapping_is_anonymous_p (const char *filename) { - static regex_t dev_zero_regex, shmem_file_regex, file_deleted_regex; + static gdb::optional<mapping_regexes> regexes; static int init_regex_p = 0; if (!init_regex_p) { - struct cleanup *c = make_cleanup (null_cleanup, NULL); - /* Let's be pessimistic and assume there will be an error while compiling the regex'es. */ init_regex_p = -1; - /* DEV_ZERO_REGEX matches "/dev/zero" filenames (with or - without the "(deleted)" string in the end). We know for - sure, based on the Linux kernel code, that memory mappings - whose associated filename is "/dev/zero" are guaranteed to be - MAP_ANONYMOUS. */ - compile_rx_or_error (&dev_zero_regex, "^/dev/zero\\( (deleted)\\)\\?$", - _("Could not compile regex to match /dev/zero " - "filename")); - /* SHMEM_FILE_REGEX matches "/SYSV%08x" filenames (with or - without the "(deleted)" string in the end). These filenames - refer to shared memory (shmem), and memory mappings - associated with them are MAP_ANONYMOUS as well. */ - compile_rx_or_error (&shmem_file_regex, - "^/\\?SYSV[0-9a-fA-F]\\{8\\}\\( (deleted)\\)\\?$", - _("Could not compile regex to match shmem " - "filenames")); - /* FILE_DELETED_REGEX is a heuristic we use to try to mimic the - Linux kernel's 'n_link == 0' code, which is responsible to - decide if it is dealing with a 'MAP_SHARED | MAP_ANONYMOUS' - mapping. In other words, if FILE_DELETED_REGEX matches, it - does not necessarily mean that we are dealing with an - anonymous shared mapping. However, there is no easy way to - detect this currently, so this is the best approximation we - have. - - As a result, GDB will dump readonly pages of deleted - executables when using the default value of coredump_filter - (0x33), while the Linux kernel will not dump those pages. - But we can live with that. */ - compile_rx_or_error (&file_deleted_regex, " (deleted)$", - _("Could not compile regex to match " - "'<file> (deleted)'")); - /* We will never release these regexes, so just discard the - cleanups. */ - discard_cleanups (c); + regexes.emplace (); /* If we reached this point, then everything succeeded. */ init_regex_p = 1; @@ -573,9 +576,9 @@ mapping_is_anonymous_p (const char *filename) } if (*filename == '\0' - || regexec (&dev_zero_regex, filename, 0, NULL, 0) == 0 - || regexec (&shmem_file_regex, filename, 0, NULL, 0) == 0 - || regexec (&file_deleted_regex, filename, 0, NULL, 0) == 0) + || regexes->dev_zero.exec (filename, 0, NULL, 0) == 0 + || regexes->shmem_file.exec (filename, 0, NULL, 0) == 0 + || regexes->file_deleted.exec (filename, 0, NULL, 0) == 0) return 1; return 0; diff --git a/gdb/probe.c b/gdb/probe.c index e65e031..cb2dcdb 100644 --- a/gdb/probe.c +++ b/gdb/probe.c @@ -36,6 +36,7 @@ #include "location.h" #include <ctype.h> #include <algorithm> +#include "common/gdb_optional.h" typedef struct bound_probe bound_probe_s; DEF_VEC_O (bound_probe_s); @@ -288,18 +289,17 @@ collect_probes (char *objname, char *provider, char *probe_name, { struct objfile *objfile; VEC (bound_probe_s) *result = NULL; - struct cleanup *cleanup, *cleanup_temps; - regex_t obj_pat, prov_pat, probe_pat; + struct cleanup *cleanup; + gdb::optional<compiled_regex> obj_pat, prov_pat, probe_pat; cleanup = make_cleanup (VEC_cleanup (bound_probe_s), &result); - cleanup_temps = make_cleanup (null_cleanup, NULL); if (provider != NULL) - compile_rx_or_error (&prov_pat, provider, _("Invalid provider regexp")); + prov_pat.emplace (provider, REG_NOSUB, _("Invalid provider regexp")); if (probe_name != NULL) - compile_rx_or_error (&probe_pat, probe_name, _("Invalid probe regexp")); + probe_pat.emplace (probe_name, REG_NOSUB, _("Invalid probe regexp")); if (objname != NULL) - compile_rx_or_error (&obj_pat, objname, _("Invalid object file regexp")); + obj_pat.emplace (objname, REG_NOSUB, _("Invalid object file regexp")); ALL_OBJFILES (objfile) { @@ -312,7 +312,7 @@ collect_probes (char *objname, char *provider, char *probe_name, if (objname) { - if (regexec (&obj_pat, objfile_name (objfile), 0, NULL, 0) != 0) + if (obj_pat->exec (objfile_name (objfile), 0, NULL, 0) != 0) continue; } @@ -326,11 +326,11 @@ collect_probes (char *objname, char *provider, char *probe_name, continue; if (provider - && regexec (&prov_pat, probe->provider, 0, NULL, 0) != 0) + && prov_pat->exec (probe->provider, 0, NULL, 0) != 0) continue; if (probe_name - && regexec (&probe_pat, probe->name, 0, NULL, 0) != 0) + && probe_pat->exec (probe->name, 0, NULL, 0) != 0) continue; bound.objfile = objfile; @@ -339,7 +339,6 @@ collect_probes (char *objname, char *provider, char *probe_name, } } - do_cleanups (cleanup_temps); discard_cleanups (cleanup); return result; } diff --git a/gdb/skip.c b/gdb/skip.c index 4bd8a9e..e767f83 100644 --- a/gdb/skip.c +++ b/gdb/skip.c @@ -34,6 +34,7 @@ #include "filenames.h" #include "fnmatch.h" #include "gdb_regex.h" +#include "common/gdb_optional.h" struct skiplist_entry { @@ -57,10 +58,7 @@ struct skiplist_entry char *function; /* If this is a function regexp, the compiled form. */ - regex_t compiled_function_regexp; - - /* Non-zero if the function regexp has been compiled. */ - int compiled_function_regexp_is_valid; + gdb::optional<compiled_regex> compiled_function_regexp; int enabled; @@ -112,8 +110,6 @@ free_skiplist_entry (struct skiplist_entry *e) { xfree (e->file); xfree (e->function); - if (e->function_is_regexp && e->compiled_function_regexp_is_valid) - regfree (&e->compiled_function_regexp); xfree (e); } @@ -202,7 +198,6 @@ skip_function_command (char *arg, int from_tty) static void compile_skip_regexp (struct skiplist_entry *e, const char *message) { - int code; int flags = REG_NOSUB; #ifdef REG_EXTENDED @@ -210,16 +205,7 @@ compile_skip_regexp (struct skiplist_entry *e, const char *message) #endif gdb_assert (e->function_is_regexp && e->function != NULL); - - code = regcomp (&e->compiled_function_regexp, e->function, flags); - if (code != 0) - { - char *err = get_regcomp_error (code, &e->compiled_function_regexp); - - make_cleanup (xfree, err); - error (_("%s: %s"), message, err); - } - e->compiled_function_regexp_is_valid = 1; + e->compiled_function_regexp.emplace (e->function, flags, message); } /* Process "skip ..." that does not match "skip file" or "skip function". */ @@ -601,8 +587,8 @@ static int skip_rfunction_p (struct skiplist_entry *e, const char *function_name) { gdb_assert (e->function != NULL && e->function_is_regexp - && e->compiled_function_regexp_is_valid); - return (regexec (&e->compiled_function_regexp, function_name, 0, NULL, 0) + && e->compiled_function_regexp); + return (e->compiled_function_regexp->exec (function_name, 0, NULL, 0) == 0); } diff --git a/gdb/symtab.c b/gdb/symtab.c index 22d81fa..f1daf8c 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -62,6 +62,7 @@ #include "parser-defs.h" #include "completer.h" #include "progspace-and-thread.h" +#include "common/gdb_optional.h" /* Forward declarations for local functions. */ @@ -4299,9 +4300,7 @@ search_symbols (const char *regexp, enum search_domain kind, struct symbol_search *found; struct symbol_search *tail; int nfound; - /* This is true if PREG contains valid data, false otherwise. */ - bool preg_p; - regex_t preg; + gdb::optional<compiled_regex> preg; /* OLD_CHAIN .. RETVAL_CHAIN is always freed, RETVAL_CHAIN .. current CLEANUP_CHAIN is freed only in the case of an error. */ @@ -4316,7 +4315,6 @@ search_symbols (const char *regexp, enum search_domain kind, ourtype4 = types4[kind]; *matches = NULL; - preg_p = false; if (regexp != NULL) { @@ -4355,18 +4353,9 @@ search_symbols (const char *regexp, enum search_domain kind, } } - errcode = regcomp (&preg, regexp, - REG_NOSUB | (case_sensitivity == case_sensitive_off - ? REG_ICASE : 0)); - if (errcode != 0) - { - char *err = get_regcomp_error (errcode, &preg); - - make_cleanup (xfree, err); - error (_("Invalid regexp (%s): %s"), err, regexp); - } - preg_p = true; - make_regfree_cleanup (&preg); + int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off + ? REG_ICASE : 0); + preg.emplace (regexp, cflags, _("Invalid regexp")); } /* Search through the partial symtabs *first* for all symbols @@ -4379,8 +4368,8 @@ search_symbols (const char *regexp, enum search_domain kind, }, [&] (const char *symname) { - return (!preg_p || regexec (&preg, symname, - 0, NULL, 0) == 0); + return (!preg || preg->exec (symname, + 0, NULL, 0) == 0); }, NULL, kind); @@ -4415,9 +4404,9 @@ search_symbols (const char *regexp, enum search_domain kind, || MSYMBOL_TYPE (msymbol) == ourtype3 || MSYMBOL_TYPE (msymbol) == ourtype4) { - if (!preg_p - || regexec (&preg, MSYMBOL_NATURAL_NAME (msymbol), 0, - NULL, 0) == 0) + if (!preg + || preg->exec (MSYMBOL_NATURAL_NAME (msymbol), 0, + NULL, 0) == 0) { /* Note: An important side-effect of these lookup functions is to expand the symbol table if msymbol is found, for the @@ -4459,9 +4448,9 @@ search_symbols (const char *regexp, enum search_domain kind, files, nfiles, 1)) && file_matches (symtab_to_fullname (real_symtab), files, nfiles, 0))) - && ((!preg_p - || regexec (&preg, SYMBOL_NATURAL_NAME (sym), 0, - NULL, 0) == 0) + && ((!preg + || preg->exec (SYMBOL_NATURAL_NAME (sym), 0, + NULL, 0) == 0) && ((kind == VARIABLES_DOMAIN && SYMBOL_CLASS (sym) != LOC_TYPEDEF && SYMBOL_CLASS (sym) != LOC_UNRESOLVED @@ -4517,9 +4506,8 @@ search_symbols (const char *regexp, enum search_domain kind, || MSYMBOL_TYPE (msymbol) == ourtype3 || MSYMBOL_TYPE (msymbol) == ourtype4) { - if (!preg_p - || regexec (&preg, MSYMBOL_NATURAL_NAME (msymbol), 0, - NULL, 0) == 0) + if (!preg || preg->exec (MSYMBOL_NATURAL_NAME (msymbol), 0, + NULL, 0) == 0) { /* For functions we can do a quick check of whether the symbol might be found via find_pc_symtab. */ diff --git a/gdb/utils.c b/gdb/utils.c index b4332f8..88a1789 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1038,58 +1038,6 @@ make_hex_string (const gdb_byte *data, size_t length) \f -/* A cleanup function that calls regfree. */ - -static void -do_regfree_cleanup (void *r) -{ - regfree ((regex_t *) r); -} - -/* Create a new cleanup that frees the compiled regular expression R. */ - -struct cleanup * -make_regfree_cleanup (regex_t *r) -{ - return make_cleanup (do_regfree_cleanup, r); -} - -/* Return an xmalloc'd error message resulting from a regular - expression compilation failure. */ - -char * -get_regcomp_error (int code, regex_t *rx) -{ - size_t length = regerror (code, rx, NULL, 0); - char *result = (char *) xmalloc (length); - - regerror (code, rx, result, length); - return result; -} - -/* Compile a regexp and throw an exception on error. This returns a - cleanup to free the resulting pattern on success. RX must not be - NULL. */ - -struct cleanup * -compile_rx_or_error (regex_t *pattern, const char *rx, const char *message) -{ - int code; - - gdb_assert (rx != NULL); - - code = regcomp (pattern, rx, REG_NOSUB); - if (code != 0) - { - char *err = get_regcomp_error (code, pattern); - - make_cleanup (xfree, err); - error (("%s: %s"), message, err); - } - - return make_regfree_cleanup (pattern); -} - /* A cleanup that simply calls ui_unregister_input_event_handler. */ static void -- 2.5.5 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFA 1/2] C++-ify break-catch-sig 2017-06-04 22:54 [RFA 0/2] C++-ify some breakpoint subclasses a bit more Tom Tromey 2017-06-04 22:54 ` [RFA 2/2] C++-ify break-catch-throw Tom Tromey @ 2017-06-04 22:54 ` Tom Tromey 2017-06-05 8:43 ` Simon Marchi 1 sibling, 1 reply; 17+ messages in thread From: Tom Tromey @ 2017-06-04 22:54 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This changes signal_catchpoint to be more of a C++ class, using std::vector and updating the users. gdb/ChangeLog 2017-06-04 Tom Tromey <tom@tromey.com> * break-catch-sig.c (gdb_signal_type): Remove typedef. (struct signal_catchpoint) <signals_to_be_caught>: Now a std::vector. (~signal_catchpoint, signal_catchpoint_insert_location) (signal_catchpoint_remove_location) (signal_catchpoint_breakpoint_hit, signal_catchpoint_print_one) (signal_catchpoint_print_mention) (signal_catchpoint_print_recreate) (signal_catchpoint_explains_signal): Update. (create_signal_catchpoint): Change type of "filter". (catch_signal_split_args): Return a std::vector. (catch_signal_command): Update. --- gdb/ChangeLog | 15 ++++++ gdb/break-catch-sig.c | 127 +++++++++++++++++++------------------------------- 2 files changed, 63 insertions(+), 79 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 7fe5334..b89b47d 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,18 @@ +2017-06-04 Tom Tromey <tom@tromey.com> + + * break-catch-sig.c (gdb_signal_type): Remove typedef. + (struct signal_catchpoint) <signals_to_be_caught>: Now a + std::vector. + (~signal_catchpoint, signal_catchpoint_insert_location) + (signal_catchpoint_remove_location) + (signal_catchpoint_breakpoint_hit, signal_catchpoint_print_one) + (signal_catchpoint_print_mention) + (signal_catchpoint_print_recreate) + (signal_catchpoint_explains_signal): Update. + (create_signal_catchpoint): Change type of "filter". + (catch_signal_split_args): Return a std::vector. + (catch_signal_command): Update. + 2017-06-03 Simon Marchi <simon.marchi@ericsson.com> * x86-linux-nat.c (struct arch_lwp_info): Remove. diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c index 3eede93..8512bd8 100644 --- a/gdb/break-catch-sig.c +++ b/gdb/break-catch-sig.c @@ -33,10 +33,6 @@ #define INTERNAL_SIGNAL(x) ((x) == GDB_SIGNAL_TRAP || (x) == GDB_SIGNAL_INT) -typedef enum gdb_signal gdb_signal_type; - -DEF_VEC_I (gdb_signal_type); - /* An instance of this type is used to represent a signal catchpoint. A breakpoint is really of this type iff its ops pointer points to SIGNAL_CATCHPOINT_OPS. */ @@ -46,14 +42,14 @@ struct signal_catchpoint : public breakpoint ~signal_catchpoint () override; /* Signal numbers used for the 'catch signal' feature. If no signal - has been specified for filtering, its value is NULL. Otherwise, + has been specified for filtering, it is empty. Otherwise, it holds a list of all signals to be caught. */ - VEC (gdb_signal_type) *signals_to_be_caught; + std::vector<gdb_signal> signals_to_be_caught; - /* If SIGNALS_TO_BE_CAUGHT is NULL, then all "ordinary" signals are + /* If SIGNALS_TO_BE_CAUGHT is empty, then all "ordinary" signals are caught. If CATCH_ALL is non-zero, then internal signals are - caught as well. If SIGNALS_TO_BE_CAUGHT is non-NULL, then this + caught as well. If SIGNALS_TO_BE_CAUGHT is not empty, then this field is ignored. */ int catch_all; @@ -89,7 +85,6 @@ signal_to_name_or_int (enum gdb_signal sig) signal_catchpoint::~signal_catchpoint () { - VEC_free (gdb_signal_type, this->signals_to_be_caught); } /* Implement the "insert_location" breakpoint_ops method for signal @@ -99,20 +94,17 @@ static int signal_catchpoint_insert_location (struct bp_location *bl) { struct signal_catchpoint *c = (struct signal_catchpoint *) bl->owner; - int i; - if (c->signals_to_be_caught != NULL) + if (!c->signals_to_be_caught.empty ()) { - gdb_signal_type iter; + gdb_signal iter; - for (i = 0; - VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter); - i++) + for (auto iter : c->signals_to_be_caught) ++signal_catch_counts[iter]; } else { - for (i = 0; i < GDB_SIGNAL_LAST; ++i) + for (int i = 0; i < GDB_SIGNAL_LAST; ++i) { if (c->catch_all || !INTERNAL_SIGNAL (i)) ++signal_catch_counts[i]; @@ -132,15 +124,12 @@ signal_catchpoint_remove_location (struct bp_location *bl, enum remove_bp_reason reason) { struct signal_catchpoint *c = (struct signal_catchpoint *) bl->owner; - int i; - if (c->signals_to_be_caught != NULL) + if (!c->signals_to_be_caught.empty ()) { - gdb_signal_type iter; + gdb_signal iter; - for (i = 0; - VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter); - i++) + for (auto iter : c->signals_to_be_caught) { gdb_assert (signal_catch_counts[iter] > 0); --signal_catch_counts[iter]; @@ -148,7 +137,7 @@ signal_catchpoint_remove_location (struct bp_location *bl, } else { - for (i = 0; i < GDB_SIGNAL_LAST; ++i) + for (int i = 0; i < GDB_SIGNAL_LAST; ++i) { if (c->catch_all || !INTERNAL_SIGNAL (i)) { @@ -174,7 +163,7 @@ signal_catchpoint_breakpoint_hit (const struct bp_location *bl, { const struct signal_catchpoint *c = (const struct signal_catchpoint *) bl->owner; - gdb_signal_type signal_number; + gdb_signal signal_number; if (ws->kind != TARGET_WAITKIND_STOPPED) return 0; @@ -184,18 +173,14 @@ signal_catchpoint_breakpoint_hit (const struct bp_location *bl, /* If we are catching specific signals in this breakpoint, then we must guarantee that the called signal is the same signal we are catching. */ - if (c->signals_to_be_caught) + if (!c->signals_to_be_caught.empty ()) { - int i; - gdb_signal_type iter; + gdb_signal iter; - for (i = 0; - VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter); - i++) + for (auto iter : c->signals_to_be_caught) if (signal_number == iter) return 1; /* Not the same. */ - gdb_assert (!iter); return 0; } else @@ -246,26 +231,25 @@ signal_catchpoint_print_one (struct breakpoint *b, uiout->field_skip ("addr"); annotate_field (5); - if (c->signals_to_be_caught - && VEC_length (gdb_signal_type, c->signals_to_be_caught) > 1) + if (!c->signals_to_be_caught.empty ()) uiout->text ("signals \""); else uiout->text ("signal \""); - if (c->signals_to_be_caught) + if (!c->signals_to_be_caught.empty ()) { - int i; - gdb_signal_type iter; + gdb_signal iter; std::string text; - for (i = 0; - VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter); - i++) + bool first = true; + for (auto iter : c->signals_to_be_caught) { const char *name = signal_to_name_or_int (iter); - if (i > 0) + if (!first) text += " "; + first = false; + text += name; } uiout->field_string ("what", text.c_str ()); @@ -287,19 +271,16 @@ signal_catchpoint_print_mention (struct breakpoint *b) { struct signal_catchpoint *c = (struct signal_catchpoint *) b; - if (c->signals_to_be_caught) + if (!c->signals_to_be_caught.empty ()) { - int i; - gdb_signal_type iter; + gdb_signal iter; - if (VEC_length (gdb_signal_type, c->signals_to_be_caught) > 1) + if (c->signals_to_be_caught.size () > 1) printf_filtered (_("Catchpoint %d (signals"), b->number); else printf_filtered (_("Catchpoint %d (signal"), b->number); - for (i = 0; - VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter); - i++) + for (auto iter : c->signals_to_be_caught) { const char *name = signal_to_name_or_int (iter); @@ -323,14 +304,11 @@ signal_catchpoint_print_recreate (struct breakpoint *b, struct ui_file *fp) fprintf_unfiltered (fp, "catch signal"); - if (c->signals_to_be_caught) + if (!c->signals_to_be_caught.empty ()) { - int i; - gdb_signal_type iter; + gdb_signal iter; - for (i = 0; - VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter); - i++) + for (auto iter : c->signals_to_be_caught) fprintf_unfiltered (fp, " %s", signal_to_name_or_int (iter)); } else if (c->catch_all) @@ -355,7 +333,7 @@ signal_catchpoint_explains_signal (struct breakpoint *b, enum gdb_signal sig) then internal signals like SIGTRAP are not caught. */ static void -create_signal_catchpoint (int tempflag, VEC (gdb_signal_type) *filter, +create_signal_catchpoint (int tempflag, std::vector<gdb_signal> &&filter, int catch_all) { struct signal_catchpoint *c; @@ -363,7 +341,7 @@ create_signal_catchpoint (int tempflag, VEC (gdb_signal_type) *filter, c = new signal_catchpoint (); init_catchpoint (c, gdbarch, tempflag, NULL, &signal_catchpoint_ops); - c->signals_to_be_caught = filter; + c->signals_to_be_caught = std::move (filter); c->catch_all = catch_all; install_breakpoint (0, c, 1); @@ -373,57 +351,50 @@ create_signal_catchpoint (int tempflag, VEC (gdb_signal_type) *filter, /* Splits the argument using space as delimiter. Returns an xmalloc'd filter list, or NULL if no filtering is required. */ -static VEC (gdb_signal_type) * +static std::vector<gdb_signal> catch_signal_split_args (char *arg, int *catch_all) { - VEC (gdb_signal_type) *result = NULL; - struct cleanup *cleanup = make_cleanup (VEC_cleanup (gdb_signal_type), - &result); + std::vector<gdb_signal> result; int first = 1; while (*arg != '\0') { int num; - gdb_signal_type signal_number; - char *one_arg, *endptr; - struct cleanup *inner_cleanup; + gdb_signal signal_number; + char *endptr; - one_arg = extract_arg (&arg); + gdb::unique_xmalloc_ptr<char> one_arg (extract_arg (&arg)); if (one_arg == NULL) break; - inner_cleanup = make_cleanup (xfree, one_arg); /* Check for the special flag "all". */ - if (strcmp (one_arg, "all") == 0) + if (strcmp (one_arg.get (), "all") == 0) { arg = skip_spaces (arg); if (*arg != '\0' || !first) error (_("'all' cannot be caught with other signals")); *catch_all = 1; - gdb_assert (result == NULL); - do_cleanups (inner_cleanup); - discard_cleanups (cleanup); - return NULL; + gdb_assert (result.empty ()); + return result; } first = 0; /* Check if the user provided a signal name or a number. */ - num = (int) strtol (one_arg, &endptr, 0); + num = (int) strtol (one_arg.get (), &endptr, 0); if (*endptr == '\0') signal_number = gdb_signal_from_command (num); else { - signal_number = gdb_signal_from_name (one_arg); + signal_number = gdb_signal_from_name (one_arg.get ()); if (signal_number == GDB_SIGNAL_UNKNOWN) - error (_("Unknown signal name '%s'."), one_arg); + error (_("Unknown signal name '%s'."), one_arg.get ()); } - VEC_safe_push (gdb_signal_type, result, signal_number); - do_cleanups (inner_cleanup); + result.push_back (signal_number); } - discard_cleanups (cleanup); + result.shrink_to_fit (); return result; } @@ -434,7 +405,7 @@ catch_signal_command (char *arg, int from_tty, struct cmd_list_element *command) { int tempflag, catch_all = 0; - VEC (gdb_signal_type) *filter; + std::vector<gdb_signal> filter; tempflag = get_cmd_context (command) == CATCH_TEMPORARY; @@ -448,10 +419,8 @@ catch_signal_command (char *arg, int from_tty, if (arg != NULL) filter = catch_signal_split_args (arg, &catch_all); - else - filter = NULL; - create_signal_catchpoint (tempflag, filter, catch_all); + create_signal_catchpoint (tempflag, std::move (filter), catch_all); } static void -- 2.9.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 1/2] C++-ify break-catch-sig 2017-06-04 22:54 ` [RFA 1/2] C++-ify break-catch-sig Tom Tromey @ 2017-06-05 8:43 ` Simon Marchi 2017-06-05 13:13 ` Tom Tromey 2017-06-05 13:53 ` Tom Tromey 0 siblings, 2 replies; 17+ messages in thread From: Simon Marchi @ 2017-06-05 8:43 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 2017-06-05 00:53, Tom Tromey wrote: > This changes signal_catchpoint to be more of a C++ class, using > std::vector and updating the users. > > gdb/ChangeLog > 2017-06-04 Tom Tromey <tom@tromey.com> > > * break-catch-sig.c (gdb_signal_type): Remove typedef. > (struct signal_catchpoint) <signals_to_be_caught>: Now a > std::vector. > (~signal_catchpoint, signal_catchpoint_insert_location) > (signal_catchpoint_remove_location) > (signal_catchpoint_breakpoint_hit, signal_catchpoint_print_one) > (signal_catchpoint_print_mention) > (signal_catchpoint_print_recreate) > (signal_catchpoint_explains_signal): Update. > (create_signal_catchpoint): Change type of "filter". > (catch_signal_split_args): Return a std::vector. > (catch_signal_command): Update. > --- Hi Tom, Thanks for doing this. The patch looks good in general. I'd suggest doing these in the same patch: - make the catch_all field/parameter/variable a bool, - remove the destructor, since it's now empty. > @@ -99,20 +94,17 @@ static int > signal_catchpoint_insert_location (struct bp_location *bl) > { > struct signal_catchpoint *c = (struct signal_catchpoint *) > bl->owner; > - int i; > > - if (c->signals_to_be_caught != NULL) > + if (!c->signals_to_be_caught.empty ()) > { > - gdb_signal_type iter; > + gdb_signal iter; You don't need this declaration... > - for (i = 0; > - VEC_iterate (gdb_signal_type, c->signals_to_be_caught, i, iter); > - i++) > + for (auto iter : c->signals_to_be_caught) ...because it's overriden by this one. Although I'd prefer if you used "gdb_signal" instead of "auto" here. It's not much longer, is more expressive. There are a few instances of this throughout the patch. > @@ -246,26 +231,25 @@ signal_catchpoint_print_one (struct breakpoint > *b, > uiout->field_skip ("addr"); > annotate_field (5); > > - if (c->signals_to_be_caught > - && VEC_length (gdb_signal_type, c->signals_to_be_caught) > 1) > + if (!c->signals_to_be_caught.empty ()) I think you changed the behavior of this condition. It should probably be: c->signals_to_be_caught.size () > 1 With those comments addressed, the patch is good to me. Thanks! Simon ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 1/2] C++-ify break-catch-sig 2017-06-05 8:43 ` Simon Marchi @ 2017-06-05 13:13 ` Tom Tromey 2017-06-05 13:53 ` Tom Tromey 1 sibling, 0 replies; 17+ messages in thread From: Tom Tromey @ 2017-06-05 13:13 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches Simon> Although I'd prefer if you used "gdb_signal" instead of "auto" Simon> here. It's not much longer, is more expressive. There are a few Simon> instances of this throughout the patch. I made this change. You might want to change the other uses of for/auto already in gdb. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFA 1/2] C++-ify break-catch-sig 2017-06-05 8:43 ` Simon Marchi 2017-06-05 13:13 ` Tom Tromey @ 2017-06-05 13:53 ` Tom Tromey 1 sibling, 0 replies; 17+ messages in thread From: Tom Tromey @ 2017-06-05 13:53 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches >> + gdb_signal iter; Simon> You don't need this declaration... There turned out to be many instances of this. I've fixed them all. Tom ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-06-07 13:40 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-04 22:54 [RFA 0/2] C++-ify some breakpoint subclasses a bit more Tom Tromey 2017-06-04 22:54 ` [RFA 2/2] C++-ify break-catch-throw Tom Tromey 2017-06-05 8:54 ` Simon Marchi 2017-06-05 19:10 ` Tom Tromey 2017-06-05 10:21 ` Pedro Alves 2017-06-05 10:33 ` Pedro Alves 2017-06-05 10:36 ` Pedro Alves 2017-06-05 12:29 ` Pedro Alves 2017-06-05 12:32 ` Pedro Alves 2017-06-05 19:27 ` Tom Tromey 2017-06-06 16:22 ` Pedro Alves 2017-06-07 12:38 ` Tom Tromey 2017-06-07 13:40 ` [pushed] Introduce compiled_regex, eliminate make_regfree_cleanup (Re: [RFA 2/2] C++-ify break-catch-throw) Pedro Alves 2017-06-04 22:54 ` [RFA 1/2] C++-ify break-catch-sig Tom Tromey 2017-06-05 8:43 ` Simon Marchi 2017-06-05 13:13 ` Tom Tromey 2017-06-05 13:53 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).