Re: [patch 2/4] Fix hw watchpoints: reordered / simultaneously hit [fixup #1]

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Mon, 12 Oct 2009 17:59:16 +0200, Jan Kratochvil wrote:
> This patch also changes the former sentence in the doc which I was not aware
> of before as internally x86 was violating this rule:
> 	Then @value{GDBN} calls @code{target_stopped_data_address} exactly
> 	once.
> But maybe if this rule is still valid linux_nat_stopped_data_address can be
> dropped and I should rework the patchset including this patch instead.

While thinking about it:

* GDB already was violating this rule calling it twice "if (debug_infrun)"
  (infrun.c line 2836) block of handle_inferior_event.

* GDB already was violating this rule calling it zero times both by
  "if (stepped_after_stopped_by_watchpoint)" (infrun.c line 3100) and in other
  cases of "return;" above this line.

* From a design point of view I do not find it right that the only function to
  query a value (target_stopped_data_address) also does + must reset it.

* The comment at update_watchpoint() describes that for modifying watchpoints
  GDB needs the LWP to be stopped and so remove+reinsert cannot hurt the LWP.
  remove+reinsert is now always done and the trigger must be reset on
  watchpoint remove so there is no need to reset the trigger earlier.

  It may start to be an issue only of someone optimizes update_watchpoint() so
  that it no longer does remove+reinsert, it would need to explicitely reset
  the trigger that time.

So I stand behind the patch to remove the trigger-reset from
target_stopped_data_address.  In current HEAD it is missing in
i386_stopped_data_address which can be visible as a bug in my patch 1/4
testcase.  The reset is present in s390_stopped_by_watchpoint - but for the
s390 it is wrong anyway as it is now in target_stopped_by_watchpoint while
gdbint.texinfo says it should be in target_stopped_data_address which is
missing on s390.


Thanks,
Jan