[RFA 0/2] C++-ify some breakpoint subclasses a bit more

[RFA 0/2] C++-ify some breakpoint subclasses a bit more

[Tom Tromey]

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

[RFA 1/2] C++-ify break-catch-sig

[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

Re: [RFA 1/2] C++-ify break-catch-sig

[Simon Marchi]

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

Re: [RFA 1/2] C++-ify break-catch-sig

[Tom Tromey]

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

Re: [RFA 1/2] C++-ify break-catch-sig

[Tom Tromey]

>> +      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

[RFA 2/2] C++-ify break-catch-throw

[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

Re: [RFA 2/2] C++-ify break-catch-throw

[Simon Marchi]

> --- 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

Re: [RFA 2/2] C++-ify break-catch-throw

[Tom Tromey]

>>>>> "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

Re: [RFA 2/2] C++-ify break-catch-throw

[Pedro Alves]

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

Re: [RFA 2/2] C++-ify break-catch-throw

[Pedro Alves]

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

Re: [RFA 2/2] C++-ify break-catch-throw

[Pedro Alves]

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

Re: [RFA 2/2] C++-ify break-catch-throw

[Pedro Alves]

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 (&reg, regexp,
-				     _("invalid regular expression"));
+  if (regexp == NULL)
+    return ada_exceptions_list_1 (NULL);
 
-  result = ada_exceptions_list_1 (regexp != NULL ? &reg : 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 (&reg);
 }
 
 /* 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

Re: [RFA 2/2] C++-ify break-catch-throw

[Pedro Alves]

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 (&reg, regexp,
-				     _("invalid regular expression"));
+  if (regexp == NULL)
+    return ada_exceptions_list_1 (NULL);
 
-  result = ada_exceptions_list_1 (regexp != NULL ? &reg : 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 (&reg);
 }
 
 /* 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

Re: [RFA 2/2] C++-ify break-catch-throw

[Tom Tromey]

>>>>> "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

Re: [RFA 2/2] C++-ify break-catch-throw

[Pedro Alves]

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 (&reg, regexp,
-				     _("invalid regular expression"));
+  if (regexp == NULL)
+    return ada_exceptions_list_1 (NULL);
 
-  result = ada_exceptions_list_1 (regexp != NULL ? &reg : 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 (&reg);
 }
 
 /* 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 &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..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

Re: [RFA 2/2] C++-ify break-catch-throw

[Tom Tromey]

>>>>> "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

[pushed] Introduce compiled_regex, eliminate make_regfree_cleanup (Re: [RFA 2/2] C++-ify break-catch-throw)

[Pedro Alves]

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 (&reg, regexp,
-				     _("invalid regular expression"));
+  if (regexp == NULL)
+    return ada_exceptions_list_1 (NULL);
 
-  result = ada_exceptions_list_1 (regexp != NULL ? &reg : 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 (&reg);
 }
 
 /* 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 &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..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