public inbox for gdb@sourceware.org
 help / color / mirror / Atom feed
* question about ARM watchpoints
@ 2014-09-01  8:57 Joel Brobecker
  2014-09-04 22:37 ` Gareth McMullin
  0 siblings, 1 reply; 11+ messages in thread
From: Joel Brobecker @ 2014-09-01  8:57 UTC (permalink / raw)
  To: gdb; +Cc: Fabien Chouteau

Hello,

Some of our ARM bareboard testing is done over QEMU, and we came
across an issue with watchpoints. The testcase is very simple:
We set a watchpoint on a integer global variable, and then
resume the execution until the variable gets updated.

Everything works pretty well, except that we noticed that GDB stops
2 instructions after the variable update, rather than at the next
instruction. For instance, this is the code we have:

       0x00000060:    movt    r3, #0
       0x00000064:    str     r2, [r3]  <<<--- new value stored here
       0x00000068:    pop     {r11, pc}

Looking at the infrun+remote debug traces, we can see that QEMU sends
GDB a watchpoint event with the PC set to the insn after the store:

    infrun: stop_pc = 0x68
    infrun: stopped by watchpoint
    infrun: stopped data address = 0xd820

At this point, GDB then does a single-step, which takes us past
the "pop" to the next instruction. My guess is that GDB probably
thinks that we got notified before the watched data got modified,
and thus needs to do a single-step in order to read the new value.

The question then becomes: Is QEMU behaving correctly, in which
case we should be modifying GDB, or is QEMU signaling GDB too
late, in which case we should be fixing QEMU?

We tried finding an answer to that question, but without much
success. We found the following in the ARMv7-M Architecture
Reference Manual, for instance:

    For all other debug events, including PC match watchpoints,
    the address is that of an instruction which in a simple execution
    model is one which executes after the instruction which caused
    the event, but which itself has not executed

But I'm a little confused when reading this, and I am not even
sure that it applies to data watchpoints. We tried experimenting
with other systems that do not involve QEMU, but came up short.
Surprisingly, watchpoints didn't even trigger when trying on a system
that runs GNU/Linux, for instance. Slightly better, but still not
completely convincing - we tried debugging via stlink, a tool
which provides a gdbserver-like interface.  The watchpoint was inserted,
and triggered an event, but stlink reported a SIGTRAP instead of
a watchpoint event. The only element which might be a clue is the fact
that the PC at the SIGTRAP event is at the instruction after the store,
so it might be confirming QEMU's behavior and indicate that GDB should
be modified.

I am sending this email to ask if anyone might have a more
authoritative answer than ours. If we were able to know for sure,
we can then work on fixing the bug on the right side of the fence :).

Thanks!
-- 
Joel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about ARM watchpoints
  2014-09-01  8:57 question about ARM watchpoints Joel Brobecker
@ 2014-09-04 22:37 ` Gareth McMullin
  2014-09-05  0:28   ` Pedro Alves
  0 siblings, 1 reply; 11+ messages in thread
From: Gareth McMullin @ 2014-09-04 22:37 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb

Hi

On Mon, Sep 1, 2014 at 8:57 PM, Joel Brobecker <brobecker@adacore.com> wrote:
> Everything works pretty well, except that we noticed that GDB stops
> 2 instructions after the variable update, rather than at the next
> instruction.

I get the same behaviour on the Black Magic Probe on ARMv7-M.
The DWT halts the core on the instruction after the address match.

> The question then becomes: Is QEMU behaving correctly, in which
> case we should be modifying GDB, or is QEMU signaling GDB too
> late, in which case we should be fixing QEMU?

What QEMU is doing is the same as the hardware DWT block, so I'd
say this is the correct behaviour.  The fix should be in GDB.  Working
around this in QEMU would leave all hardware based implementations
broken.

Regards,
Gareth

-- 
Black Sphere Technologies Ltd.

Web: www.blacksphere.co.nz
Mobile: +64 27 777 2182
Tel: +64 9 478 8885
Skype: gareth.mcmullin
LinkedIn: http://nz.linkedin.com/in/gsmcmullin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about ARM watchpoints
  2014-09-04 22:37 ` Gareth McMullin
@ 2014-09-05  0:28   ` Pedro Alves
  2014-09-05  3:51     ` Joel Brobecker
  0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2014-09-05  0:28 UTC (permalink / raw)
  To: Gareth McMullin, Joel Brobecker; +Cc: gdb

On 09/04/2014 11:36 PM, Gareth McMullin wrote:
> Hi
> 
> On Mon, Sep 1, 2014 at 8:57 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> Everything works pretty well, except that we noticed that GDB stops
>> 2 instructions after the variable update, rather than at the next
>> instruction.
> 
> I get the same behaviour on the Black Magic Probe on ARMv7-M.
> The DWT halts the core on the instruction after the address match.
> 
>> The question then becomes: Is QEMU behaving correctly, in which
>> case we should be modifying GDB, or is QEMU signaling GDB too
>> late, in which case we should be fixing QEMU?
> 
> What QEMU is doing is the same as the hardware DWT block, so I'd
> say this is the correct behaviour.  The fix should be in GDB.  Working
> around this in QEMU would leave all hardware based implementations
> broken.

So sounds like this line should be skipped on ARMv7-M:

  arm-tdep.c:  set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1);

Could you try removing it?

But then I wonder why we never heard of this before.  Are there
ARMv7-M's that behave differently?  And what about ARMv6-M ?

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about ARM watchpoints
  2014-09-05  0:28   ` Pedro Alves
@ 2014-09-05  3:51     ` Joel Brobecker
  2014-09-05  8:17       ` Pedro Alves
  2014-09-12 12:22       ` Yao Qi
  0 siblings, 2 replies; 11+ messages in thread
From: Joel Brobecker @ 2014-09-05  3:51 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Gareth McMullin, gdb

> So sounds like this line should be skipped on ARMv7-M:
> 
>   arm-tdep.c:  set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1);
> 
> Could you try removing it?
> 
> But then I wonder why we never heard of this before.  Are there
> ARMv7-M's that behave differently?  And what about ARMv6-M ?

That's what I wanted to try too, and will do soon. As to why we never
heard of this before - the only affirmative answer would be from someone
better able to undertand the docs than me.  But here's a wild guess: the
fact that GDB stopped one instruction too late is invisible to the user
99% of the time. What triggered me seeing it was a change in code
generation which caused the update to be at the penultimate instruction
of a function. I wouldn't have seen it if the update was anywhere before
that.

-- 
Joel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about ARM watchpoints
  2014-09-05  3:51     ` Joel Brobecker
@ 2014-09-05  8:17       ` Pedro Alves
  2014-09-05  9:46         ` Pedro Alves
  2014-09-12 12:22       ` Yao Qi
  1 sibling, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2014-09-05  8:17 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Gareth McMullin, gdb

On 09/05/2014 04:51 AM, Joel Brobecker wrote:
>> So sounds like this line should be skipped on ARMv7-M:
>>
>>   arm-tdep.c:  set_gdbarch_have_nonsteppable_watchpoint (gdbarch, 1);
>>
>> Could you try removing it?
>>
>> But then I wonder why we never heard of this before.  Are there
>> ARMv7-M's that behave differently?  And what about ARMv6-M ?
> 
> That's what I wanted to try too, and will do soon. As to why we never
> heard of this before - the only affirmative answer would be from someone
> better able to undertand the docs than me.  But here's a wild guess: the
> fact that GDB stopped one instruction too late is invisible to the user
> 99% of the time. What triggered me seeing it was a change in code
> generation which caused the update to be at the penultimate instruction
> of a function. I wouldn't have seen it if the update was anywhere before
> that.

Ah, indeed.

Hmm.  Sounds like we need a test to catch this.  :-)

Most preferably, a portable one, which may be tricky.

I've written something like this before:

volatile int i1;
volatile int i2;
volatile int i3;

int main ()
{
	i1 = 1;
	i2 = 2;
	i3 = 3;
}

assuming each write would be one insn, but, then that turned
out to be a wrong assumption, for ARM even, I think.

Hmm...  I had an idea.  How about:

volatile int global;

set_global (int val)
{
	global = val;
}

main ()
{
   set_global (1);
   set_global (2);
}

#01 - run to main
#02 - set a _software_ watchpoint on i1 (w/ hw watchpoints force-disabled)
#03 - continue
#04 - (GDB single-steps the inferior, and checks the watchpoint's
     value at each step.)
#05 - watchpoint triggers
#06 - the PC now points to the instruction right after the instruction that
     actually caused the memory write.  So this is the address a hardware
     watchpoint should present the stop to the user too.  Store the PC address.
#07 - delete the software watchpoint
#08 - set a _hardware_ watchpoint on i1.
#09 - continue
#10 - hardware watchpoint triggers.  PC should point to the address
      stored in #6.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about ARM watchpoints
  2014-09-05  8:17       ` Pedro Alves
@ 2014-09-05  9:46         ` Pedro Alves
  2014-09-16 11:18           ` Pedro Alves
  0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2014-09-05  9:46 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Gareth McMullin, gdb

On 09/05/2014 09:17 AM, Pedro Alves wrote:

> Hmm.  Sounds like we need a test to catch this.  :-)
> 
...

> Hmm...  I had an idea.  How about:
> 
> volatile int global;
> 
> set_global (int val)
> {
> 	global = val;
> }
> 
> main ()
> {
>    set_global (1);
>    set_global (2);
> }
> 
> #01 - run to main
> #02 - set a _software_ watchpoint on i1 (w/ hw watchpoints force-disabled)
> #03 - continue
> #04 - (GDB single-steps the inferior, and checks the watchpoint's
>      value at each step.)
> #05 - watchpoint triggers
> #06 - the PC now points to the instruction right after the instruction that
>      actually caused the memory write.  So this is the address a hardware
>      watchpoint should present the stop to the user too.  Store the PC address.
> #07 - delete the software watchpoint
> #08 - set a _hardware_ watchpoint on i1.
> #09 - continue
> #10 - hardware watchpoint triggers.  PC should point to the address
>       stored in #6.

Like this?  This "hopefully" is failing on ARMv7-M currently.

Passes on x86-64 native and gdbserver.

Also uploaded to:

  git@github.com:palves/gdb.git palves/hw_watch_test

----------->8-----------
From e4035b618fdca66038bae8fd51b14cc2913f6298 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 5 Sep 2014 10:29:08 +0100
Subject: [PATCH] Add test to make sure GDB knows which "kind" of watchpoint
 the target has

This adds a test that makes sure GDB knows whether the target has
continuable, or non-continuable watchpoints.

That is, the test confirms that GDB presents a watchpoint value change
at the first instruction right after the instruction that changes
memory.

gdb/testsuite/ChangeLog:

2014-09-05  Pedro Alves  <palves@redhat.com>

	* gdb.base/watchpoint-stops-at-right-insn.c: New file.
	* gdb.base/watchpoint-stops-at-right-insn.exp: New file.
---
 .../gdb.base/watchpoint-stops-at-right-insn.c      |  33 ++++
 .../gdb.base/watchpoint-stops-at-right-insn.exp    | 177 +++++++++++++++++++++
 2 files changed, 210 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.c
 create mode 100644 gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.exp

diff --git a/gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.c b/gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.c
new file mode 100644
index 0000000..005d63f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.c
@@ -0,0 +1,33 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 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/>.  */
+
+volatile int global;
+
+void
+set_global (int val)
+{
+  global = val;
+}
+
+int
+main (void)
+{
+  set_global (1);
+  set_global (2);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.exp b/gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.exp
new file mode 100644
index 0000000..9ad6080
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.exp
@@ -0,0 +1,177 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2014 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/>.
+
+# This file is part of the gdb testsuite.
+
+# Test that GDB presents a hardware watchpoint stop at the first
+# instruction right after the instruction that changes memory.
+#
+# Some targets trigger a hardware watchpoint after the instruction
+# that wrote memory executes, thus with the memory already changed and
+# the PC pointing to the instruction after the instruction that wrote
+# to memory.  These targets are said to have "continuable"
+# watchpoints, referring to the fact that to make progress after the
+# watchpoint triggers, GDB just needs to continue the target.
+#
+# Other targets trigger a hardware watchpoint at the instruction which
+# has attempted to write to the piece of memory under control of the
+# watchpoint, with the instruction actually not executed yet.  To be
+# able to check whether the watched value changed, GDB needs to
+# complete the memory write, single-stepping the target once.  These
+# targets are said to have "non-continuable" watchpoints.
+#
+# This test makes sure that GDB knows which kind of watchpoint the
+# target has, using this sequence of steps:
+#
+# 1 - run to main
+#
+# 2 - set a software watchpoint
+#
+# 3 - continue until watchpoint triggers
+#
+# 4 - the PC now points to the instruction right after the instruction
+#     that actually caused the memory write.  So this is the address a
+#     hardware watchpoint should present the stop to the user too.
+#     Store the PC address.
+#
+# 5 - replace the software watchpoint by a hardware watchpoint
+#
+# 6 - continue until hardware watchpoint triggers
+#
+# 7 - the PC must point to the same address the software watchpoint
+#     triggered at.
+#
+# If the target has continuable watchpoints, but GDB thinks it has
+# non-continuable watchpoints, GDB will stop the inferior two
+# instructions after the watched value change, rather than at the next
+# instruction.
+#
+# If the target has non-continuable watchpoints, while GDB thinks it
+# has continuable watchpoints, GDB will see a watchpoint trigger,
+# notice no value changed, and immediatly continue the target.  Now,
+# either the target manages to step-over the watchpoint transparently,
+# and GDB thus fails to present to value change to the user, or, the
+# watchpoint will keep re-triggering, with the program never making
+# any progress.
+
+standard_testfile
+
+# No use testing this if we can't use hardware watchpoints.
+if {[target_info exists gdb,no_hardware_watchpoints]} {
+    return -1
+}
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+    untested ${testfile}.exp
+    return -1
+}
+
+if { ![runto main] } then {
+    fail "run to main"
+    return
+}
+
+# Get the current PC.  TEST is used as test prefix.
+
+proc get_pc {test} {
+    global hex gdb_prompt
+
+    set addr ""
+    gdb_test_multiple "p /x \$pc" "$test" {
+	-re " = ($hex).*$gdb_prompt $" {
+	    set addr $expect_out(1,string)
+	    pass "$test"
+	}
+    }
+
+    return $addr
+}
+
+# So we get an immediate warning/error if the target doesn't support a
+# given watchpoint type.
+gdb_test_no_output "set breakpoint always-inserted on"
+
+set hw_watchpoints_supported 0
+
+set test "set probe hw watchpoint"
+gdb_test_multiple "watch global" $test {
+    -re "You may have requested too many.*$gdb_prompt $" {
+	pass $test
+    }
+    -re "Target does not support.*$gdb_prompt $" {
+	pass $test
+    }
+    -re "$gdb_prompt $" {
+	pass $test
+	set hw_watchpoints_supported 1
+    }
+}
+
+if {!$hw_watchpoints_supported} {
+    unsupported "no hw watchpoints support"
+    return
+}
+
+delete_breakpoints
+
+proc test {always_inserted} {
+    global srcfile binfile
+
+    with_test_prefix "always-inserted $always_inserted" {
+
+	clean_restart $binfile
+
+	if { ![runto main] } then {
+	    fail "run to main"
+	    return
+	}
+
+	# Force use of software watchpoints.
+	gdb_test_no_output "set can-use-hw-watchpoints 0"
+
+	gdb_test "watch global" \
+	    "Watchpoint .*: global" \
+	    "set software watchpoint on global variable"
+
+	gdb_test "continue" \
+	    "Watchpoint .*: global.*Old value = 0.*New value = 1.*set_global \\(val=1\\).*$srcfile.*" \
+	    "software watchpoint triggers"
+
+	set sw_watch_pc [get_pc "get sw watchpoint PC"]
+
+	delete_breakpoints
+
+	# Allow hardware watchpoints again.
+	gdb_test_no_output "set can-use-hw-watchpoints 1"
+
+	gdb_test "watch global" \
+	    "Hardware watchpoint .*: global" \
+	    "set hardware watchpoint on global variable"
+
+	gdb_test "continue" \
+	    "Hardware watchpoint .*: global.*Old value = 1.*New value = 2.*set_global \\(val=2\\).*$srcfile.*" \
+	    "hardware watchpoint triggers"
+
+	set hw_watch_pc [get_pc "get hw watchpoint PC"]
+
+	gdb_assert {$sw_watch_pc == $sw_watch_pc} "hw watchpoint stops at right instruction"
+    }
+}
+
+foreach always_inserted {"off" "on" } {
+    test $always_inserted
+}
-- 
1.9.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about ARM watchpoints
  2014-09-05  3:51     ` Joel Brobecker
  2014-09-05  8:17       ` Pedro Alves
@ 2014-09-12 12:22       ` Yao Qi
  2014-09-15 13:02         ` Joel Brobecker
  1 sibling, 1 reply; 11+ messages in thread
From: Yao Qi @ 2014-09-12 12:22 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Pedro Alves, Gareth McMullin, gdb

Joel Brobecker <brobecker@adacore.com> writes:

Joel,
I got the same result as yours on qemu.  We have arm board, but the
kernel is too old to support hardware watchpoint, so unable to see how
it behaves on real hardware.

> That's what I wanted to try too, and will do soon. As to why we never
> heard of this before - the only affirmative answer would be from someone
> better able to undertand the docs than me.  But here's a wild guess: the
> fact that GDB stopped one instruction too late is invisible to the user
> 99% of the time. What triggered me seeing it was a change in code
> generation which caused the update to be at the penultimate instruction
> of a function. I wouldn't have seen it if the update was anywhere before
> that.

This (GDB stops 2 instructions after the variable update) causes fails
in two fails for arm-none-eabi target on qemu,

  FAIL: gdb.base/recurse.exp: continue to first instance watchpoint, second time
  FAIL: gdb.base/recurse.exp: continue to second instance watchpoint, second time

GDB outputs:

continue^M
Continuing.^M
Hardware watchpoint 3: b^M
^M
Old value = 5^M
New value = 120^M
recurse (a=5) at ../../../../git/gdb/testsuite/gdb.base/recurse.c:21^M
21      }^M
(gdb) FAIL: gdb.base/recurse.exp: continue to second instance watchpoint, second time

while the test expects the program stops at the line "return":

gdb_test "continue" \
	    "Continuing.*\[Ww\]atchpoint.*: b.*Old value = 5.*New value = 120.*return.*" \
	    "continue to second instance watchpoint, second time"

19        b *= recurse (a - 1);
   0x000001ca <+26>:    ldr     r3, [r7, #4]
   0x000001cc <+28>:    subs    r3, #1
   0x000001ce <+30>:    adds    r0, r3, #0
   0x000001d0 <+32>:    bl      0x1b0 <recurse>
   0x000001d4 <+36>:    adds    r2, r0, #0
   0x000001d6 <+38>:    ldr     r3, [r7, #12]
   0x000001d8 <+40>:    muls    r3, r2
   0x000001da <+42>:    str     r3, [r7, #12]  <-- memory store

20        return b;
   0x000001dc <+44>:    ldr     r3, [r7, #12]

21      }
=> 0x000001de <+46>:    adds    r0, r3, #0
   0x000001e0 <+48>:    mov     sp, r7
   0x000001e2 <+50>:    add     sp, #16

As we can see, if GDB stops one instruction after the variable update
(0x000001dc), the source line is correct, and fails will go away.

These two fails were reported on 2012-09 in codesourcery, but we didn't
analyze them until I start to fix these fails recently.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about ARM watchpoints
  2014-09-12 12:22       ` Yao Qi
@ 2014-09-15 13:02         ` Joel Brobecker
  0 siblings, 0 replies; 11+ messages in thread
From: Joel Brobecker @ 2014-09-15 13:02 UTC (permalink / raw)
  To: Yao Qi; +Cc: Pedro Alves, Gareth McMullin, gdb

Thank you all, for your answers. I finally sent a patch in to avoid
the extra single-step:
https://www.sourceware.org/ml/gdb-patches/2014-09/msg00498.html

-- 
Joel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about ARM watchpoints
  2014-09-05  9:46         ` Pedro Alves
@ 2014-09-16 11:18           ` Pedro Alves
  2014-09-16 12:13             ` Joel Brobecker
  0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2014-09-16 11:18 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Gareth McMullin, gdb

On 09/05/2014 10:46 AM, Pedro Alves wrote:

> Like this?  This "hopefully" is failing on ARMv7-M currently.
> 
> Passes on x86-64 native and gdbserver.
> 
> Also uploaded to:
> 
>   git@github.com:palves/gdb.git palves/hw_watch_test

Hey guys, I guess I should just push it in (and post it to gdb-patches) ?

> 
> ----------->8-----------
> From e4035b618fdca66038bae8fd51b14cc2913f6298 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 5 Sep 2014 10:29:08 +0100
> Subject: [PATCH] Add test to make sure GDB knows which "kind" of watchpoint
>  the target has
> 
> This adds a test that makes sure GDB knows whether the target has
> continuable, or non-continuable watchpoints.
> 
> That is, the test confirms that GDB presents a watchpoint value change
> at the first instruction right after the instruction that changes
> memory.
> 
> gdb/testsuite/ChangeLog:
> 
> 2014-09-05  Pedro Alves  <palves@redhat.com>
> 
> 	* gdb.base/watchpoint-stops-at-right-insn.c: New file.
> 	* gdb.base/watchpoint-stops-at-right-insn.exp: New file.
> ---
>  .../gdb.base/watchpoint-stops-at-right-insn.c      |  33 ++++
>  .../gdb.base/watchpoint-stops-at-right-insn.exp    | 177 +++++++++++++++++++++
>  2 files changed, 210 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.c
>  create mode 100644 gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.exp
> 
> diff --git a/gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.c b/gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.c
> new file mode 100644
> index 0000000..005d63f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.c
> @@ -0,0 +1,33 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2014 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/>.  */
> +
> +volatile int global;
> +
> +void
> +set_global (int val)
> +{
> +  global = val;
> +}
> +
> +int
> +main (void)
> +{
> +  set_global (1);
> +  set_global (2);
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.exp b/gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.exp
> new file mode 100644
> index 0000000..9ad6080
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/watchpoint-stops-at-right-insn.exp
> @@ -0,0 +1,177 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2014 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/>.
> +
> +# This file is part of the gdb testsuite.
> +
> +# Test that GDB presents a hardware watchpoint stop at the first
> +# instruction right after the instruction that changes memory.
> +#
> +# Some targets trigger a hardware watchpoint after the instruction
> +# that wrote memory executes, thus with the memory already changed and
> +# the PC pointing to the instruction after the instruction that wrote
> +# to memory.  These targets are said to have "continuable"
> +# watchpoints, referring to the fact that to make progress after the
> +# watchpoint triggers, GDB just needs to continue the target.
> +#
> +# Other targets trigger a hardware watchpoint at the instruction which
> +# has attempted to write to the piece of memory under control of the
> +# watchpoint, with the instruction actually not executed yet.  To be
> +# able to check whether the watched value changed, GDB needs to
> +# complete the memory write, single-stepping the target once.  These
> +# targets are said to have "non-continuable" watchpoints.
> +#
> +# This test makes sure that GDB knows which kind of watchpoint the
> +# target has, using this sequence of steps:
> +#
> +# 1 - run to main
> +#
> +# 2 - set a software watchpoint
> +#
> +# 3 - continue until watchpoint triggers
> +#
> +# 4 - the PC now points to the instruction right after the instruction
> +#     that actually caused the memory write.  So this is the address a
> +#     hardware watchpoint should present the stop to the user too.
> +#     Store the PC address.
> +#
> +# 5 - replace the software watchpoint by a hardware watchpoint
> +#
> +# 6 - continue until hardware watchpoint triggers
> +#
> +# 7 - the PC must point to the same address the software watchpoint
> +#     triggered at.
> +#
> +# If the target has continuable watchpoints, but GDB thinks it has
> +# non-continuable watchpoints, GDB will stop the inferior two
> +# instructions after the watched value change, rather than at the next
> +# instruction.
> +#
> +# If the target has non-continuable watchpoints, while GDB thinks it
> +# has continuable watchpoints, GDB will see a watchpoint trigger,
> +# notice no value changed, and immediatly continue the target.  Now,
> +# either the target manages to step-over the watchpoint transparently,
> +# and GDB thus fails to present to value change to the user, or, the
> +# watchpoint will keep re-triggering, with the program never making
> +# any progress.
> +
> +standard_testfile
> +
> +# No use testing this if we can't use hardware watchpoints.
> +if {[target_info exists gdb,no_hardware_watchpoints]} {
> +    return -1
> +}
> +
> +if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
> +    untested ${testfile}.exp
> +    return -1
> +}
> +
> +if { ![runto main] } then {
> +    fail "run to main"
> +    return
> +}
> +
> +# Get the current PC.  TEST is used as test prefix.
> +
> +proc get_pc {test} {
> +    global hex gdb_prompt
> +
> +    set addr ""
> +    gdb_test_multiple "p /x \$pc" "$test" {
> +	-re " = ($hex).*$gdb_prompt $" {
> +	    set addr $expect_out(1,string)
> +	    pass "$test"
> +	}
> +    }
> +
> +    return $addr
> +}
> +
> +# So we get an immediate warning/error if the target doesn't support a
> +# given watchpoint type.
> +gdb_test_no_output "set breakpoint always-inserted on"
> +
> +set hw_watchpoints_supported 0
> +
> +set test "set probe hw watchpoint"
> +gdb_test_multiple "watch global" $test {
> +    -re "You may have requested too many.*$gdb_prompt $" {
> +	pass $test
> +    }
> +    -re "Target does not support.*$gdb_prompt $" {
> +	pass $test
> +    }
> +    -re "$gdb_prompt $" {
> +	pass $test
> +	set hw_watchpoints_supported 1
> +    }
> +}
> +
> +if {!$hw_watchpoints_supported} {
> +    unsupported "no hw watchpoints support"
> +    return
> +}
> +
> +delete_breakpoints
> +
> +proc test {always_inserted} {
> +    global srcfile binfile
> +
> +    with_test_prefix "always-inserted $always_inserted" {
> +
> +	clean_restart $binfile
> +
> +	if { ![runto main] } then {
> +	    fail "run to main"
> +	    return
> +	}
> +
> +	# Force use of software watchpoints.
> +	gdb_test_no_output "set can-use-hw-watchpoints 0"
> +
> +	gdb_test "watch global" \
> +	    "Watchpoint .*: global" \
> +	    "set software watchpoint on global variable"
> +
> +	gdb_test "continue" \
> +	    "Watchpoint .*: global.*Old value = 0.*New value = 1.*set_global \\(val=1\\).*$srcfile.*" \
> +	    "software watchpoint triggers"
> +
> +	set sw_watch_pc [get_pc "get sw watchpoint PC"]
> +
> +	delete_breakpoints
> +
> +	# Allow hardware watchpoints again.
> +	gdb_test_no_output "set can-use-hw-watchpoints 1"
> +
> +	gdb_test "watch global" \
> +	    "Hardware watchpoint .*: global" \
> +	    "set hardware watchpoint on global variable"
> +
> +	gdb_test "continue" \
> +	    "Hardware watchpoint .*: global.*Old value = 1.*New value = 2.*set_global \\(val=2\\).*$srcfile.*" \
> +	    "hardware watchpoint triggers"
> +
> +	set hw_watch_pc [get_pc "get hw watchpoint PC"]
> +
> +	gdb_assert {$sw_watch_pc == $sw_watch_pc} "hw watchpoint stops at right instruction"
> +    }
> +}
> +
> +foreach always_inserted {"off" "on" } {
> +    test $always_inserted
> +}
> 


Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about ARM watchpoints
  2014-09-16 11:18           ` Pedro Alves
@ 2014-09-16 12:13             ` Joel Brobecker
  2014-09-16 13:21               ` Pedro Alves
  0 siblings, 1 reply; 11+ messages in thread
From: Joel Brobecker @ 2014-09-16 12:13 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Gareth McMullin, gdb

> > Like this?  This "hopefully" is failing on ARMv7-M currently.
> > 
> > Passes on x86-64 native and gdbserver.
> > 
> > Also uploaded to:
> > 
> >   git@github.com:palves/gdb.git palves/hw_watch_test
> 
> Hey guys, I guess I should just push it in (and post it to gdb-patches) ?

Sorry! I forgot about this clever little testcase. Agreed!

-- 
Joel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: question about ARM watchpoints
  2014-09-16 12:13             ` Joel Brobecker
@ 2014-09-16 13:21               ` Pedro Alves
  0 siblings, 0 replies; 11+ messages in thread
From: Pedro Alves @ 2014-09-16 13:21 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Gareth McMullin, gdb

On 09/16/2014 01:13 PM, Joel Brobecker wrote:
>>> Like this?  This "hopefully" is failing on ARMv7-M currently.
>>>
>>> Passes on x86-64 native and gdbserver.
>>>
>>> Also uploaded to:
>>>
>>>   git@github.com:palves/gdb.git palves/hw_watch_test
>>
>> Hey guys, I guess I should just push it in (and post it to gdb-patches) ?
> 
> Sorry! I forgot about this clever little testcase. Agreed!

No worries.  :-)  Pushed now.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-09-16 13:21 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-01  8:57 question about ARM watchpoints Joel Brobecker
2014-09-04 22:37 ` Gareth McMullin
2014-09-05  0:28   ` Pedro Alves
2014-09-05  3:51     ` Joel Brobecker
2014-09-05  8:17       ` Pedro Alves
2014-09-05  9:46         ` Pedro Alves
2014-09-16 11:18           ` Pedro Alves
2014-09-16 12:13             ` Joel Brobecker
2014-09-16 13:21               ` Pedro Alves
2014-09-12 12:22       ` Yao Qi
2014-09-15 13:02         ` Joel Brobecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).