Set breakpoint on the right line

Message ID 1407806302-14295-1-git-send-email-yao@codesourcery.com
State Committed
Headers

Commit Message

Yao Qi Aug. 12, 2014, 1:18 a.m. UTC
  In gdb.base/watchpoint-hw-hit-once.exp, test scans source and set
breakpoint on the line having "break-at-exit",

  gdb_breakpoint [gdb_get_line_number "break-at-exit"]

However, in watchpoint-hw-hit-once.c, there are two lines having
this key word:

  dummy = 1;	/* Stub to catch break-at-exit after WATCHEE has been hit.  */
  dummy = 2;	/* break-at-exit */

so the test sets breakpoint on the first one, while I think it is
expected to set breakpoint on the second one, as far as I can tell
from the comments in watchpoint-hw-hit-once.c:

  /* Stub lines are present as no breakpoints/watchpoint gets hit if current PC
     already stays on the line PC while entering "step"/"continue".  */

This patch is to change the source matching pattern so that test
can correctly set breakpoint on the right line.  This patch fixes
a fail we found on arm-none-eabi target.  Run it again
on x86_64-linux, no result changes.

gdb/testsuite:

2014-08-12  Yao Qi  <yao@codesourcery.com>

	* gdb.base/watchpoint-hw-hit-once.exp: Set breakpoint on the
	right line.
---
 gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Yao Qi Aug. 19, 2014, 12:46 a.m. UTC | #1
On 08/12/2014 09:18 AM, Yao Qi wrote:
> In gdb.base/watchpoint-hw-hit-once.exp, test scans source and set
> breakpoint on the line having "break-at-exit",
> 
>   gdb_breakpoint [gdb_get_line_number "break-at-exit"]
> 
> However, in watchpoint-hw-hit-once.c, there are two lines having
> this key word:
> 
>   dummy = 1;	/* Stub to catch break-at-exit after WATCHEE has been hit.  */
>   dummy = 2;	/* break-at-exit */
> 
> so the test sets breakpoint on the first one, while I think it is
> expected to set breakpoint on the second one, as far as I can tell
> from the comments in watchpoint-hw-hit-once.c:
> 
>   /* Stub lines are present as no breakpoints/watchpoint gets hit if current PC
>      already stays on the line PC while entering "step"/"continue".  */
> 
> This patch is to change the source matching pattern so that test
> can correctly set breakpoint on the right line.  This patch fixes
> a fail we found on arm-none-eabi target.  Run it again
> on x86_64-linux, no result changes.
> 
> gdb/testsuite:
> 
> 2014-08-12  Yao Qi  <yao@codesourcery.com>
> 
> 	* gdb.base/watchpoint-hw-hit-once.exp: Set breakpoint on the
> 	right line.

Ping.  https://sourceware.org/ml/gdb-patches/2014-08/msg00190.html
  
Joel Brobecker Aug. 19, 2014, 6:38 a.m. UTC | #2
> This patch is to change the source matching pattern so that test
> can correctly set breakpoint on the right line.  This patch fixes
> a fail we found on arm-none-eabi target.  Run it again
> on x86_64-linux, no result changes.
> 
> gdb/testsuite:
> 
> 2014-08-12  Yao Qi  <yao@codesourcery.com>
> 
> 	* gdb.base/watchpoint-hw-hit-once.exp: Set breakpoint on the
> 	right line.

I think you are right, but can you describe the error you are getting
with arm-none-eabi?
  
Yao Qi Aug. 19, 2014, 1:31 p.m. UTC | #3
On 08/19/2014 02:38 PM, Joel Brobecker wrote:
> I think you are right, but can you describe the error you are getting
> with arm-none-eabi?

Sure, the fail is shown as below:

continue^M
Continuing.^M
^M
*** EXIT code 0^M
[Inferior 1 (Remote target) exited normally]^M
(gdb) FAIL: gdb.base/watchpoint-hw-hit-once.exp: continue to break-at-exit (the program exited)

I also copy it in the commit log.  Patch is pushed in.
  
Joel Brobecker Aug. 19, 2014, 1:48 p.m. UTC | #4
> > I think you are right, but can you describe the error you are getting
> > with arm-none-eabi?
> 
> Sure, the fail is shown as below:
> 
> continue^M
> Continuing.^M
> ^M
> *** EXIT code 0^M
> [Inferior 1 (Remote target) exited normally]^M
> (gdb) FAIL: gdb.base/watchpoint-hw-hit-once.exp: continue to break-at-exit (the program exited)
> 
> I also copy it in the commit log.  Patch is pushed in.

I actually didn't pre-approve, but never mind :-).

I think I understand, now. The problem using the first break-at-exit
line was that the watchpoint would be triggering at the same location
the breakpoint was inserted. So the "continue" test right after would
never hit that breakpoint. If my understanding is correct, the patch
is officially approved :).
  
Yao Qi Aug. 20, 2014, 12:17 a.m. UTC | #5
On 08/19/2014 09:48 PM, Joel Brobecker wrote:
> I actually didn't pre-approve, but never mind :-).
> 

Ur, I misunderstood you, sorry.

> I think I understand, now. The problem using the first break-at-exit
> line was that the watchpoint would be triggering at the same location
> the breakpoint was inserted. So the "continue" test right after would
> never hit that breakpoint. If my understanding is correct, the patch
> is officially approved :).

Yes, that is correct.
  

Patch

diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp b/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp
index 53de3d5..49c0d6a 100644
--- a/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp
+++ b/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp
@@ -29,7 +29,7 @@  if ![runto_main] {
 
 gdb_test "rwatch watchee"
 
-gdb_breakpoint [gdb_get_line_number "break-at-exit"]
+gdb_breakpoint [gdb_get_line_number "dummy = 2"]
 
 gdb_test "continue" "Continuing.\r\nHardware read watchpoint \[0-9\]+: watchee\r\n\r\nValue = 0\r\n.*"