[RFAv2 1/2] Fix regression for multi breakpoints command line clearing.

[RFAv2 1/2] Fix regression for multi breakpoints command line clearing.

[Philippe Waroquiers]

breakpoint.c is modified to fix the regression introduced
when clearing the commands of several breakpoints by giving an empty
list of commands, by just typing "end".
GDB should read an empty list of command once, but it reads
it for each breakpoint, as an empty list of command is NULL,
and NULL is interpreted as 'not having read the command list yet'.

The fix consists in having a boolean set to true once the
command list has been read.

gdb/ChangeLog

2018-08-26  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* breakpoint.c (commands_command_1): New boolean cmd_read
	to detect cmd was already read.
---
 gdb/breakpoint.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8f0feaa474..4e7dac5157 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1219,6 +1219,10 @@ commands_command_1 (const char *arg, int from_tty,
 		    struct command_line *control)
 {
   counted_command_line cmd;
+  /* cmd_read will be true once we have read cmd.  Note that cmd might still be
+     NULL after the call to read_command_lines if the user provides an empty
+     list of command by just typing "end".  */
+  bool cmd_read = false;
 
   std::string new_arg;
 
@@ -1235,8 +1239,9 @@ commands_command_1 (const char *arg, int from_tty,
   map_breakpoint_numbers
     (arg, [&] (breakpoint *b)
      {
-       if (cmd == NULL)
+       if (!cmd_read)
 	 {
+	   gdb_assert (cmd == NULL);
 	   if (control != NULL)
 	     cmd = control->body_list_0;
 	   else
@@ -1256,6 +1261,7 @@ commands_command_1 (const char *arg, int from_tty,
 
 	       cmd = read_command_lines (str.c_str (), from_tty, 1, validator);
 	     }
+	   cmd_read = true;
 	 }
 
        /* If a breakpoint was on the list more than once, we don't need to
-- 
2.18.0

[RFAv2 2/2] Modify gdb.base/commands.exp to test multi breakpoints command clearing.

[Philippe Waroquiers]

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

	* gdb.base/commands.exp: Test multi breakpoints command clearing.
---
 gdb/testsuite/gdb.base/commands.exp | 47 +++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index 259b89b803..52a22bb5dd 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -281,6 +281,48 @@ proc_with_prefix breakpoint_command_test {} {
     gdb_test "print value" " = 5"
 }
 
+# Test clearing the commands of several breakpoints with one single "end".
+proc_with_prefix breakpoint_clear_command_test {} {
+    runto_or_return factorial
+
+    set any "\[^\r\n\]*"
+    delete_breakpoints
+    gdb_test "break factorial" "Breakpoint.*at.*"
+    gdb_test_no_output "set \$bpnumfactorial = \$bpnum"
+    gdb_test "break main" "Breakpoint.*at.*"
+    gdb_test_no_output "set \$bpnummain = \$bpnum"
+
+    gdb_test \
+	[multi_line_input \
+	     {commands $bpnumfactorial $bpnummain} \
+	     {  print 1234321} \
+	     {end}] \
+	"End with.*" \
+	"set commands of two breakpoints to print 1234321"
+    gdb_test "info breakpoints" \
+	[multi_line \
+	     "${any}What${any}" \
+	     "${any}in factorial${any}" \
+	     "${any}print 1234321${any}" \
+	     "${any}in main${any}" \
+	     "${any}print 1234321${any}" \
+	    ] \
+	"print 1234321 command present in the two breakpoints"
+    gdb_test \
+	[multi_line_input \
+	     {commands $bpnumfactorial $bpnummain} \
+	     {end}] \
+	"End with.*" \
+	"clear the command list of the two breakpoints"
+    gdb_test "info breakpoints" \
+	[multi_line \
+	     "${any}What${any}" \
+	     "${any}in factorial${any}" \
+	     "${any}in main${any}" \
+	    ] \
+	"print 1234321 command is not present anymore in the two breakpoints"
+    }
+
 # Test a simple user defined command (with arguments)
 proc_with_prefix user_defined_command_test {} {
     global valnum_re
@@ -368,8 +410,8 @@ proc_with_prefix user_defined_command_case_sensitivity {} {
 
     gdb_test "print 456\nend" "" "enter commands 2"
 
-    gdb_test "Homer-Simpson" " = 123" "execute command"
-    gdb_test "HomeR-SimpsoN" " = 456" "execute command"
+    gdb_test "Homer-Simpson" " = 123" "execute command Homer-Simpson"
+    gdb_test "HomeR-SimpsoN" " = 456" "execute command HomeR-SimpsoN"
     gdb_test "HOMER-SIMPSON" "Undefined command.*" "try to call in upper case"
     gdb_test "homer-simpson" "Undefined command.*" "try to call in lower case"
 }
@@ -1134,6 +1176,7 @@ progvar_complex_if_while_test
 if_while_breakpoint_command_test
 infrun_breakpoint_command_test
 breakpoint_command_test
+breakpoint_clear_command_test
 user_defined_command_test
 user_defined_command_case_sensitivity
 user_defined_command_args_eval
-- 
2.18.0

[RFAv2 0/2] Fix regression for multi breakpoints command line clearing.

[Philippe Waroquiers]

Fix regression for multi breakpoints command line clearing

Without the fix, to clear the commands associated to several breakpoints,
the user had to type 'end' once per breakpoint.

Fix done in breakpoint.c.
gdb.base/commands.exp changed to reproduce/test the problem.


Compared to first RFA, the changes are:
* Remove the code (+ comment) that was fixing the 'use after free',
  as a fix was pushed by Tom in 12582533306990c9406aedd960fa411c317a67de.
* gdb.base/commands.exp changed according to the comments given by Pedro.
    

Re: [RFAv2 1/2] Fix regression for multi breakpoints command line clearing.

[Tom Tromey]

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

Philippe> 2018-08-26  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

Philippe> 	* breakpoint.c (commands_command_1): New boolean cmd_read
Philippe> 	to detect cmd was already read.

Thanks for doing this.
This is ok.  I think it should go on the 8.2 branch as well.

Tom

Re: [RFAv2 2/2] Modify gdb.base/commands.exp to test multi breakpoints command clearing.

[Tom Tromey]

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

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

Philippe> 	* gdb.base/commands.exp: Test multi breakpoints command clearing.

Thank you.  This is ok.

Tom

Re: [RFAv2 1/2] Fix regression for multi breakpoints command line clearing.

[Philippe Waroquiers]

On Tue, 2018-08-28 at 09:58 -0600, Tom Tromey wrote
> This is ok.  I think it should go on the 8.2 branch as well.

Pushed the series to master and 8.2 branch.

Thanks to Pedro and Tom for the reviews.

Philippe