[Lldb-commits] [PATCH] D25057: Fix ARM/AArch64 Step-Over watchpoint issue remove provision for duplicate watchpoints

[Pavel Labath via lldb-commits]

[phabricator seems to be down, so I'll reply here]

Thank you for looking into this.

Hmm... I think we can consider this an improvement over the status quo, but
it's not going to solve the problem in all cases. E.g., on aarch64, a
single stp instruction with neon registers can access a block of memory of
size 32 (on arm a similar thing can be achieved with a push instruction
with a large set of registers, and I believe there are some cache
management instructions that can access even larger blocks of memory),
which can trigger watchpoints in multiple slots even if they are watching
different regions of memory. This patch will not handle that (in fact, the
problem cannot be fully solved from within nativeregistercontext classes).

The way I was considering handling this is to change the client's behavior
when stepping over the watchpoint -- instead of just disabling the
watchpoint that got reported as hit, we would disable all watchpoints and
re-enable them after the step is complete. This would handle all cases and
you would still be able to set two watchpoints on the same memory region if
you wanted to. The only downside I see there is that we would not be able
to report the fact that multiple watchpoints got triggered, but we don't
have that ability now anyway, and if we really wanted to, we could later
change that code to do some kind of a loop disabling watchpoints one by one
(but I think that's going to happen soon).

So, if you really want to do it this way, I think we can make that happen,
but I would encourage you to try the holistic approach first.

Greg, how this idea sound to you ?


On 28 September 2016 at 17:26, Muhammad Omair Javaid <
omair.javaid at linaro.org> wrote:

> omjavaid created this revision.
> omjavaid added a reviewer: labath.
> omjavaid added a subscriber: lldb-commits.
> Herald added subscribers: samparker, srhines, danalbert, tberghammer,
> rengolin, aemerson.
>
> On ARM Linux targets  watchpoints are reported by PTrace before the
> instruction causing watchpoint to trigger is executed.
> This means we cannot step-over watchpoint causing instruction as long as
> there is watchpoint installed on that location.
> For this reason we cannot have multiple watchpoint slots watching same
> region word/byte/halfword etc as we have to disable watchpoint for stepping
> over and only watchpoint index reported by lldb-server is disabled.
> Similarly we cannot keep a single hardware watchpoint slot referring to
> multiple watchpoints in LLDB. This actually means that hware watchpoint
> indexes are exclusively assigned to a watchpoint and cannot be shared by
> other watchpoint. Also no other watchpoint should watch same address
> already watched by any other index.
>
> More discussion on this can be found here:
> https://reviews.llvm.org/D24610
>
> This patch fixes issue with stepping over watchpoint by removing the
> provision to enable multiple watchpoints referring to same hardware
> watchpoint slot (same HW index). Also we restrict one watchpoint per word
> aligned address so duplication of address will not be allowed to avoid any
> step-over failures.
>
> I have also attached a test-case that tests this scenario.
>
> https://reviews.llvm.org/D25057
>
> Files:
>   packages/Python/lldbsuite/test/functionalities/watchpoint/
> multi_watchpoint_slots/Makefile
>   packages/Python/lldbsuite/test/functionalities/watchpoint/
> multi_watchpoint_slots/TestWatchpointMultipleSlots.py
>   packages/Python/lldbsuite/test/functionalities/watchpoint/
> multi_watchpoint_slots/main.c
>   source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
>   source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160929/9f611b8f/attachment.html>