[RFA_v4 8/8] Add a self-test for cli-utils.c

[RFA_v4 8/8] Add a self-test for cli-utils.c

[Philippe Waroquiers]

tests added for:
* number_or_range_parser
  In particular, it tests the cur_tok when parsing is finished.

* parse_flags

* parse_flags_qcs

gdb/ChangeLog
2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
	unittests/cli-utils-selftests.c
	* unittests/cli-utils-selftests.c: New file.
---
 gdb/Makefile.in                     |   1 +
 gdb/unittests/cli-utils-selftests.c | 253 ++++++++++++++++++++++++++++
 2 files changed, 254 insertions(+)
 create mode 100644 gdb/unittests/cli-utils-selftests.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 9c0dbbfda5..24852702c4 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -420,6 +420,7 @@ SUBDIR_PYTHON_CFLAGS =
 
 SUBDIR_UNITTESTS_SRCS = \
 	unittests/array-view-selftests.c \
+	unittests/cli-utils-selftests.c \
 	unittests/common-utils-selftests.c \
 	unittests/environ-selftests.c \
 	unittests/format_pieces-selftests.c \
diff --git a/gdb/unittests/cli-utils-selftests.c b/gdb/unittests/cli-utils-selftests.c
new file mode 100644
index 0000000000..b952c7c77f
--- /dev/null
+++ b/gdb/unittests/cli-utils-selftests.c
@@ -0,0 +1,253 @@
+/* Unit tests for the cli-utils.c file.
+
+   Copyright (C) 2018 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 "cli/cli-utils.h"
+#include "selftest.h"
+
+namespace selftests {
+namespace cli_utils {
+
+static void
+test_number_or_range_parser ()
+{
+  /* Test parsing a simple integer.  */
+  {
+    number_or_range_parser one ("1");
+
+    SELF_CHECK (!one.finished ());
+    SELF_CHECK (one.get_number () == 1);
+    SELF_CHECK (one.finished ());
+    SELF_CHECK (strcmp (one.cur_tok (), "") == 0);
+  }
+
+  /* Test parsing an integer followed by a non integer.  */
+  {
+    number_or_range_parser one_after ("1 after");
+
+    SELF_CHECK (!one_after.finished ());
+    SELF_CHECK (one_after.get_number () == 1);
+    SELF_CHECK (one_after.finished ());
+    SELF_CHECK (strcmp (one_after.cur_tok (), "after") == 0);
+  }
+
+  /* Test parsing a range.  */
+  {
+    number_or_range_parser one_three ("1-3");
+
+    for (int i = 1; i < 4; i++)
+      {
+	SELF_CHECK (!one_three.finished ());
+	SELF_CHECK (one_three.get_number () == i);
+      }
+    SELF_CHECK (one_three.finished ());
+    SELF_CHECK (strcmp (one_three.cur_tok (), "") == 0);
+  }
+
+  /* Test parsing a range followed by a non-integer.  */
+  {
+    number_or_range_parser one_three_after ("1-3 after");
+
+    for (int i = 1; i < 4; i++)
+      {
+	SELF_CHECK (!one_three_after.finished ());
+	SELF_CHECK (one_three_after.get_number () == i);
+      }
+    SELF_CHECK (one_three_after.finished ());
+    SELF_CHECK (strcmp (one_three_after.cur_tok (), "after") == 0);
+  }
+
+  /* Test a negative integer gives an error.  */
+  {
+    number_or_range_parser minus_one ("-1");
+
+    SELF_CHECK (!minus_one.finished ());
+    TRY
+      {
+	minus_one.get_number ();
+	SELF_CHECK (false);
+      }
+    CATCH (ex, RETURN_MASK_ERROR)
+      {
+	SELF_CHECK (ex.reason == RETURN_ERROR);
+	SELF_CHECK (ex.error == GENERIC_ERROR);
+	SELF_CHECK (strcmp (ex.message, "negative value") == 0);
+	SELF_CHECK (strcmp (minus_one.cur_tok (), "-1") == 0);
+      }
+    END_CATCH;
+  }
+
+  /* Test that a - followed by not a number does not give an error.  */
+  {
+    number_or_range_parser nan ("-whatever");
+
+    SELF_CHECK (nan.finished ());
+    SELF_CHECK (strcmp (nan.cur_tok (), "-whatever") == 0);
+  }
+}
+
+static void
+test_parse_flags ()
+{
+  const char *flags = "abc";
+  const char *non_flags_args = "non flags args";
+
+  /* Extract twice the same flag, separated by one space.  */
+  {
+    const char *t1 = "-a -a non flags args";
+
+    SELF_CHECK (parse_flags (&t1, flags) == 1);
+    SELF_CHECK (parse_flags (&t1, flags) == 1);
+    SELF_CHECK (strcmp (t1, non_flags_args) == 0);
+  }
+
+  /* Extract some flags, separated by one or more spaces.  */
+  {
+    const char *t2 = "-c     -b -c  -b -c    non flags args";
+
+    SELF_CHECK (parse_flags (&t2, flags) == 3);
+    SELF_CHECK (parse_flags (&t2, flags) == 2);
+    SELF_CHECK (parse_flags (&t2, flags) == 3);
+    SELF_CHECK (parse_flags (&t2, flags) == 2);
+    SELF_CHECK (parse_flags (&t2, flags) == 3);
+    SELF_CHECK (strcmp (t2, non_flags_args) == 0);
+  }
+
+  /* Check behaviour where there is no flag to extract.  */
+  {
+    const char *t3 = non_flags_args;
+
+    SELF_CHECK (parse_flags (&t3, flags) == 0);
+    SELF_CHECK (strcmp (t3, non_flags_args) == 0);
+  }
+
+  /* Extract 2 known flags in front of unknown flags.  */
+  {
+    const char *t4 = "-c -b -x -y -z -c";
+
+    SELF_CHECK (parse_flags (&t4, flags) == 3);
+    SELF_CHECK (parse_flags (&t4, flags) == 2);
+    SELF_CHECK (strcmp (t4, "-x -y -z -c") == 0);
+    SELF_CHECK (parse_flags (&t4, flags) == 0);
+    SELF_CHECK (strcmp (t4, "-x -y -z -c") == 0);
+  }
+
+  /* Check combined flags are not recognised.  */
+  {
+    const char *t5 = "-c -cb -c";
+
+    SELF_CHECK (parse_flags (&t5, flags) == 3);
+    SELF_CHECK (parse_flags (&t5, flags) == 0);
+    SELF_CHECK (strcmp (t5, "-cb -c") == 0);
+  }
+}
+
+static void
+test_parse_flags_qcs ()
+{
+  const char *non_flags_args = "non flags args";
+
+  /* Test parsing of 2 flags out of the known 3.  */
+  {
+    const char *t1 = "-q -s    non flags args";
+    qcs_flags flags;
+
+    SELF_CHECK (parse_flags_qcs ("test_parse_flags_qcs.t1.q",
+				 &t1,
+				 &flags) == 1);
+    SELF_CHECK (flags.quiet && !flags.cont && !flags.silent);
+    SELF_CHECK (parse_flags_qcs ("test_parse_flags_qcs.t1.s",
+				 &t1,
+				 &flags) == 1);
+    SELF_CHECK (flags.quiet && !flags.cont && flags.silent);
+    SELF_CHECK (strcmp (t1, non_flags_args) == 0);
+  }
+
+  /* Test parsing when there is no flag.  */
+  {
+    const char *t2 = "non flags args";
+    qcs_flags flags;
+
+    SELF_CHECK (parse_flags_qcs ("test_parse_flags_qcs.t2",
+				 &t2,
+				 &flags) == 0);
+    SELF_CHECK (!flags.quiet && !flags.cont && !flags.silent);
+    SELF_CHECK (strcmp (t2, non_flags_args) == 0);
+  }
+
+  /* Test parsing stops at a negative integer.  */
+  {
+    const char *t3 = "-123 non flags args";
+    const char *orig_t3 = t3;
+    qcs_flags flags;
+
+    SELF_CHECK (parse_flags_qcs ("test_parse_flags_qcs.t3",
+				 &t3,
+				 &flags) == 0);
+    SELF_CHECK (!flags.quiet && !flags.cont && !flags.silent);
+    SELF_CHECK (strcmp (t3, orig_t3) == 0);
+  }
+
+  /* Test mutual exclusion between -c and -s.  */
+  {
+    const char *t4 = "-c -s non flags args";
+    qcs_flags flags;
+
+    TRY
+      {
+	SELF_CHECK (parse_flags_qcs ("test_parse_flags_qcs.t4.cs",
+				     &t4,
+				     &flags) == 1);
+
+	(void) parse_flags_qcs ("test_parse_flags_qcs.t4.cs",
+				&t4,
+				&flags);
+	SELF_CHECK (false);
+      }
+    CATCH (ex, RETURN_MASK_ERROR)
+      {
+	SELF_CHECK (ex.reason == RETURN_ERROR);
+	SELF_CHECK (ex.error == GENERIC_ERROR);
+	SELF_CHECK
+	  (strcmp (ex.message,
+		   "test_parse_flags_qcs.t4.cs: "
+		   "-c and -s are mutually exclusive") == 0);
+      }
+    END_CATCH;
+  }
+
+}
+
+static void
+test_cli_utils ()
+{
+  selftests::cli_utils::test_number_or_range_parser ();
+  selftests::cli_utils::test_parse_flags ();
+  selftests::cli_utils::test_parse_flags_qcs ();
+}
+
+}
+}
+
+void
+_initialize_cli_utils_selftests ()
+{
+  selftests::register_test ("cli_utils",
+			    selftests::cli_utils::test_cli_utils);
+}
-- 
2.18.0

[RFA_v4 1/8] Add helper functions parse_flags and parse_flags_qcs

[Philippe Waroquiers]

Add helper functions parse_flags and parse_flags_qcs.
parse_flags helper function allows to look for a set of flags at
the start of a string.
A flag must be given individually.

parse_flags_qcs is a specialised helper function to handle
the flags -q, -c and -s, that are used in the new command 'frame apply'
and in the command 'thread apply.

Modify number_or_range_parser::get_number to differentiate a
- followed by digits from a - followed by an alpha (i.e. a flag or an option).
That is needed for the addition of the [FLAG]... arguments to
thread apply ID... [FLAG]... COMMAND

Remove bool number_or_range_parser::m_finished, rather
implement the 'finished' logic inside number_or_range_parser::finished.
The new logic properly detects the end of parsing even if not at
end of the string. This ensures that number_or_range_parser::cur_tok
really points past the last parsed token when parsing is finished.
Before, it was always pointing at the end of the string.
As parsing now is finished directly when not positioned on a number,
number_is_in_list must do an error check before the loop getting all
numbers.

The error message for 'thread apply -$unknownconvvar p 1'
is now the more clear:
  Convenience variable must have integer value.
  Invalid thread ID: -$unknownconvvar p 1
instead of previously:
  negative value

gdb/ChangeLog
2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* cli-utils.c (number_or_range_parser::get_number): Only handle
	numbers or convenience var as numbers.
	(parse_flags): New function.
	(parse_flags_qcs): New function.
	(number_or_range_parser::finished): Ensure parsing end is detected
	before end of string.
	* cli-utils.h (parse_flags): New function.
	(parse_flags_qcs): New function.
	(number_or_range_parser): Remove m_finished bool.
	(number_or_range_parser::skip_range): Set m_in_range to false.

gdb/testsuite/ChangeLog
2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/skip.exp: Update expected error message.
---
 gdb/cli/cli-utils.c             | 96 +++++++++++++++++++++++++++++++--
 gdb/cli/cli-utils.h             | 45 ++++++++++++++--
 gdb/testsuite/gdb.base/skip.exp |  6 +--
 3 files changed, 134 insertions(+), 13 deletions(-)

diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index c55b5035e4..98b7414991 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -137,7 +137,6 @@ number_or_range_parser::number_or_range_parser (const char *string)
 void
 number_or_range_parser::init (const char *string)
 {
-  m_finished = false;
   m_cur_tok = string;
   m_last_retval = 0;
   m_end_value = 0;
@@ -169,7 +168,10 @@ number_or_range_parser::get_number ()
       /* Default case: state->m_cur_tok is pointing either to a solo
 	 number, or to the first number of a range.  */
       m_last_retval = get_number_trailer (&m_cur_tok, '-');
-      if (*m_cur_tok == '-')
+      /* If get_number_trailer has found a -, it might be the start
+	 of a command option.  So, do not parse a range if the - is
+	 followed by an alpha.  */
+      if (*m_cur_tok == '-' && !isalpha (*(m_cur_tok + 1)))
 	{
 	  const char **temp;
 
@@ -196,8 +198,17 @@ number_or_range_parser::get_number ()
 	}
     }
   else
-    error (_("negative value"));
-  m_finished = *m_cur_tok == '\0';
+    {
+      if (isdigit (*(m_cur_tok + 1)))
+	error (_("negative value"));
+      if (*(m_cur_tok + 1) == '$')
+	{
+	  /* Convenience variable.  */
+	  m_last_retval = ::get_number (&m_cur_tok);
+	  if (m_last_retval < 0)
+	    error (_("negative value"));
+	}
+    }
   return m_last_retval;
 }
 
@@ -215,6 +226,21 @@ number_or_range_parser::setup_range (int start_value, int end_value,
   m_end_value = end_value;
 }
 
+/* See documentation in cli-utils.h.  */
+
+bool
+number_or_range_parser::finished () const
+{
+  /* Parsing is finished when at end of string or null string,
+     or we are not in a range and not in front of an integer, negative
+     integer, convenience var or negative convenience var.  */
+  return (m_cur_tok == NULL || *m_cur_tok == '\0'
+	  || (!m_in_range
+	      && !(isdigit (*m_cur_tok) || *m_cur_tok == '$')
+	      && !(*m_cur_tok == '-'
+		   && (isdigit (m_cur_tok[1]) || m_cur_tok[1] == '$'))));
+}
+
 /* Accept a number and a string-form list of numbers such as is 
    accepted by get_number_or_range.  Return TRUE if the number is
    in the list.
@@ -230,12 +256,15 @@ number_is_in_list (const char *list, int number)
     return 1;
 
   number_or_range_parser parser (list);
+
+  if (parser.finished ())
+    error (_("Arguments must be numbers or '$' variables."));
   while (!parser.finished ())
     {
       int gotnum = parser.get_number ();
 
       if (gotnum == 0)
-	error (_("Args must be numbers or '$' variables."));
+	error (_("Arguments must be numbers or '$' variables."));
       if (gotnum == number)
 	return 1;
     }
@@ -304,3 +333,60 @@ check_for_argument (const char **str, const char *arg, int arg_len)
     }
   return 0;
 }
+
+/* See documentation in cli-utils.h.  */
+
+int
+parse_flags (const char **str, const char *flags)
+{
+  const char *p = skip_spaces (*str);
+
+  if (p[0] == '-'
+      && isalpha (p[1])
+      && (p[2] == '\0' || isspace (p[2])))
+    {
+      const char pf = p[1];
+      const char *f = flags;
+
+      while (*f != '\0')
+	{
+	  if (*f == pf)
+	    {
+	      *str = skip_spaces (p + 2);
+	      return f - flags + 1;
+	    }
+	  f++;
+	}
+    }
+
+  return 0;
+}
+
+/* See documentation in cli-utils.h.  */
+
+bool
+parse_flags_qcs (const char *which_command, const char **str,
+		 qcs_flags *flags)
+{
+  switch (parse_flags (str, "qcs"))
+    {
+    case 0:
+      return false;
+    case 1:
+      flags->quiet = true;
+      break;
+    case 2:
+      flags->cont = true;
+      break;
+    case 3:
+      flags->silent = true;
+      break;
+    default:
+      gdb_assert_not_reached ("int qcs flag out of bound");
+    }
+
+  if (flags->cont && flags->silent)
+    error (_("%s: -c and -s are mutually exclusive"), which_command);
+
+  return true;
+}
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index e34ee0df37..fa7d02d719 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -75,8 +75,7 @@ public:
 		    const char *end_ptr);
 
   /* Returns true if parsing has completed.  */
-  bool finished () const
-  { return m_finished; }
+  bool finished () const;
 
   /* Return the string being parsed.  When parsing has finished, this
      points past the last parsed token.  */
@@ -96,6 +95,7 @@ public:
   {
     gdb_assert (m_in_range);
     m_cur_tok = m_end_ptr;
+    m_in_range = false;
   }
 
 private:
@@ -103,9 +103,6 @@ private:
   number_or_range_parser (const number_or_range_parser &);
   number_or_range_parser &operator= (const number_or_range_parser &);
 
-  /* True if parsing has completed.  */
-  bool m_finished;
-
   /* The string being parsed.  When parsing has finished, this points
      past the last parsed token.  */
   const char *m_cur_tok;
@@ -173,4 +170,42 @@ check_for_argument (char **str, const char *arg, int arg_len)
 			     arg, arg_len);
 }
 
+/* A helper function that looks for a set of flags at the start of a
+   string.  The possible flags are given as a null terminated string.
+   A flag in STR must either be at the end of the string,
+   or be followed by whitespace.
+   Returns 0 if no valid flag is found at the start of STR.
+   Otherwise updates *STR, and returns N (which is > 0),
+   such that FLAGS [N - 1] is the valid found flag.  */
+extern int parse_flags (const char **str, const char *flags);
+
+/* qcs_flags struct regroups the flags parsed by parse_flags_qcs.  */
+
+struct qcs_flags
+{
+  bool quiet = false;
+  bool cont = false;
+  bool silent = false;
+};
+
+/* A helper function that uses parse_flags to handle the flags qcs :
+     A flag -q sets FLAGS->QUIET to true.
+     A flag -c sets FLAGS->CONT to true.
+     A flag -s sets FLAGS->SILENT to true.
+
+   The caller is responsible to initialize *FLAGS to false before the (first)
+   call to parse_flags_qcs.
+   parse_flags_qcs can then be called iteratively to search for more
+   valid flags, as part of a 'main parsing loop' searching for -q/-c/-s
+   flags together with other flags and options.
+
+   Returns true and updates *STR and one of FLAGS->QUIET, FLAGS->CONT,
+   FLAGS->SILENT if it finds a valid flag.
+   Returns false if no valid flag is found at the beginning of STR.
+
+   Throws an error if a flag is found such that both FLAGS->CONT and
+   FLAGS->SILENT are true.  */
+
+extern bool parse_flags_qcs (const char *which_command, const char **str,
+			     qcs_flags *flags);
 #endif /* CLI_UTILS_H */
diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp
index 4b556b10a5..223c93d0d9 100644
--- a/gdb/testsuite/gdb.base/skip.exp
+++ b/gdb/testsuite/gdb.base/skip.exp
@@ -69,9 +69,9 @@ gdb_test "skip function baz" "Function baz will be skipped when stepping\."
 gdb_test "skip enable 999" "No skiplist entries found with number 999."
 gdb_test "skip disable 999" "No skiplist entries found with number 999."
 gdb_test "skip delete 999" "No skiplist entries found with number 999."
-gdb_test "skip enable a" "Args must be numbers or '\\$' variables."
-gdb_test "skip disable a" "Args must be numbers or '\\$' variables."
-gdb_test "skip delete a" "Args must be numbers or '\\$' variables."
+gdb_test "skip enable a" "Arguments must be numbers or '\\$' variables."
+gdb_test "skip disable a" "Arguments must be numbers or '\\$' variables."
+gdb_test "skip delete a" "Arguments must be numbers or '\\$' variables."
 
 # Ask for info on a skiplist entry which doesn't exist.
 
-- 
2.18.0

[RFA_v4 2/8] Implement frame apply [all | COUNT | -COUNT | level LEVEL... ] [FLAG]... COMMAND.

[Philippe Waroquiers]

Implement frame apply [all | COUNT | -COUNT | level LEVEL... ] [FLAG]... COMMAND.
Also implement the command 'faas COMMAND', a shortcut for
'frame apply all -s COMMAND'.

The syntax of 'frame apply' to specify some innermost or outermost
frames is similar to 'backtrace' command, using COUNT or -COUNT.

To apply a COMMAND to a more specific set of frames, the following
new command and syntax can be used:
frame apply level LEVEL... [FLAG]... COMMAND
where LEVEL is one or more frame levels or range of frame levels.

The new command 'frame apply' allows to apply a COMMAND to a number of frames,
or to all frames, or to a set of frames.
The optional [FLAG]... arguments allow to control what output to produce
and how to handle errors raised when applying COMMAND to a frame.

Some example usages for this new command:
   frame apply all info frame
      Produce info frame for all frames.
   frame apply all p $sp
      For each frame, print the location, followed by the frame sp.
   frame apply all -q p $sp
      Same as before, but -q flag (q = quiet) indicates to only print
      the frames sp.
   frame apply all p some_local_var_somewhere
      Print some_local_var_somewhere in all frames. 'frame apply'
      will abort as soon as the print command fails.
   frame apply all -c p some_local_var_somewhere
      Same as before, but -c flag (c = continue) means to
      print the error and continue applying command in case the
      print command fails.
   frame apply all -s p some_local_var_somewhere
      Same as before, but -s flag (s = silent) means to
      be silent for frames where the print command fails.
      In other words, this allows to 'search' the frame in which
      some_local_var_somewhere can be printed.
   frame apply all -s -q p some_local_var_somewhere
      Same as before, but does not print the frame info in which
      the variable is found.
   frame apply level 2-4 5 8-10 -s p i = i + 1
      Increments i in the identified frames.

gdb/ChangeLog
2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* stack.c: (trailing_outermost_frame): New function, mostly
	extracted from backtrace_command_1.
	(leading_innermost_frame): New function.
	(backtrace_command_1): Update to call trailing_outermost_frame.
	(frame_apply_command_count): New function.
	(frame_apply_level_command): New function.
	(frame_apply_all_command): New function.
	(frame_apply_command): New function.
	(faas_command): New function.
	(frame_cmd_list): New variable.
	(_initialize_stack): Update to setup the new commands 'frame apply'
	and 'faas'.
---
 gdb/stack.c | 308 ++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 285 insertions(+), 23 deletions(-)

diff --git a/gdb/stack.c b/gdb/stack.c
index 13d8cf22b5..e51c689f43 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1687,6 +1687,58 @@ info_frame_command (const char *addr_exp, int from_tty)
   }
 }
 
+/* Return the innermost frame at level LEVEL.  */
+
+static struct frame_info *
+leading_innermost_frame (int level)
+{
+  struct frame_info *leading;
+
+  leading = get_current_frame ();
+
+  gdb_assert (level >= 0);
+
+  while (leading != nullptr && level)
+    {
+      QUIT;
+      leading = get_prev_frame (leading);
+      level--;
+    }
+
+  return leading;
+}
+
+/* Return the starting frame needed to handle COUNT outermost frames.  */
+
+static struct frame_info *
+trailing_outermost_frame (int count)
+{
+  struct frame_info *current;
+  struct frame_info *trailing;
+
+  trailing = get_current_frame ();
+
+  gdb_assert (count > 0);
+
+  current = trailing;
+  while (current != nullptr && count--)
+    {
+      QUIT;
+      current = get_prev_frame (current);
+    }
+
+  /* Will stop when CURRENT reaches the top of the stack.
+     TRAILING will be COUNT below it.  */
+  while (current != nullptr)
+    {
+      QUIT;
+      trailing = get_prev_frame (trailing);
+      current = get_prev_frame (current);
+    }
+
+  return trailing;
+}
+
 /* Print briefly all stack frames or just the innermost COUNT_EXP
    frames.  */
 
@@ -1751,32 +1803,14 @@ backtrace_command_1 (const char *count_exp, frame_filter_flags flags,
 	 variable TRAILING to the frame from which we should start
 	 printing.  Second, it must set the variable count to the number
 	 of frames which we should print, or -1 if all of them.  */
-      trailing = get_current_frame ();
 
       if (count_exp != NULL && count < 0)
 	{
-	  struct frame_info *current;
-
-	  count = -count;
-
-	  current = trailing;
-	  while (current && count--)
-	    {
-	      QUIT;
-	      current = get_prev_frame (current);
-	    }
-
-	  /* Will stop when CURRENT reaches the top of the stack.
-	     TRAILING will be COUNT below it.  */
-	  while (current)
-	    {
-	      QUIT;
-	      trailing = get_prev_frame (trailing);
-	      current = get_prev_frame (current);
-	    }
-
+	  trailing = trailing_outermost_frame (-count);
 	  count = -1;
 	}
+      else
+	trailing = get_current_frame ();
 
       for (fi = trailing; fi && count--; fi = get_prev_frame (fi))
 	{
@@ -2494,9 +2528,198 @@ func_command (const char *arg, int from_tty)
     }
 }
 
+/* Apply a GDB command to all stack frames, or a set of identified frames,
+   or innermost COUNT frames.
+   With a negative COUNT, apply command on outermost -COUNT frames.
+
+   frame apply 3 info frame     Apply 'info frame' to frames 0, 1, 2
+   frame apply -3 info frame    Apply 'info frame' to outermost 3 frames.
+   frame apply all x/i $pc      Apply 'x/i $pc' cmd to all frames.
+   frame apply all -s p local_var_no_idea_in_which_frame
+                If a frame has a local variable called
+                local_var_no_idea_in_which_frame, print frame
+                and value of local_var_no_idea_in_which_frame.
+   frame apply all -s -q p local_var_no_idea_in_which_frame
+                Same as before, but only print the variable value.
+   frame apply level 2-5 0 4-7 -s p i = i + 1
+                Adds 1 to the variable i in the specified frames.
+                Note that i will be incremented twice in
+                frames 4 and 5.  */
+
+/* Apply a GDB command to COUNT stack frames, starting at TRAILING.
+   CMD starts with 0 or more qcs flags followed by the GDB command to apply.
+   COUNT -1 means all frames starting at TRAILING.  WHICH_COMMAND is used
+   for error messages.  */
+
+static void
+frame_apply_command_count (const char *which_command,
+			   const char *cmd, int from_tty,
+			   struct frame_info *trailing, int count)
+{
+  qcs_flags flags;
+  struct frame_info *fi;
+
+  while (cmd != NULL && parse_flags_qcs (which_command, &cmd, &flags))
+    ;
+
+  if (cmd == NULL || *cmd == '\0')
+    error (_("Please specify a command to apply on the selected frames"));
+
+  /* The below will restore the current inferior/thread/frame.
+     Usually, only the frame is effectively to be restored.
+     But in case CMD switches of inferior/thread, better restore
+     these also.  */
+  scoped_restore_current_thread restore_thread;
+
+  for (fi = trailing; fi && count--; fi = get_prev_frame (fi))
+    {
+      QUIT;
+
+      select_frame (fi);
+      TRY
+	{
+	  std::string cmd_result;
+	  {
+	    /* In case CMD switches of inferior/thread/frame, the below
+	       restores the inferior/thread/frame.  FI can then be
+	       set to the selected frame.  */
+	    scoped_restore_current_thread restore_fi_current_frame;
+
+	    cmd_result = execute_command_to_string (cmd, from_tty);
+	  }
+	  fi = get_selected_frame (_("frame apply "
+				     "unable to get selected frame."));
+	  if (!flags.silent || cmd_result.length () > 0)
+	    {
+	      if (!flags.quiet)
+		print_stack_frame (fi, 1, LOCATION, 0);
+	      printf_filtered ("%s", cmd_result.c_str ());
+	    }
+	}
+      CATCH (ex, RETURN_MASK_ERROR)
+	{
+	  fi = get_selected_frame (_("frame apply "
+				     "unable to get selected frame."));
+	  if (!flags.silent)
+	    {
+	      if (!flags.quiet)
+		print_stack_frame (fi, 1, LOCATION, 0);
+	      if (flags.cont)
+		printf_filtered ("%s\n", ex.message);
+	      else
+		throw_exception (ex);
+	    }
+	}
+      END_CATCH;
+    }
+}
+
+/* Implementation of the "frame apply level" command.  */
+
+static void
+frame_apply_level_command (const char *cmd, int from_tty)
+{
+  if (!target_has_stack)
+    error (_("No stack."));
+
+  bool level_found = false;
+  const char *levels_str = cmd;
+  number_or_range_parser levels (levels_str);
+
+  /* Skip the LEVEL list to find the flags and command args.  */
+  while (!levels.finished ())
+    {
+      const int level_beg = levels.get_number ();
+
+      level_found = true;
+      if (levels.in_range ())
+	levels.skip_range ();
+    }
+
+  if (!level_found)
+    error (_("Missing or invalid LEVEL... argument"));
+
+  cmd = levels.cur_tok ();
+
+  /* Redo the LEVELS parsing, but applying COMMAND.  */
+  levels.init (levels_str);
+  while (!levels.finished ())
+    {
+      const int level_beg = levels.get_number ();
+      int n_frames;
+
+      if (levels.in_range ())
+	{
+	  n_frames = levels.end_value () - level_beg + 1;
+	  levels.skip_range ();
+	}
+      else
+	n_frames = 1;
+
+      frame_apply_command_count ("frame apply level", cmd, from_tty,
+				 leading_innermost_frame (level_beg), n_frames);
+    }
+}
+
+/* Implementation of the "frame apply all" command.  */
+
+static void
+frame_apply_all_command (const char *cmd, int from_tty)
+{
+  if (!target_has_stack)
+    error (_("No stack."));
+
+  frame_apply_command_count ("frame apply all", cmd, from_tty,
+			     get_current_frame (), INT_MAX);
+}
+
+/* Implementation of the "frame apply" command.  */
+
+static void
+frame_apply_command (const char* cmd, int from_tty)
+{
+  int count;
+  struct frame_info *trailing;
+
+  if (!target_has_stack)
+    error (_("No stack."));
+
+  if (cmd == NULL)
+    error (_("Missing COUNT argument."));
+  count = get_number_trailer (&cmd, 0);
+  if (count == 0)
+    error (_("Invalid COUNT argument."));
+
+  if (count < 0)
+    {
+      trailing = trailing_outermost_frame (-count);
+      count = -1;
+    }
+  else
+    trailing = get_current_frame ();
+
+  frame_apply_command_count ("frame apply", cmd, from_tty,
+			     trailing, count);
+}
+
+/* Implementation of the "faas" command.  */
+
+static void
+faas_command (const char *cmd, int from_tty)
+{
+  std::string expanded = std::string ("frame apply all -s ") + cmd;
+  execute_command (expanded.c_str (), from_tty);
+}
+
+
+/* Commands with a prefix of `frame'.  */
+struct cmd_list_element *frame_cmd_list = NULL;
+
 void
 _initialize_stack (void)
 {
+  static struct cmd_list_element *frame_apply_list = NULL;
+
   add_com ("return", class_stack, return_command, _("\
 Make selected stack frame return to its caller.\n\
 Control remains in the debugger, but when you continue\n\
@@ -2519,14 +2742,53 @@ An argument says how many frames down to go."));
 Same as the `down' command, but does not print anything.\n\
 This is useful in command scripts."));
 
-  add_com ("frame", class_stack, frame_command, _("\
+  add_prefix_cmd ("frame", class_stack, frame_command, _("\
 Select and print a stack frame.\nWith no argument, \
 print the selected stack frame.  (See also \"info frame\").\n\
 An argument specifies the frame to select.\n\
-It can be a stack frame number or the address of the frame."));
+It can be a stack frame number or the address of the frame."),
+		  &frame_cmd_list, "frame ", 1, &cmdlist);
 
   add_com_alias ("f", "frame", class_stack, 1);
 
+#define FRAME_APPLY_FLAGS_HELP "\
+Prints the frame location information followed by COMMAND output.\n\
+FLAG arguments are -q (quiet), -c (continue), -s (silent).\n\
+Flag -q disables printing the frame location information.\n\
+By default, if a COMMAND raises an error, frame apply is aborted.\n\
+Flag -c indicates to print the error and continue.\n\
+Flag -s indicates to silently ignore a COMMAND that raises an error\n\
+or produces no output."
+
+  add_prefix_cmd ("apply", class_stack, frame_apply_command,
+		  _("Apply a command to a number of frames.\n\
+Usage: frame apply COUNT [FLAG]... COMMAND\n\
+With a negative COUNT argument, applies the command on outermost -COUNT frames.\n"
+FRAME_APPLY_FLAGS_HELP),
+		  &frame_apply_list, "frame apply ", 1, &frame_cmd_list);
+
+  add_cmd ("all", class_stack, frame_apply_all_command,
+	   _("\
+Apply a command to all frames.\n\
+\n\
+Usage: frame apply all [FLAG]... COMMAND\n"
+FRAME_APPLY_FLAGS_HELP),
+	   &frame_apply_list);
+
+  add_cmd ("level", class_stack, frame_apply_level_command,
+	   _("\
+Apply a command to a list of frames.\n\
+\n\
+Usage: frame apply level LEVEL... [FLAG]... COMMAND\n\
+ID is a space-separated list of LEVELs of frames to apply COMMAND on.\n"
+FRAME_APPLY_FLAGS_HELP),
+	   &frame_apply_list);
+
+  add_com ("faas", class_stack, faas_command, _("\
+Apply a command to all frames (ignoring errors and empty output).\n\
+Usage: faas COMMAND\n\
+shortcut for 'frame apply all -s COMMAND'"));
+
   add_com_suppress_notification ("select-frame", class_stack, select_frame_command, _("\
 Select a stack frame without printing anything.\n\
 An argument specifies the frame to select.\n\
-- 
2.18.0

[RFA_v4 3/8] Add [FLAG]... arguments to 'thread apply'.

[Philippe Waroquiers]

Enhance 'thread apply' command to also accept [FLAG]... arguments.

An example usage for this new argument:
   thread apply all -s frame apply all -s p some_local_var_somewhere
      Prints the thread id, frame location and some_local_var_somewhere
      value in frames of threads that have such local var.

To make the life of the user easier, the most typical use cases
have shortcuts :
   taas  : shortcut for 'thread apply all -s'
   tfaas : shortcut for 'thread apply all -s frame apply all -s"

An example usage :
   tfaas p some_local_var_somewhere
     same as the longer:
        'thread apply all -s frame apply all -s p some_local_var_somewhere'

gdb/ChangeLog
2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* thread.c (thr_try_catch_cmd): New function.
	(thread_apply_all_command): Handle qcs flags.
	(thread_apply_command): Handle qcs flags.
	(taas_command): New function.
	(tfaas_command): New function.
	(_initialize_thread): Update to setup the new commands 'taas
	and 'tfaas'. Change doc string for 'thread apply'.
---
 gdb/thread.c | 133 +++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 109 insertions(+), 24 deletions(-)

diff --git a/gdb/thread.c b/gdb/thread.c
index 517a80715e..5071fdb27f 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1583,6 +1583,44 @@ tp_array_compar (const thread_info *a, const thread_info *b)
     return (a->per_inf_num > b->per_inf_num);
 }
 
+/* Switch to thread THR and execute CMD.
+   FLAGS.QUIET controls the printing of the thread information.
+   FLAGS.CONT and FLAGS.SILENT control how to handle errors.  */
+
+static void
+thr_try_catch_cmd (thread_info *thr, const char *cmd, int from_tty,
+		   const qcs_flags &flags)
+{
+  switch_to_thread (thr);
+  TRY
+    {
+      std::string cmd_result = execute_command_to_string (cmd, from_tty);
+      if (!flags.silent || cmd_result.length () > 0)
+	{
+	  if (!flags.quiet)
+	    printf_filtered (_("\nThread %s (%s):\n"),
+			     print_thread_id (thr),
+			     target_pid_to_str (inferior_ptid));
+	  printf_filtered ("%s", cmd_result.c_str ());
+	}
+    }
+  CATCH (ex, RETURN_MASK_ERROR)
+    {
+      if (!flags.silent)
+	{
+	  if (!flags.quiet)
+	    printf_filtered (_("\nThread %s (%s):\n"),
+			     print_thread_id (thr),
+			     target_pid_to_str (inferior_ptid));
+	  if (flags.cont)
+	    printf_filtered ("%s\n", ex.message);
+	  else
+	    throw_exception (ex);
+	}
+    }
+  END_CATCH;
+}
+
 /* Apply a GDB command to a list of threads.  List syntax is a whitespace
    separated list of numbers, or ranges, or the keyword `all'.  Ranges consist
    of two numbers separated by a hyphen.  Examples:
@@ -1594,16 +1632,27 @@ tp_array_compar (const thread_info *a, const thread_info *b)
 static void
 thread_apply_all_command (const char *cmd, int from_tty)
 {
+  qcs_flags flags;
+
   tp_array_compar_ascending = false;
-  if (cmd != NULL
-      && check_for_argument (&cmd, "-ascending", strlen ("-ascending")))
+
+  while (cmd != NULL)
     {
-      cmd = skip_spaces (cmd);
-      tp_array_compar_ascending = true;
+      if (check_for_argument (&cmd, "-ascending", strlen ("-ascending")))
+	{
+	  cmd = skip_spaces (cmd);
+	  tp_array_compar_ascending = true;
+	  continue;
+	}
+
+      if (parse_flags_qcs ("thread apply all", &cmd, &flags))
+	continue;
+
+      break;
     }
 
   if (cmd == NULL || *cmd == '\000')
-    error (_("Please specify a command following the thread ID list"));
+    error (_("Please specify a command at the end of 'thread apply all'"));
 
   update_thread_list ();
 
@@ -1639,14 +1688,7 @@ thread_apply_all_command (const char *cmd, int from_tty)
 
       for (thread_info *thr : thr_list_cpy)
 	if (thread_alive (thr))
-	  {
-	    switch_to_thread (thr);
-	    printf_filtered (_("\nThread %s (%s):\n"),
-			     print_thread_id (thr),
-			     target_pid_to_str (inferior_ptid));
-
-	    execute_command (cmd, from_tty);
-	  }
+	  thr_try_catch_cmd (thr, cmd, from_tty, flags);
     }
 }
 
@@ -1655,7 +1697,9 @@ thread_apply_all_command (const char *cmd, int from_tty)
 static void
 thread_apply_command (const char *tidlist, int from_tty)
 {
+  qcs_flags flags;
   const char *cmd = NULL;
+  const char *cmd_or_flags;
   tid_range_parser parser;
 
   if (tidlist == NULL || *tidlist == '\000')
@@ -1673,6 +1717,10 @@ thread_apply_command (const char *tidlist, int from_tty)
 	}
     }
 
+  cmd_or_flags = cmd;
+  while (cmd != NULL && parse_flags_qcs ("thread apply", &cmd, &flags))
+    ;
+
   if (cmd == NULL)
     error (_("Please specify a command following the thread ID list"));
 
@@ -1682,7 +1730,7 @@ thread_apply_command (const char *tidlist, int from_tty)
   scoped_restore_current_thread restore_thread;
 
   parser.init (tidlist, current_inferior ()->num);
-  while (!parser.finished () && parser.cur_tok () < cmd)
+  while (!parser.finished () && parser.cur_tok () < cmd_or_flags)
     {
       struct thread_info *tp = NULL;
       struct inferior *inf;
@@ -1727,14 +1775,30 @@ thread_apply_command (const char *tidlist, int from_tty)
 	  continue;
 	}
 
-      switch_to_thread (tp);
-
-      printf_filtered (_("\nThread %s (%s):\n"), print_thread_id (tp),
-		       target_pid_to_str (inferior_ptid));
-      execute_command (cmd, from_tty);
+      thr_try_catch_cmd (tp, cmd, from_tty, flags);
     }
 }
 
+
+/* Implementation of the "taas" command.  */
+
+static void
+taas_command (const char *cmd, int from_tty)
+{
+  std::string expanded = std::string ("thread apply all -s ") + cmd;
+  execute_command (expanded.c_str (), from_tty);
+}
+
+/* Implementation of the "tfaas" command.  */
+
+static void
+tfaas_command (const char *cmd, int from_tty)
+{
+  std::string expanded
+    = std::string ("thread apply all -s frame apply all -s ") + cmd;
+  execute_command (expanded.c_str (), from_tty);
+}
+
 /* Switch to the specified thread, or print the current thread.  */
 
 void
@@ -2032,22 +2096,43 @@ Use this command to switch between threads.\n\
 The new thread ID must be currently known."),
 		  &thread_cmd_list, "thread ", 1, &cmdlist);
 
+#define THREAD_APPLY_FLAGS_HELP "\
+Prints per-inferior thread number and target system's thread id\n\
+followed by COMMAND output.\n\
+FLAG arguments are -q (quiet), -c (continue), -s (silent).\n\
+Flag -q disables printing the thread information.\n\
+By default, if a COMMAND raises an error, thread apply is aborted.\n\
+Flag -c indicates to print the error and continue.\n\
+Flag -s indicates to silently ignore a COMMAND that raises an error\n\
+or produces no output."
+
   add_prefix_cmd ("apply", class_run, thread_apply_command,
 		  _("Apply a command to a list of threads.\n\
-Usage: thread apply ID... COMMAND\n\
-ID is a space-separated list of IDs of threads to apply COMMAND on."),
+Usage: thread apply ID... [FLAG]... COMMAND\n\
+ID is a space-separated list of IDs of threads to apply COMMAND on.\n"
+THREAD_APPLY_FLAGS_HELP),
 		  &thread_apply_list, "thread apply ", 1, &thread_cmd_list);
 
   add_cmd ("all", class_run, thread_apply_all_command,
 	   _("\
 Apply a command to all threads.\n\
 \n\
-Usage: thread apply all [-ascending] COMMAND\n\
+Usage: thread apply all [-ascending] [FLAG]... COMMAND\n\
 -ascending: Call COMMAND for all threads in ascending order.\n\
-            The default is descending order.\
-"),
+            The default is descending order.\n"
+THREAD_APPLY_FLAGS_HELP),
 	   &thread_apply_list);
 
+  add_com ("taas", class_run, taas_command, _("\
+Apply a command to all threads (ignoring errors and empty output).\n\
+Usage: taas COMMAND\n\
+shortcut for 'thread apply all -s COMMAND'"));
+
+  add_com ("tfaas", class_run, tfaas_command, _("\
+Apply a command to all frames of all threads (ignoring errors and empty output).\n\
+Usage: tfaas COMMAND\n\
+shortcut for 'thread apply all -s frame apply all -s COMMAND'"));
+
   add_cmd ("name", class_run, thread_name_command,
 	   _("Set the current thread's name.\n\
 Usage: thread name [NAME]\n\
-- 
2.18.0

[RFA_v4 5/8] Announce the user visible changes for frame/thread apply in NEWS.

[Philippe Waroquiers]

'frame apply', faas, taas, tfaas commands and [FLAG]... arg for thread apply.

gdb/ChangeLog
2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* NEWS: Mention new commands. Mention change to 'thread apply'.
---
 gdb/NEWS | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 2d1d161233..2309e34ac2 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,33 @@
 
 *** Changes since GDB 8.2
 
+* New commands
+
+frame apply [all | COUNT | -COUNT | level LEVEL...] [FLAG]... COMMAND
+  Apply a command to some frames.
+  FLAG arguments allow to control what output to produce and how to handle
+  errors raised when applying COMMAND to a frame.
+
+taas COMMAND
+  Apply a command to all threads (ignoring errors and empty output).
+  Shortcut for 'thread apply all -s COMMAND'.
+
+faas COMMAND
+  Apply a command to all frames (ignoring errors and empty output).
+  Shortcut for 'frame apply all -s COMMAND'.
+
+tfaas COMMAND
+  Apply a command to all frames of all threads (ignoring errors and empty
+  output).
+  Shortcut for 'thread apply all -s frame apply all -s COMMAND'.
+
+* Changed commands
+
+thread apply [all | COUNT | -COUNT] [FLAG]... COMMAND
+  The 'thread apply' command accepts new FLAG arguments.
+  FLAG arguments allow to control what output to produce and how to handle
+  errors raised when applying COMMAND to a thread.
+
 *** Changes in GDB 8.2
 
 * The 'set disassembler-options' command now supports specifying options
-- 
2.18.0

[RFA_v4 7/8] Modify gdb.threads/pthreads.exp to test FLAG qcs arguments for thread apply. Also, add prefixes to make some non unique tests unique.

[Philippe Waroquiers]

gdb/testsuite/ChangeLog
2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.threads/pthreads.exp: Test qcs FLAG arguments.
	Add some test prefixes to make tests unique.
---
 gdb/testsuite/gdb.threads/pthreads.exp | 83 ++++++++++++++++++++++++--
 1 file changed, 79 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/pthreads.exp b/gdb/testsuite/gdb.threads/pthreads.exp
index 830432b833..bbda52e92e 100644
--- a/gdb/testsuite/gdb.threads/pthreads.exp
+++ b/gdb/testsuite/gdb.threads/pthreads.exp
@@ -188,8 +188,10 @@ proc check_control_c {} {
     global gdb_prompt
 
     # Verify that all threads are running.
-    if [all_threads_running] then {
-	pass "all threads running after startup"
+    with_test_prefix "after startup" {
+	if [all_threads_running] then {
+	    pass "all threads running after startup"
+	}
     }
 
     # Send a continue followed by ^C to the process to stop it.
@@ -216,8 +218,10 @@ proc check_control_c {} {
     gdb_test "bt" ".*"
 
     # Verify that all threads can be run again after a ^C stop.
-    if [all_threads_running] then {
-	pass "all threads running after continuing from ^C stop"
+    with_test_prefix "after continue" {
+	if [all_threads_running] then {
+	    pass "all threads running after continuing from ^C stop"
+	}
     }
     return 0
 }
@@ -267,6 +271,76 @@ proc check_backtraces {} {
     }
 }
 
+proc check_qcs {} {
+    set any "\[^\r\n\]*"
+    set ws "\[ \t\]\+"
+    set number "\[0-9]\+"
+
+    # Check -c (continue) and -s (silently continue) flags.
+    gdb_test "thread apply 2-3 p notfound" \
+	[multi_line \
+	     "" \
+	     "Thread 2 ${any}" \
+	     "No symbol \\\"notfound\\\" in current context." \
+	    ] \
+	"run a failing command that aborts thread apply"
+
+    gdb_test "thread apply 2-3 -c p notfound" \
+	[multi_line \
+	     "" \
+	     "Thread 2 ${any}" \
+	     "No symbol \\\"notfound\\\" in current context." \
+	     "" \
+	     "Thread 3 ${any}" \
+	     "No symbol \\\"notfound\\\" in current context." \
+	    ] \
+	"run a failing command, -c to continue"
+
+    with_test_prefix "silent flag" {
+	foreach_with_prefix cmd_and_args {
+	    "thread apply all -s frame apply all -s"
+	    "tfaas"
+	    "taas faas"} {
+		set cmd "$cmd_and_args p i"
+		gdb_test $cmd \
+		    [multi_line \
+			 "" \
+			 "Thread 3 ${any}" \
+			 "#${number}${ws}${any} in thread2 ${any}" \
+			 "\\\$\[0-9]+ = ${number}${any}" \
+			 "" \
+			 "Thread 2 ${any}" \
+			 "#${number}${ws}${any} in thread1 ${any}" \
+			 "\\\$\[0-9]+ = ${number}${any}" \
+			] \
+		    "run a failing command except in one frame of thread 2,3, -s to silently continue"
+	    }
+    }
+
+    # Check quietness.
+    gdb_test "thread apply all -s -q frame apply all -s p i" \
+	[multi_line \
+	     "#${number}${ws}${any} in thread2 ${any}" \
+	     "\\\$\[0-9]+ = ${number}${any}" \
+	     "#${number}${ws}${any} in thread1 ${any}" \
+	     "\\\$\[0-9]+ = ${number}${any}" \
+	    ] \
+	"run a failing command except in one frame of thread 2,3, -s to silently continue.  Do not show thread information"
+
+    gdb_test "thread apply all -s -q frame apply all -s -q p i" \
+	[multi_line \
+	     "\\\$\[0-9]+ = ${number}${any}" \
+	     "\\\$\[0-9]+ = ${number}${any}" \
+	    ] \
+	"run a failing command except in one frame of thread 2,3, -s to silently continue.  Do not show thread and frame info"
+
+    # Check invalid flag combinations.
+    gdb_test "thread apply all -c -s p 1" \
+	"thread apply all: -c and -s are mutually exclusive" \
+	"check -c and -s cannot be used simultaneously"
+
+}
+
 if [runto_main] then {
     if [test_startup] then {
 	if [check_control_c] then {
@@ -274,5 +348,6 @@ if [runto_main] then {
 	    return
 	}
 	check_backtraces
+	check_qcs
     }
 }
-- 
2.18.0

[RFA_v4 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'

[Philippe Waroquiers]

This is the fifth iteration of the patch series that:
 * implements a new command
     'frame apply [all | COUNT | -COUNT | level LEVEL...] [FLAG]... COMMAND'.
 * enhance 'thread apply COMMAND' by adding FLAG arguments.
 * adds some shortcuts commands.
 * documents the above in gdb.texinfo and NEWS.
 * adds a unit test for cli-utils.c.
 * adds test for 'frame apply'.
 * modify gdb.threads/pthreads.exp to test 'thread apply' FLAG arguments.

The fifth version is RFA v4. It handles the third set of comments
given by Pedro.
The main changes compared to RFA v3 are:
  * frame level LEVEL...  replaces   frame id ID...

The fourth version is RFA v3. It handles the comments given by Pedro.
The main changes compared to RFA v2 are:
  * The -q (quiet) flag replaces the verbosity/-v/-q concept.
  * Addition of 'frame apply level LEVEL... [FLAG]... COMMAND' to allow
    applying a command on a selected list of frames.
  * [FLAG]... arguments are now parsed iteratively.
  * Documentation and on-line help updated accordingly.
  * tests updated accordingly.

The third version was RFA v2, handling the comments of Pedro about giving
the flags individually.
The changes compared to RFA v1 is:
  * The flags must be given individually, such as -v -v -c.
  * The documentation and on-line help was updated accordingly.
  * ChangeLog information is distributed in each commit log message.

The second version was RFA v1, changes compared to RFC are:
  * Replied to all comments of Eli and Simon.
  * Tests and ChangeLog entries added.
  * Comments received from Pedro

The first version was an RFC
  * Code and doc was complete, but was lacking ChangeLog and tests.
  * Comments received from Eli and Simon.

[RFA_v4 6/8] Add a test for 'frame apply'

[Philippe Waroquiers]

Add a test for 'frame apply'

gdb/testsuite/ChangeLog
2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/frameapply.c: New file.
	* gdb.base/frameapply.exp: New file.
---
 gdb/testsuite/gdb.base/frameapply.c   |  71 +++++++++
 gdb/testsuite/gdb.base/frameapply.exp | 217 ++++++++++++++++++++++++++
 2 files changed, 288 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/frameapply.c
 create mode 100644 gdb/testsuite/gdb.base/frameapply.exp

diff --git a/gdb/testsuite/gdb.base/frameapply.c b/gdb/testsuite/gdb.base/frameapply.c
new file mode 100644
index 0000000000..dccf4031ed
--- /dev/null
+++ b/gdb/testsuite/gdb.base/frameapply.c
@@ -0,0 +1,71 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2018 Free Software Foundation, Inc.
+
+   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/>.  */
+
+static void
+setup_done (void)
+{
+}
+
+static int
+f1 (int f1arg)
+{
+  int f1loc;
+
+  f1loc = f1arg - 1;
+
+  setup_done ();
+  return f1loc;
+}
+
+static int
+f2 (int f2arg)
+{
+  int f2loc;
+
+  f2loc = f1 (f2arg - 1);
+
+  return f2loc;
+}
+
+static int
+f3 (int f3arg)
+{
+  int f3loc;
+
+  f3loc = f2 (f3arg - 1);
+
+  return f3loc;
+}
+
+static int
+f4 (int f4arg)
+{
+  int f4loc;
+
+  f4loc = f3 (f4arg - 1);
+
+  return f4loc;
+}
+
+int
+main (void)
+{
+  int result;
+
+  result = f4 (4);
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/frameapply.exp b/gdb/testsuite/gdb.base/frameapply.exp
new file mode 100644
index 0000000000..9f340e5615
--- /dev/null
+++ b/gdb/testsuite/gdb.base/frameapply.exp
@@ -0,0 +1,217 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2018 Free Software Foundation, Inc.
+
+# 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/>.
+
+
+# Test 'frame apply [all | COUNT | -COUNT | level LEVEL...] [FLAG]... COMMAND'.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile}] } {
+    return -1
+}
+
+clean_restart ${binfile}
+
+
+if ![runto setup_done] then {
+    fail "can't run to setup_done"
+    return 0
+}
+
+set any "\[^\r\n\]*"
+set ws "\[ \t\]\+"
+set number "\[0-9]\+"
+
+
+# Check all | COUNT | -COUNT | level LEVEL... with a simple command.
+with_test_prefix "simple command" {
+    foreach_with_prefix frame_apply_args {
+	"all"
+	"6"
+	"-6"
+	"level 0-5"
+	"level 0-3 4-5"
+	"level 0 1-2 3-5"
+	"level 0 1 2 3 4 5"
+	"level 0-0 1-1 2-2 3-3 4-4 5-5" } {
+	    set cmd "frame apply $frame_apply_args p /x 20"
+	    gdb_test $cmd \
+		[multi_line \
+		     "#0${ws}setup_done ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		     "#1${ws}${any} f1 ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		     "#2${ws}${any} f2 ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		     "#3${ws}${any} f3 ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		     "#4${ws}${any} f4 ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		     "#5${ws}${any} main ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		    ] \
+		"run a simple command on all frames"
+	}
+}
+
+# Check frame apply on 3 innermost frames.
+with_test_prefix "innermost 3" {
+    foreach_with_prefix frame_apply_args {
+	"3"
+	"level 0-2" } {
+	    set cmd "frame apply $frame_apply_args p /x 20"
+	    gdb_test $cmd \
+		[multi_line \
+		     "#0${ws}setup_done ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		     "#1${ws}${any} f1 ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		     "#2${ws}${any} f2 ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		    ] \
+		"run a simple command on the 3 innermost frames"
+	}
+}
+
+# Check frame apply on 3 outermost frames.
+with_test_prefix "outermost 3" {
+    foreach_with_prefix frame_apply_args {
+	"-3" } {
+	    set cmd "frame apply $frame_apply_args p /x 20"
+	    gdb_test $cmd \
+		[multi_line \
+		     "#3${ws}${any} f3 ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		     "#4${ws}${any} f4 ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		     "#5${ws}${any} main ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		    ] \
+		"run a simple command on the 3 outermost frames"
+	}
+}
+
+# Check -c (continue) and -s (silently continue) flags.
+with_test_prefix "!cont !silent flags" {
+    foreach_with_prefix frame_apply_args {
+	"all"
+	"level 0-5"
+    } {
+	set cmd "frame apply $frame_apply_args p f3arg"
+	gdb_test $cmd \
+	    [multi_line \
+		 "#0${ws}setup_done ${any}" \
+		 "No symbol \\\"f3arg\\\" in current context." \
+		] \
+	    "run a failing command that aborts frame apply"
+    }
+}
+
+with_test_prefix "cont !silent flags" {
+    foreach_with_prefix frame_apply_args {
+	"all -c"
+	"level 0-5 -c"} {
+	    set cmd "frame apply $frame_apply_args p f3arg"
+	    gdb_test $cmd \
+		[multi_line \
+		     "#0${ws}setup_done ${any}" \
+		     "No symbol \\\"f3arg\\\" in current context." \
+		     "#1${ws}${any} f1 ${any}" \
+		     "No symbol \\\"f3arg\\\" in current context." \
+		     "#2${ws}${any} f2 ${any}" \
+		     "No symbol \\\"f3arg\\\" in current context." \
+		     "#3${ws}${any} f3 ${any}" \
+		     "\\\$\[0-9]+ = 3${any}" \
+		     "#4${ws}${any} f4 ${any}" \
+		     "No symbol \\\"f3arg\\\" in current context." \
+		     "#5${ws}${any} main ${any}" \
+		     "No symbol \\\"f3arg\\\" in current context." \
+		    ] \
+		"run a command failing in all frames except #3, -c to continue"
+	}
+}
+
+with_test_prefix "!cont silent flags" {
+    foreach_with_prefix cmd_and_args {
+	"frame apply all -s"
+	"faas"
+	"frame apply level 0-5 -s"} {
+	    set cmd "$cmd_and_args p f3arg"
+	    gdb_test $cmd \
+		[multi_line \
+		     "#3${ws}${any} f3 ${any}" \
+		     "\\\$\[0-9]+ = 3${any}" \
+		    ] \
+		"run a command failing in all frames except #3, -s to silently continue"
+	}
+}
+
+# Check quietness.
+with_test_prefix "!quiet flag" {
+    foreach_with_prefix frame_apply_args {
+	"2"
+	"level 0-1"} {
+	    set cmd "frame apply $frame_apply_args p /x 20"
+	    gdb_test $cmd \
+		[multi_line \
+		     "#0${ws}setup_done ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		     "#1${ws}${any} f1 ${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		    ] \
+		"run a command, printing location"
+	}
+}
+
+with_test_prefix "quiet flag" {
+    foreach_with_prefix frame_apply_args {
+	"2 -q"
+	"level 0-1 -q"} {
+	    set cmd "frame apply $frame_apply_args p /x 20"
+	    gdb_test $cmd \
+		[multi_line \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		     "\\\$\[0-9]+ = 0x14${any}" \
+		    ] \
+		"run a command with -q quiet flag, printing only command results"
+	}
+}
+
+# Check multiple flags together.
+with_test_prefix "quiet silent flags" {
+    foreach_with_prefix frame_apply_args {
+	"all -q -s -q"
+	"level 0-5 -q -s -q"} {
+	    set cmd "frame apply $frame_apply_args p f3arg"
+	    gdb_test $cmd \
+		"\\\$\[0-9]+ = 3${any}" \
+		"run a command failing in all frames except #3, -s to silently continue, quiet"
+	}
+}
+
+# Check invalid flag combinations.
+gdb_test "frame apply all -c -s p f3arg" \
+    "frame apply all: -c and -s are mutually exclusive" \
+    "check -c and -s cannot be used simultaneously"
+
+# Check some cases of missing or wrong args.
+gdb_test "frame apply" "Missing COUNT argument." "missing COUNT"
+gdb_test "frame apply -c" "Invalid COUNT argument." "invalid COUNT arg"
+gdb_test "frame apply level 4-2 p 1" "inverted range" "inverted range"
+gdb_test "frame apply level 0-3" \
+    "Please specify a command to apply on the selected frames" \
+    "missing command"
-- 
2.18.0

[RFA_v4 4/8] Documents the new commands 'frame apply', faas, taas, tfaas

[Philippe Waroquiers]

Documents the new commands 'frame apply', faas, taas, tfaas.
Documents the new arguments [FLAG]... added to 'thread apply'.

gdb/doc/ChangeLog
2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.texinfo (Debugging Programs with Multiple Threads):
	Document changes to 'thread apply'. Document 'taas'.
	Document 'tfaas'.
	(Examining the Stack): Document 'frame apply'. Document 'faas'.
---
 gdb/doc/gdb.texinfo | 188 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 186 insertions(+), 2 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 245d3f1b97..a49ef6cda0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2936,7 +2936,7 @@ programs:
 @item automatic notification of new threads
 @item @samp{thread @var{thread-id}}, a command to switch among threads
 @item @samp{info threads}, a command to inquire about existing threads
-@item @samp{thread apply [@var{thread-id-list}] [@var{all}] @var{args}},
+@item @samp{thread apply [@var{thread-id-list} | all] @var{args}},
 a command to apply a command to a list of threads
 @item thread-specific breakpoints
 @item @samp{set print thread-events}, which controls printing of 
@@ -3173,7 +3173,7 @@ threads.
 
 @kindex thread apply
 @cindex apply command to several threads
-@item thread apply [@var{thread-id-list} | all [-ascending]] @var{command}
+@item thread apply [@var{thread-id-list} | all [-ascending]] [@var{flag}]@dots{} @var{command}
 The @code{thread apply} command allows you to apply the named
 @var{command} to one or more threads.  Specify the threads that you
 want affected using the thread ID list syntax (@pxref{thread ID
@@ -3182,6 +3182,59 @@ command to all threads in descending order, type @kbd{thread apply all
 @var{command}}.  To apply a command to all threads in ascending order,
 type @kbd{thread apply all -ascending @var{command}}.
 
+The @var{flag} arguments control what output to produce and how to handle
+errors raised when applying @var{command} to a thread.  @var{flag}
+must start with a @code{-} directly followed by one letter in
+@code{qcs}.  If several flags are provided, they must be given
+individually, such as @code{-c -q}.
+
+By default, @value{GDBN} displays some thread information before the
+output produced by @var{command}, and an error raised during the
+execution of a @var{command} will abort @code{thread apply}.  The
+following flags can be used to fine-tune this behavior:
+
+@table @code
+@item -c
+The flag @code{-c}, which stands for @samp{continue}, causes any
+errors in @var{command} to be displayed, and the execution of
+@code{thread apply} then continues.
+@item -s
+The flag @code{-s}, which stands for @samp{silent}, causes any errors
+or empty output produced by a @var{command} to be silently ignored.
+That is, the execution continues, but the thread information and errors
+are not printed.
+@item -q
+The flag @code{-q} (@samp{quiet}) disables printing the thread
+information.
+@end table
+
+Flags @code{-c} and @code{-s} cannot be used together.
+
+@kindex taas
+@cindex apply command to all threads (ignoring errors and empty output)
+@item taas @var{command}
+Shortcut for @code{thread apply all -s @var{command}}.
+Applies @var{command} on all threads, ignoring errors and empty output.
+
+@kindex tfaas
+@cindex apply a command to all frames of all threads (ignoring errors and empty output)
+@item tfaas @var{command}
+Shortcut for @code{thread apply all -s frame apply all -s @var{command}}.
+Applies @var{command} on all frames of all threads, ignoring errors
+and empty output.  Note that the flag @code{-s} is specified twice:
+The first @code{-s} ensures that @code{thread apply} only shows the thread
+information of the threads for which @code{frame apply} produces
+some output.  The second @code{-s} is needed to ensure that @code{frame
+apply} shows the frame information of a frame only if the
+@var{command} successfully produced some output.
+
+It can for example be used to print a local variable or a function
+argument without knowing the thread or frame where this variable or argument
+is, using:
+@smallexample
+(@value{GDBP}) tfaas p some_local_var_i_do_not_remember_where_it_is
+@end smallexample
+
 
 @kindex thread name
 @cindex name a thread
@@ -7304,6 +7357,7 @@ currently executing frame and describes it briefly, similar to the
 * Backtrace::                   Backtraces
 * Selection::                   Selecting a frame
 * Frame Info::                  Information on a frame
+* Frame Apply::                 Applying a command to several frames
 * Frame Filter Management::     Managing frame filters
 
 @end menu
@@ -7716,6 +7770,136 @@ accessible at the point of execution of the selected frame.
 
 @end table
 
+@node Frame Apply
+@section Applying a Command to Several Frames.
+@kindex frame apply
+@cindex apply command to several frames
+@table @code
+@item frame apply [all | @var{count} | @var{-count} | level @var{level}@dots{}] [@var{flag}]@dots{} @var{command}
+The @code{frame apply} command allows you to apply the named
+@var{command} to one or more frames.
+
+@table @code
+@item @code{all}
+Specify @code{all} to apply @var{command} to all frames.
+
+@item @var{count}
+Use @var{count} to apply @var{command} to the innermost @var{count}
+frames, where @var{count} is a positive number.
+
+@item @var{-count}
+Use @var{-count} to apply @var{command} to the outermost @var{count}
+frames, where @var{count} is a positive number.
+
+@item @code{level}
+Use @code{level} to apply @var{command} to the set of frames identified
+by the @var{level} list.  @var{level} is a frame level or a range of frame
+levels as @var{level1}-@var{level2}.  The frame level is the number shown
+in the first field of the @samp{backtrace} command output.
+E.g., @samp{2-4 6-8 3} indicates to apply @var{command} for the frames
+at levels 2, 3, 4, 6, 7, 8, and then again on frame at level 3.
+
+@end table
+
+@end table
+
+Note that the frames on which @code{frame apply} applies a command are
+also influenced by the @code{set backtrace} settings such as @code{set
+backtrace past-main} and @code{set backtrace limit N}.  See
+@xref{Backtrace,,Backtraces}.
+
+The @var{flag} arguments control what output to produce and how to handle
+errors raised when applying @var{command} to a frame.  @var{flag}
+must start with a @code{-} directly followed by one letter in
+@code{qcs}.  If several flags are provided, they must be given
+individually, such as @code{-c -q}.
+
+By default, @value{GDBN} displays some frame information before the
+output produced by @var{command}, and an error raised during the
+execution of a @var{command} will abort @code{frame apply}.  The
+following flags can be used to fine-tune this behavior:
+
+@table @code
+@item -c
+The flag @code{-c}, which stands for @samp{continue}, causes any
+errors in @var{command} to be displayed, and the execution of
+@code{frame apply} then continues.
+@item -s
+The flag @code{-s}, which stands for @samp{silent}, causes any errors
+or empty output produced by a @var{command} to be silently ignored.
+That is, the execution continues, but the frame information and errors
+are not printed.
+@item -q
+The flag @code{-q} (@samp{quiet}) disables printing the frame
+information.
+@end table
+
+The following example shows how the flags @code{-c} and @code{-s} are
+working when applying the command @code{p j} to all frames, where
+variable @code{j} can only be successfully printed in the outermost
+@code{#1 main} frame.
+
+@smallexample
+@group
+(gdb) frame apply all p j
+#0  some_function (i=5) at fun.c:4
+No symbol "j" in current context.
+(gdb) frame apply all -c p j
+#0  some_function (i=5) at fun.c:4
+No symbol "j" in current context.
+#1  0x565555fb in main (argc=1, argv=0xffffd2c4) at fun.c:11
+$1 = 5
+(gdb) frame apply all -s p j
+#1  0x565555fb in main (argc=1, argv=0xffffd2c4) at fun.c:11
+$2 = 5
+(gdb)
+@end group
+@end smallexample
+
+By default, @samp{frame apply}, prints the frame location
+information before the command output:
+
+@smallexample
+@group
+(gdb) frame apply all p $sp
+#0  some_function (i=5) at fun.c:4
+$4 = (void *) 0xffffd1e0
+#1  0x565555fb in main (argc=1, argv=0xffffd2c4) at fun.c:11
+$5 = (void *) 0xffffd1f0
+(gdb)
+@end group
+@end smallexample
+
+If flag @code{-q} is given, no frame information is printed:
+@smallexample
+@group
+(gdb) frame apply all -q p $sp
+$12 = (void *) 0xffffd1e0
+$13 = (void *) 0xffffd1f0
+(gdb)
+@end group
+@end smallexample
+
+@table @code
+
+@kindex faas
+@cindex apply a command to all frames (ignoring errors and empty output)
+@item faas @var{command}
+Shortcut for @code{frame apply all -s @var{command}}.
+Applies @var{command} on all frames, ignoring errors and empty output.
+
+It can for example be used to print a local variable or a function
+argument without knowing the frame where this variable or argument
+is, using:
+@smallexample
+(@value{GDBP}) faas p some_local_var_i_do_not_remember_where_it_is
+@end smallexample
+
+Note that the command @code{tfaas @var{command}} applies @var{command}
+on all frames of all threads.  See @xref{Threads,,Threads}.
+@end table
+
+
 @node Frame Filter Management
 @section Management of Frame Filters.
 @cindex managing frame filters
-- 
2.18.0

Re: [RFA_v4 4/8] Documents the new commands 'frame apply', faas, taas, tfaas

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Tue, 10 Jul 2018 23:39:22 +0200
> 
> gdb/doc/ChangeLog
> 2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* gdb.texinfo (Debugging Programs with Multiple Threads):
> 	Document changes to 'thread apply'. Document 'taas'.
                                          ^^
Two spaces between sentences, please.

I believe I already approved the rest.  Thanks.

Re: [RFA_v4 4/8] Documents the new commands 'frame apply', faas, taas, tfaas

[Philippe Waroquiers]

On Wed, 2018-07-11 at 06:06 +0300, Eli Zaretskii wrote:
> > From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> > Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> > Date: Tue, 10 Jul 2018 23:39:22 +0200
> > 
> > gdb/doc/ChangeLog
> > 2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> > 
> > 	* gdb.texinfo (Debugging Programs with Multiple Threads):
> > 	Document changes to 'thread apply'. Document 'taas'.
> 
>                                           ^^
> Two spaces between sentences, please.
Thanks, fixed.
(I am now running a bunch of grep on git diff to help finding
the above errors, but I need also to grep the commit
messages).

> 
> I believe I already approved the rest.  Thanks.

Yes, doc/NEWS/on-line help were previously approved on RFA_v2.

Between RFA_v2 and this RFA_v4, some non minor changes were done
e.g. to the new FLAG arguments of 'frame apply' and 'thread apply',
but I saw no explicit re-approval of NEWS and on-line help doc
strings.
(I assume this mail re-approves the doc part RFA_v4, but in any
case a re-review of the code is needed before push of the series).

Thanks

Philippe

Re: [RFA_v4 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'

[Pedro Alves]

On 07/10/2018 10:39 PM, Philippe Waroquiers wrote:
> This is the fifth iteration of the patch series that:
>  * implements a new command
>      'frame apply [all | COUNT | -COUNT | level LEVEL...] [FLAG]... COMMAND'.
>  * enhance 'thread apply COMMAND' by adding FLAG arguments.
>  * adds some shortcuts commands.
>  * documents the above in gdb.texinfo and NEWS.
>  * adds a unit test for cli-utils.c.
>  * adds test for 'frame apply'.
>  * modify gdb.threads/pthreads.exp to test 'thread apply' FLAG arguments.
> 
> The fifth version is RFA v4. It handles the third set of comments
> given by Pedro.
> The main changes compared to RFA v3 are:
>   * frame level LEVEL...  replaces   frame id ID...

Very nice.  

The changes to the testcases and the unit tests makes
them much clearer, IMHO, well done.

This version looks great to me.

Thanks,
Pedro Alves

Re: [RFA_v4 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'

[Philippe Waroquiers]

On Wed, 2018-07-11 at 11:58 +0100, Pedro Alves wrote:
> On 07/10/2018 10:39 PM, Philippe Waroquiers wrote:
> > This is the fifth iteration of the patch series that:
> >  * implements a new command
> >      'frame apply [all | COUNT | -COUNT | level LEVEL...] [FLAG]... COMMAND'.
> >  * enhance 'thread apply COMMAND' by adding FLAG arguments.
> >  * adds some shortcuts commands.
> >  * documents the above in gdb.texinfo and NEWS.
> >  * adds a unit test for cli-utils.c.
> >  * adds test for 'frame apply'.
> >  * modify gdb.threads/pthreads.exp to test 'thread apply' FLAG arguments.
> > 
> > The fifth version is RFA v4. It handles the third set of comments
> > given by Pedro.
> > The main changes compared to RFA v3 are:
> >   * frame level LEVEL...  replaces   frame id ID...
> 
> Very nice.  
> 
> The changes to the testcases and the unit tests makes
> them much clearer, IMHO, well done.
> 
> This version looks great to me.
This version was pushed now ...

Thanks to Simon/Eli/Pedro for help/advise and the numerous reviews.

Philippe

Re: [RFA_v4 2/8] Implement frame apply [all | COUNT | -COUNT | level LEVEL... ] [FLAG]... COMMAND.

[Simon Marchi]

Hi Philippe,

On 2018-07-10 05:39 PM, Philippe Waroquiers wrote:
> +/* Implementation of the "frame apply level" command.  */
> +
> +static void
> +frame_apply_level_command (const char *cmd, int from_tty)
> +{
> +  if (!target_has_stack)
> +    error (_("No stack."));
> +
> +  bool level_found = false;
> +  const char *levels_str = cmd;
> +  number_or_range_parser levels (levels_str);
> +
> +  /* Skip the LEVEL list to find the flags and command args.  */
> +  while (!levels.finished ())
> +    {
> +      const int level_beg = levels.get_number ();

This variable is unused, so is caught by Tom's -Wunused-variable patch.
Is it just ok to remove the local variable and the assignment (but keep
the call)?

Simon

Re: [RFA_v4 2/8] Implement frame apply [all | COUNT | -COUNT | level LEVEL... ] [FLAG]... COMMAND.

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> Hi Philippe,
Simon> On 2018-07-10 05:39 PM, Philippe Waroquiers wrote:
>> +/* Implementation of the "frame apply level" command.  */
>> +
>> +static void
>> +frame_apply_level_command (const char *cmd, int from_tty)
>> +{
>> +  if (!target_has_stack)
>> +    error (_("No stack."));
>> +
>> +  bool level_found = false;
>> +  const char *levels_str = cmd;
>> +  number_or_range_parser levels (levels_str);
>> +
>> +  /* Skip the LEVEL list to find the flags and command args.  */
>> +  while (!levels.finished ())
>> +    {
>> +      const int level_beg = levels.get_number ();

Simon> This variable is unused, so is caught by Tom's -Wunused-variable patch.
Simon> Is it just ok to remove the local variable and the assignment (but keep
Simon> the call)?

That's what I have in my most recent patch.
get_number advances through the arguments, so it must be called.  But,
the result isn't used in this loop, but rather when the arguments are
re-parsed later.

Tom

Re: [RFA_v4 1/8] Add helper functions parse_flags and parse_flags_qcs

[Joel Brobecker]

Hello,

On Tue, Jul 10, 2018 at 11:39:19PM +0200, Philippe Waroquiers wrote:
> Add helper functions parse_flags and parse_flags_qcs.
> parse_flags helper function allows to look for a set of flags at
> the start of a string.
> A flag must be given individually.
> 
> parse_flags_qcs is a specialised helper function to handle
> the flags -q, -c and -s, that are used in the new command 'frame apply'
> and in the command 'thread apply.
> 
> Modify number_or_range_parser::get_number to differentiate a
> - followed by digits from a - followed by an alpha (i.e. a flag or an option).
> That is needed for the addition of the [FLAG]... arguments to
> thread apply ID... [FLAG]... COMMAND
> 
> Remove bool number_or_range_parser::m_finished, rather
> implement the 'finished' logic inside number_or_range_parser::finished.
> The new logic properly detects the end of parsing even if not at
> end of the string. This ensures that number_or_range_parser::cur_tok
> really points past the last parsed token when parsing is finished.
> Before, it was always pointing at the end of the string.
> As parsing now is finished directly when not positioned on a number,
> number_is_in_list must do an error check before the loop getting all
> numbers.
> 
> The error message for 'thread apply -$unknownconvvar p 1'
> is now the more clear:
>   Convenience variable must have integer value.
>   Invalid thread ID: -$unknownconvvar p 1
> instead of previously:
>   negative value
> 
> gdb/ChangeLog
> 2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* cli-utils.c (number_or_range_parser::get_number): Only handle
> 	numbers or convenience var as numbers.
> 	(parse_flags): New function.
> 	(parse_flags_qcs): New function.
> 	(number_or_range_parser::finished): Ensure parsing end is detected
> 	before end of string.
> 	* cli-utils.h (parse_flags): New function.
> 	(parse_flags_qcs): New function.
> 	(number_or_range_parser): Remove m_finished bool.
> 	(number_or_range_parser::skip_range): Set m_in_range to false.

For the record, this patch is causing some regressions that look
related to memory management (in this case, probably reading memory
that has already been freed). It's not always user-visible, but the
easiest way I have found to demonstrate the issue is to use valgrind
with the following example:

    | $ cat c.c
    | int
    | main (void)
    | {
    | }
    |
    | $ gcc -g c.c -o c
    |
    | $ valgrind /path/to/gdb -q c -ex 'break main'
    | ==107454== Memcheck, a memory error detector
    | ==107454== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
    | ==107454== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
    | ==107454== Command: /work/brobecke/bld/gdb-public-no-python/gdb/gdb -q c -ex break\ main
    | ==107454==
    | /homes/brobecke/.gdbinit:87: Error in sourced command file:
    | No symbol table is loaded.  Use the "file" command.
    | Reading symbols from c...done.
    | cBreakpoint 1 at 0x401131: file c.c, line 4.
    | (gdb) command 1
    | Type commands for breakpoint(s) 1, one per line.
    | End with a line saying just "end".
    | >end
    | ==119811== Invalid read of size 1
    | [snip]
    | ==119811==  Address 0x65c1349 is 9 bytes inside a block of size 10 free'd

Before this patch, no such valgrind error.

We detected this issue, because, with one of our Ada testcase, we actually
get an error:

    | (gdb) command 1
    | Type commands for breakpoint(s) 1, one per line.
    | End with a line saying just "end".
    | >print msg.all
    | >end
    | No breakpoint number 90.

I will look into tomorrow - this is just a heads up so someone else
doesn't spend time investigating the same issue.

> gdb/testsuite/ChangeLog
> 2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> 
> 	* gdb.base/skip.exp: Update expected error message.
> ---
>  gdb/cli/cli-utils.c             | 96 +++++++++++++++++++++++++++++++--
>  gdb/cli/cli-utils.h             | 45 ++++++++++++++--
>  gdb/testsuite/gdb.base/skip.exp |  6 +--
>  3 files changed, 134 insertions(+), 13 deletions(-)
> 
> diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
> index c55b5035e4..98b7414991 100644
> --- a/gdb/cli/cli-utils.c
> +++ b/gdb/cli/cli-utils.c
> @@ -137,7 +137,6 @@ number_or_range_parser::number_or_range_parser (const char *string)
>  void
>  number_or_range_parser::init (const char *string)
>  {
> -  m_finished = false;
>    m_cur_tok = string;
>    m_last_retval = 0;
>    m_end_value = 0;
> @@ -169,7 +168,10 @@ number_or_range_parser::get_number ()
>        /* Default case: state->m_cur_tok is pointing either to a solo
>  	 number, or to the first number of a range.  */
>        m_last_retval = get_number_trailer (&m_cur_tok, '-');
> -      if (*m_cur_tok == '-')
> +      /* If get_number_trailer has found a -, it might be the start
> +	 of a command option.  So, do not parse a range if the - is
> +	 followed by an alpha.  */
> +      if (*m_cur_tok == '-' && !isalpha (*(m_cur_tok + 1)))
>  	{
>  	  const char **temp;
>  
> @@ -196,8 +198,17 @@ number_or_range_parser::get_number ()
>  	}
>      }
>    else
> -    error (_("negative value"));
> -  m_finished = *m_cur_tok == '\0';
> +    {
> +      if (isdigit (*(m_cur_tok + 1)))
> +	error (_("negative value"));
> +      if (*(m_cur_tok + 1) == '$')
> +	{
> +	  /* Convenience variable.  */
> +	  m_last_retval = ::get_number (&m_cur_tok);
> +	  if (m_last_retval < 0)
> +	    error (_("negative value"));
> +	}
> +    }
>    return m_last_retval;
>  }
>  
> @@ -215,6 +226,21 @@ number_or_range_parser::setup_range (int start_value, int end_value,
>    m_end_value = end_value;
>  }
>  
> +/* See documentation in cli-utils.h.  */
> +
> +bool
> +number_or_range_parser::finished () const
> +{
> +  /* Parsing is finished when at end of string or null string,
> +     or we are not in a range and not in front of an integer, negative
> +     integer, convenience var or negative convenience var.  */
> +  return (m_cur_tok == NULL || *m_cur_tok == '\0'
> +	  || (!m_in_range
> +	      && !(isdigit (*m_cur_tok) || *m_cur_tok == '$')
> +	      && !(*m_cur_tok == '-'
> +		   && (isdigit (m_cur_tok[1]) || m_cur_tok[1] == '$'))));
> +}
> +
>  /* Accept a number and a string-form list of numbers such as is 
>     accepted by get_number_or_range.  Return TRUE if the number is
>     in the list.
> @@ -230,12 +256,15 @@ number_is_in_list (const char *list, int number)
>      return 1;
>  
>    number_or_range_parser parser (list);
> +
> +  if (parser.finished ())
> +    error (_("Arguments must be numbers or '$' variables."));
>    while (!parser.finished ())
>      {
>        int gotnum = parser.get_number ();
>  
>        if (gotnum == 0)
> -	error (_("Args must be numbers or '$' variables."));
> +	error (_("Arguments must be numbers or '$' variables."));
>        if (gotnum == number)
>  	return 1;
>      }
> @@ -304,3 +333,60 @@ check_for_argument (const char **str, const char *arg, int arg_len)
>      }
>    return 0;
>  }
> +
> +/* See documentation in cli-utils.h.  */
> +
> +int
> +parse_flags (const char **str, const char *flags)
> +{
> +  const char *p = skip_spaces (*str);
> +
> +  if (p[0] == '-'
> +      && isalpha (p[1])
> +      && (p[2] == '\0' || isspace (p[2])))
> +    {
> +      const char pf = p[1];
> +      const char *f = flags;
> +
> +      while (*f != '\0')
> +	{
> +	  if (*f == pf)
> +	    {
> +	      *str = skip_spaces (p + 2);
> +	      return f - flags + 1;
> +	    }
> +	  f++;
> +	}
> +    }
> +
> +  return 0;
> +}
> +
> +/* See documentation in cli-utils.h.  */
> +
> +bool
> +parse_flags_qcs (const char *which_command, const char **str,
> +		 qcs_flags *flags)
> +{
> +  switch (parse_flags (str, "qcs"))
> +    {
> +    case 0:
> +      return false;
> +    case 1:
> +      flags->quiet = true;
> +      break;
> +    case 2:
> +      flags->cont = true;
> +      break;
> +    case 3:
> +      flags->silent = true;
> +      break;
> +    default:
> +      gdb_assert_not_reached ("int qcs flag out of bound");
> +    }
> +
> +  if (flags->cont && flags->silent)
> +    error (_("%s: -c and -s are mutually exclusive"), which_command);
> +
> +  return true;
> +}
> diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
> index e34ee0df37..fa7d02d719 100644
> --- a/gdb/cli/cli-utils.h
> +++ b/gdb/cli/cli-utils.h
> @@ -75,8 +75,7 @@ public:
>  		    const char *end_ptr);
>  
>    /* Returns true if parsing has completed.  */
> -  bool finished () const
> -  { return m_finished; }
> +  bool finished () const;
>  
>    /* Return the string being parsed.  When parsing has finished, this
>       points past the last parsed token.  */
> @@ -96,6 +95,7 @@ public:
>    {
>      gdb_assert (m_in_range);
>      m_cur_tok = m_end_ptr;
> +    m_in_range = false;
>    }
>  
>  private:
> @@ -103,9 +103,6 @@ private:
>    number_or_range_parser (const number_or_range_parser &);
>    number_or_range_parser &operator= (const number_or_range_parser &);
>  
> -  /* True if parsing has completed.  */
> -  bool m_finished;
> -
>    /* The string being parsed.  When parsing has finished, this points
>       past the last parsed token.  */
>    const char *m_cur_tok;
> @@ -173,4 +170,42 @@ check_for_argument (char **str, const char *arg, int arg_len)
>  			     arg, arg_len);
>  }
>  
> +/* A helper function that looks for a set of flags at the start of a
> +   string.  The possible flags are given as a null terminated string.
> +   A flag in STR must either be at the end of the string,
> +   or be followed by whitespace.
> +   Returns 0 if no valid flag is found at the start of STR.
> +   Otherwise updates *STR, and returns N (which is > 0),
> +   such that FLAGS [N - 1] is the valid found flag.  */
> +extern int parse_flags (const char **str, const char *flags);
> +
> +/* qcs_flags struct regroups the flags parsed by parse_flags_qcs.  */
> +
> +struct qcs_flags
> +{
> +  bool quiet = false;
> +  bool cont = false;
> +  bool silent = false;
> +};
> +
> +/* A helper function that uses parse_flags to handle the flags qcs :
> +     A flag -q sets FLAGS->QUIET to true.
> +     A flag -c sets FLAGS->CONT to true.
> +     A flag -s sets FLAGS->SILENT to true.
> +
> +   The caller is responsible to initialize *FLAGS to false before the (first)
> +   call to parse_flags_qcs.
> +   parse_flags_qcs can then be called iteratively to search for more
> +   valid flags, as part of a 'main parsing loop' searching for -q/-c/-s
> +   flags together with other flags and options.
> +
> +   Returns true and updates *STR and one of FLAGS->QUIET, FLAGS->CONT,
> +   FLAGS->SILENT if it finds a valid flag.
> +   Returns false if no valid flag is found at the beginning of STR.
> +
> +   Throws an error if a flag is found such that both FLAGS->CONT and
> +   FLAGS->SILENT are true.  */
> +
> +extern bool parse_flags_qcs (const char *which_command, const char **str,
> +			     qcs_flags *flags);
>  #endif /* CLI_UTILS_H */
> diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp
> index 4b556b10a5..223c93d0d9 100644
> --- a/gdb/testsuite/gdb.base/skip.exp
> +++ b/gdb/testsuite/gdb.base/skip.exp
> @@ -69,9 +69,9 @@ gdb_test "skip function baz" "Function baz will be skipped when stepping\."
>  gdb_test "skip enable 999" "No skiplist entries found with number 999."
>  gdb_test "skip disable 999" "No skiplist entries found with number 999."
>  gdb_test "skip delete 999" "No skiplist entries found with number 999."
> -gdb_test "skip enable a" "Args must be numbers or '\\$' variables."
> -gdb_test "skip disable a" "Args must be numbers or '\\$' variables."
> -gdb_test "skip delete a" "Args must be numbers or '\\$' variables."
> +gdb_test "skip enable a" "Arguments must be numbers or '\\$' variables."
> +gdb_test "skip disable a" "Arguments must be numbers or '\\$' variables."
> +gdb_test "skip delete a" "Arguments must be numbers or '\\$' variables."
>  
>  # Ask for info on a skiplist entry which doesn't exist.
>  
> -- 
> 2.18.0

-- 
Joel

Re: [RFA_v4 1/8] Add helper functions parse_flags and parse_flags_qcs

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel>     | (gdb) command 1
Joel>     | Type commands for breakpoint(s) 1, one per line.
Joel>     | End with a line saying just "end".
Joel>     | >end
Joel>     | ==119811== Invalid read of size 1
Joel>     | [snip]
Joel>     | ==119811==  Address 0x65c1349 is 9 bytes inside a block of size 10 free'd

Joel> Before this patch, no such valgrind error.

Joel> We detected this issue, because, with one of our Ada testcase, we actually
Joel> get an error:

Joel> I will look into tomorrow - this is just a heads up so someone else
Joel> doesn't spend time investigating the same issue.

Hi Joel.

I also found this bug this weekend, while trying out -fsanitize=address.

I have a patch for this one.  I haven't written the ChangeLog yet but I
will try to do it as soon as possible.

Actually I have patches to make gdb nearly -fsanitize=address clean; or
at least, the ordinary test suite on my machine is down to 1 failure
(there are some additional gdbserver failures in bugzilla that I haven't
looked at yet).  My series also addresses much of -fsanitize=undefined
as well.

Tom

Re: [RFA_v4 1/8] Add helper functions parse_flags and parse_flags_qcs

[Philippe Waroquiers]

On Mon, 2018-07-30 at 13:15 -0700, Joel Brobecker wrote:
> > gdb/ChangeLog
> > 2018-07-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> > 
> > 	* cli-utils.c (number_or_range_parser::get_number): Only handle
> > 	numbers or convenience var as numbers.
> > 	(parse_flags): New function.
> > 	(parse_flags_qcs): New function.
> > 	(number_or_range_parser::finished): Ensure parsing end is detected
> > 	before end of string.
> > 	* cli-utils.h (parse_flags): New function.
> > 	(parse_flags_qcs): New function.
> > 	(number_or_range_parser): Remove m_finished bool.
> > 	(number_or_range_parser::skip_range): Set m_in_range to false.
> 
> For the record, this patch is causing some regressions that look
> related to memory management (in this case, probably reading memory
> that has already been freed). It's not always user-visible, but the
> easiest way I have found to demonstrate the issue is to use valgrind
> with the following example:
As I felt a little bit guilty, I still started to look at it (and then I saw
the mail of Tom telling he already has a fix).
As far as I can see, the underlying problem was already there with the
below reproducer and gdb 8.1, but you just had to do 2 breakpoints and
then use 'command 1 2':
gdb -ex 'break main' -ex 'break main' ./c
GNU gdb (GDB) 8.1
....
Breakpoint 1 at 0x669: file c.c, line 1.
Note: breakpoint 1 also set at pc 0x669.
Breakpoint 2 at 0x669: file c.c, line 1.
(gdb) command 1 2
Type commands for breakpoint(s) 1 2, one per line.
End with a line saying just "end".
>end
==26216== Invalid read of size 1
==26216==    at 0x21B424: number_or_range_parser::get_number() (cli-utils.c:167)
==26216==    by 0x2B52F7: map_breakpoint_numbers(char const*, gdb::function_view<void (breakpoint*)>) (breakpoint.c:14130)
==26216==    by 0x2BEF88: commands_command_1(char const*, int, command_line*) (breakpoint.c:1274)
==26216==    by 0x2148E8: cmd_func(cmd_list_element*, char const*, int) (cli-decode.c:1886)
==26216==    by 0x44D8B7: execute_command(char const*, int) (top.c:630)
....


Outside of valgrind, the behaviour is strange,
as gdb 8.1 (and gdb 8.2) ask twice a list of commands :

(gdb) command 1 2
Type commands for breakpoint(s) 1 2, one per line.
End with a line saying just "end".
>end
Type commands for breakpoint(s) 1 2, one per line.
End with a line saying just "end".
>end
(gdb) 

As far as I could see, the problem is that when handling a line of input
(provided via a const char* arg), reading some more lines of input will
free the previous line (but which is still being parsed in our case).

Waiting impatiently to see the fix done by Tom.

Philippe, feeling not too guilty anymore :)

Re: [RFA_v4 1/8] Add helper functions parse_flags and parse_flags_qcs

[Joel Brobecker]

> I also found this bug this weekend, while trying out -fsanitize=address.
>
> I have a patch for this one.  I haven't written the ChangeLog yet but I
> will try to do it as soon as possible.
> 
> Actually I have patches to make gdb nearly -fsanitize=address clean; or
> at least, the ordinary test suite on my machine is down to 1 failure
> (there are some additional gdbserver failures in bugzilla that I haven't
> looked at yet).  My series also addresses much of -fsanitize=undefined
> as well.

Very nice!

Thanks Tom and Philippe,
-- 
Joel

Re: [RFA_v4 1/8] Add helper functions parse_flags and parse_flags_qcs

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> I also found this bug this weekend, while trying out -fsanitize=address.
>> 
>> I have a patch for this one.  I haven't written the ChangeLog yet but I
>> will try to do it as soon as possible.
>> 
>> Actually I have patches to make gdb nearly -fsanitize=address clean; or
>> at least, the ordinary test suite on my machine is down to 1 failure
>> (there are some additional gdbserver failures in bugzilla that I haven't
>> looked at yet).  My series also addresses much of -fsanitize=undefined
>> as well.

Joel> Very nice!

You may have to wait a bit longer because the buildbot is telling me
this patch is no good.  I'll try to debug it tonight.

Tom

Re: [RFA_v4 1/8] Add helper functions parse_flags and parse_flags_qcs

[Philippe Waroquiers]

On Tue, 2018-07-31 at 09:40 -0600, Tom Tromey wrote:
> > > > > > "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
> > > I also found this bug this weekend, while trying out -fsanitize=address.
> > > 
> > > I have a patch for this one.  I haven't written the ChangeLog yet but I
> > > will try to do it as soon as possible.
> > > 
> > > Actually I have patches to make gdb nearly -fsanitize=address clean; or
> > > at least, the ordinary test suite on my machine is down to 1 failure
> > > (there are some additional gdbserver failures in bugzilla that I haven't
> > > looked at yet).  My series also addresses much of -fsanitize=undefined
> > > as well.
> 
> Joel> Very nice!
> 
> You may have to wait a bit longer because the buildbot is telling me
> this patch is no good.  I'll try to debug it tonight.

As far as I can see, this problem is a regression that appeared in gdb 8.1,
but which was made (more) visible by the 'parse_flags' patch in (future) 8.3.
At least valgrind + gdb 8.0 does not give a problem with the small reproducer 
  command 1 2
  end
while it gives an error with gdb 8.1.

We also have a (small) functional regression:
with gdb 8.0, it was possible to remove all commands of a set of
breapoints by doing the above 'command 1 2/end'.

From 8.1 onwards, when giving such empty command list, gdb asks
the list of command for each breakpoint, instead of asking it once.

The below patch seems to solve the memory corruption (at least
for the simple case), but does not solve the functional regression that
appeared in 8.1.

I am wondering if the correct solution would not be to avoid
having input lines memory being managed 'manually' like it is now,
as having the 'input const char* arg' disappearing 'under the carpet'
is quite tricky, and we might have other places where a previous
line of input must be kept alive, while new lines of input have
to be read.

Philippe

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6b6e1f6c25..dabd81e138 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1222,6 +1222,9 @@ commands_command_1 (const char *arg, int from_tty,
 
   std::string new_arg;
 
+  /* arg might be an input line that might be released when reading
+     new input lines for the list of commands.  So, build a new arg
+     to keep the input alive during the map_breakpoint_numbers call.  */
   if (arg == NULL || !*arg)
     {
       if (breakpoint_count - prev_breakpoint_count > 1)
@@ -1231,6 +1234,11 @@ commands_command_1 (const char *arg, int from_tty,
        new_arg = string_printf ("%d", breakpoint_count);
       arg = new_arg.c_str ();
     }
+  else
+    {
+      new_arg = arg;
+      arg = new_arg.c_str ();
+    }
 
   map_breakpoint_numbers
     (arg, [&] (breakpoint *b)

Re: [RFA_v4 1/8] Add helper functions parse_flags and parse_flags_qcs

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> I am wondering if the correct solution would not be to avoid
Philippe> having input lines memory being managed 'manually' like it is now,
Philippe> as having the 'input const char* arg' disappearing 'under the carpet'
Philippe> is quite tricky, and we might have other places where a previous
Philippe> line of input must be kept alive, while new lines of input have
Philippe> to be read.

Yeah, my approach has been to require the callers of
handle_line_of_input to handle the storage, so nothing relies on
saved_command_line surviving across reentrant calls.

Thanks for pointing out that other regression.  I don't think I was
aware of that one.

Tom

Re: [RFA_v4 1/8] Add helper functions parse_flags and parse_flags_qcs

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
Philippe> I am wondering if the correct solution would not be to avoid
Philippe> having input lines memory being managed 'manually' like it is now,
Philippe> as having the 'input const char* arg' disappearing 'under the carpet'
Philippe> is quite tricky, and we might have other places where a previous
Philippe> line of input must be kept alive, while new lines of input have
Philippe> to be read.

Tom> Yeah, my approach has been to require the callers of
Tom> handle_line_of_input to handle the storage, so nothing relies on
Tom> saved_command_line surviving across reentrant calls.

The bug in my patch turns out to be this line in top.c:

      if (repeat_arguments != NULL && cmd_start == saved_command_line)

... which relies on handle_line_of_input returning saved_command_line
exactly rather than a copy.

Fixing this in a good way will require passing a bit more information
through the maze of functions.  This part of gdb sure seems complicated
for what it does.

A less principled way to fix this would be to stuff another global in
there somewhere, to indicate that the current command is a repeat.  This
is "safe" in that nearly nothing ought to check the global.  However, I
think I'd rather try not to introduce more globals, at least unless I
get too annoyed.

Tom