This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

[commit] [patchv2 1/2] save breakpoints does not save disabled breakpoints correctly

From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
To: Yao Qi <yao at codesourcery dot com>, Pedro Alves <palves at redhat dot com>
Cc: gdb-patches at sourceware dot org, Miroslav Franc <mfranc at redhat dot com>
Date: Sun, 12 Oct 2014 21:55:30 +0200
Subject: [commit] [patchv2 1/2] save breakpoints does not save disabled breakpoints correctly
Authentication-results: sourceware.org; auth=none

On Fri, 10 Oct 2014 07:55:50 +0200, Yao Qi wrote:
> These two patches look right to me.

Therefore checked them in.


On Fri, 10 Oct 2014 14:31:21 +0200, Pedro Alves wrote:
> In places were we want to avoid that, and we want to write a pattern
> that spawns multiple lines, we build the regex with join.  E.g.,
> gdb.btrace/instruction_history.exp:
> 
> # test that we see the expected instructions
> gdb_test "record instruction-history 3,7" [join [list \
>   "3\t   0x\[0-9a-f\]+ <loop\\+\[0-9\]+>:\tje     0x\[0-9a-f\]+ <loop\\+\[0-9\]+>" \
>   "4\t   0x\[0-9a-f\]+ <loop\\+\[0-9\]+>:\tdec    %eax" \
>   "5\t   0x\[0-9a-f\]+ <loop\\+\[0-9\]+>:\tjmp    0x\[0-9a-f\]+ <loop\\+\[0-9\]+>" \
>   "6\t   0x\[0-9a-f\]+ <loop\\+\[0-9\]+>:\tcmp    \\\$0x0,%eax" \
>   "7\t   0x\[0-9a-f\]+ <loop\\+\[0-9\]+>:\tje     0x\[0-9a-f\]+ <loop\\+\[0-9\]+>\r" \
>   ] "\r\n"]

This looks more safe to me but the other was has been approved.


Thanks,
Jan

--- Begin Message ---

From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
Date: Sun, 12 Oct 2014 21:47:13 +0200
Subject: [PATCH 1/2] Use gdb_test_sequence in gdb.base/save-bp.exp.

But IMO it is a functionality regression as:

 * gdb_test_sequence permits arbitary number of lines of text between those
   lines being matched.  Former regex string did not allow it.
   This may make a difference if GDB regresses by printing some unexpected
   line after the breakpoint info line (like a "silent" line).

>  * \[\r\n\]+ can be used to anchor the beginning of the pattern, in the sense
>    of Perl regex ^ /m match.  At least I have found such cases in existing
>    *.exp files so I used that.  Using ^ really does not work.
>
>    But I am not aware how to do Perl regex $ /m match.  Using $ really does
>    not work.  But this means that for example the trailing
>      ( \\((host|target) evals\\))?
>    on the line
>      "\[\r\n\]+\[ \t\]+stop only if i == 1( \\((host|target) evals\\))?"
>    originally made sense there but now it can be removed as it has no longer
>    any functionality there - it will match now any trailing line garbage.

by Yao Qi:

In this test case, ( \\((host|target) evals\\))? isn't needed in the
pattern.  What we test here is to save breakpoints into file and restore
them from file.  The contents saved in file are:

break save-bp.c:31
  condition $bpnum i == 1

the information about the place where the condition is evaluated isn't
saved, so we don't need to check.  Breakpoint save and restore has
nothing to do with where the condition is evaluated (host or target).  I
am fine to leave it here now.

gdb/testsuite/ChangeLog
2014-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/save-bp.exp (info break): Use gdb_test_sequence.
---
 gdb/testsuite/ChangeLog            |  4 ++++
 gdb/testsuite/gdb.base/save-bp.exp | 15 +++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 348adff..fdd18d7 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2014-10-12  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	* gdb.base/save-bp.exp (info break): Use gdb_test_sequence.
+
 2014-10-11  Yao Qi  <yao@codesourcery.com>
 
 	* gdb.server/server-kill.exp: Execute command
diff --git a/gdb/testsuite/gdb.base/save-bp.exp b/gdb/testsuite/gdb.base/save-bp.exp
index ba98633..61f647c 100644
--- a/gdb/testsuite/gdb.base/save-bp.exp
+++ b/gdb/testsuite/gdb.base/save-bp.exp
@@ -72,5 +72,16 @@ gdb_test "source $bps" "" "source bps"
 # Now, verify that all breakpoints have been created correctly...
 set bp_row_start "\[0-9\]+ +breakpoint +keep +y +0x\[0-9a-f\]+ +in"
 set dprintf_row_start "\[0-9\]+ +dprintf +keep +y +0x\[0-9a-f\]+ +in"
-gdb_test "info break" \
-  " *Num +Type +Disp +Enb +Address +What\r\n$bp_row_start break_me at .*$srcfile:\[0-9\]+\r\n$bp_row_start main at .*$srcfile:$loc_bp2\r\n$bp_row_start main at .*$srcfile:$loc_bp3 +thread 1\r\n\[ \t]+stop only in thread 1\r\n$bp_row_start main at .*$srcfile:$loc_bp4\r\n\[ \t\]+stop only if i == 1( \\((host|target) evals\\))?\r\n$bp_row_start main at .*$srcfile:$loc_bp5\r\n\[ \t\]+silent\r\n$dprintf_row_start main at .*$srcfile:$loc_bp5\r\n\[ \t\]+printf.*"
+gdb_test_sequence "info break" "info break" [list				\
+  "\[\r\n\]+Num +Type +Disp +Enb +Address +What"				\
+  "\[\r\n\]+$bp_row_start break_me at \[^\r\n\]*$srcfile:\[0-9\]+"		\
+  "\[\r\n\]+$bp_row_start main at \[^\r\n\]*$srcfile:$loc_bp2"			\
+  "\[\r\n\]+$bp_row_start main at \[^\r\n\]*$srcfile:$loc_bp3 +thread 1"	\
+  "\[\r\n\]+\[ \t]+stop only in thread 1"					\
+  "\[\r\n\]+$bp_row_start main at \[^\r\n\]*$srcfile:$loc_bp4"			\
+  "\[\r\n\]+\[ \t\]+stop only if i == 1( \\((host|target) evals\\))?"		\
+  "\[\r\n\]+$bp_row_start main at \[^\r\n\]*$srcfile:$loc_bp5"			\
+  "\[\r\n\]+\[ \t\]+silent"							\
+  "\[\r\n\]+$dprintf_row_start main at \[^\r\n\]*$srcfile:$loc_bp5"		\
+  "\[\r\n\]+\[ \t\]+printf"							\
+]
-- 
2.1.0

--- End Message ---

--- Begin Message ---

From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
Date: Sun, 12 Oct 2014 21:52:39 +0200
Subject: [PATCH 2/2] Fix "save breakpoints" for "disable $bpnum" command.

gdb/ChangeLog
2014-10-12  Miroslav Franc  <mfranc@redhat.com>

	Fix "save breakpoints" for "disable $bpnum" command.
	* breakpoint.c (save_breakpoints): Add $bpnum for disable.

gdb/testsuite/ChangeLog
2014-10-12  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix "save breakpoints" for "disable $bpnum" command.
	* gdb.base/save-bp.c (main): Add label.
	* gdb.base/save-bp.exp: Add 8th disabled breakpoint.  Match it.
---
 gdb/ChangeLog                      | 5 +++++
 gdb/breakpoint.c                   | 2 +-
 gdb/testsuite/ChangeLog            | 6 ++++++
 gdb/testsuite/gdb.base/save-bp.c   | 2 +-
 gdb/testsuite/gdb.base/save-bp.exp | 6 ++++++
 5 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6e15e53..724a971 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2014-10-12  Miroslav Franc  <mfranc@redhat.com>
+
+	Fix "save breakpoints" for "disable $bpnum" command.
+	* breakpoint.c (save_breakpoints): Add $bpnum for disable.
+
 2014-10-10  Pedro Alves  <palves@redhat.com>
 
 	* Makefile.in (ALL_TARGET_OBS): Remove mips-irix-tdep.o and solib-irix.o.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3044916..a144a7e 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -16130,7 +16130,7 @@ save_breakpoints (char *filename, int from_tty,
       }
 
     if (tp->enable_state == bp_disabled)
-      fprintf_unfiltered (fp, "disable\n");
+      fprintf_unfiltered (fp, "disable $bpnum\n");
 
     /* If this is a multi-location breakpoint, check if the locations
        should be individually disabled.  Watchpoint locations are
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index fdd18d7..bb362dc 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,11 @@
 2014-10-12  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
+	Fix "save breakpoints" for "disable $bpnum" command.
+	* gdb.base/save-bp.c (main): Add label.
+	* gdb.base/save-bp.exp: Add 8th disabled breakpoint.  Match it.
+
+2014-10-12  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
 	* gdb.base/save-bp.exp (info break): Use gdb_test_sequence.
 
 2014-10-11  Yao Qi  <yao@codesourcery.com>
diff --git a/gdb/testsuite/gdb.base/save-bp.c b/gdb/testsuite/gdb.base/save-bp.c
index 9a72fe8..f01f031 100644
--- a/gdb/testsuite/gdb.base/save-bp.c
+++ b/gdb/testsuite/gdb.base/save-bp.c
@@ -31,6 +31,6 @@ main (void)
     break_me (); /* Try a condition-specific breakpoint.  */
 
   break_me (); /* Finally, try a breakpoint with commands.  */
-  return 0;
+  return 0; /* Return line. */
 }
 
diff --git a/gdb/testsuite/gdb.base/save-bp.exp b/gdb/testsuite/gdb.base/save-bp.exp
index 61f647c..c8c9481 100644
--- a/gdb/testsuite/gdb.base/save-bp.exp
+++ b/gdb/testsuite/gdb.base/save-bp.exp
@@ -47,6 +47,10 @@ gdb_test "commands\nsilent\nend" "End with.*" "add breakpoint commands"
 
 gdb_test "dprintf ${srcfile}:${loc_bp5},\"At foo entry\\n\"" "Dprintf .*"
 
+set loc_bp8 [gdb_get_line_number "Return line"]
+gdb_breakpoint "${srcfile}:${loc_bp8}"
+gdb_test_no_output {disable $bpnum}
+
 # Now, save the breakpoints into a file...
 if {[is_remote host]} {
     set bps bps
@@ -71,6 +75,7 @@ gdb_test "source $bps" "" "source bps"
 
 # Now, verify that all breakpoints have been created correctly...
 set bp_row_start "\[0-9\]+ +breakpoint +keep +y +0x\[0-9a-f\]+ +in"
+set disabled_row_start "\[0-9\]+ +breakpoint +keep +n +0x\[0-9a-f\]+ +in"
 set dprintf_row_start "\[0-9\]+ +dprintf +keep +y +0x\[0-9a-f\]+ +in"
 gdb_test_sequence "info break" "info break" [list				\
   "\[\r\n\]+Num +Type +Disp +Enb +Address +What"				\
@@ -84,4 +89,5 @@ gdb_test_sequence "info break" "info break" [list				\
   "\[\r\n\]+\[ \t\]+silent"							\
   "\[\r\n\]+$dprintf_row_start main at \[^\r\n\]*$srcfile:$loc_bp5"		\
   "\[\r\n\]+\[ \t\]+printf"							\
+  "\[\r\n\]+$disabled_row_start main at \[^\r\n\]*$srcfile:$loc_bp8"		\
 ]
-- 
2.1.0

--- End Message ---

References:
- Re: [patchv2 1/2] save breakpoints does not save disabled breakpoints correctly
  - From: Yao Qi

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]