<div dir="ltr"><div>[phabricator seems to be down, so I'll reply here]</div><div><br></div>Thank you for looking into this.<div><br></div><div>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).</div><div><br></div><div>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).</div><div><br></div><div>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.</div><div><br></div><div>Greg, how this idea sound to you ?</div><div><br></div><div class="gmail_extra"><br><div class="gmail_quote">On 28 September 2016 at 17:26, Muhammad Omair Javaid <span dir="ltr"><<a href="mailto:omair.javaid@linaro.org" target="_blank">omair.javaid@linaro.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">omjavaid created this revision.<br>
omjavaid added a reviewer: labath.<br>
omjavaid added a subscriber: lldb-commits.<br>
Herald added subscribers: samparker, srhines, danalbert, tberghammer, rengolin, aemerson.<br>
<br>
On ARM Linux targets  watchpoints are reported by PTrace before the instruction causing watchpoint to trigger is executed.<br>
This means we cannot step-over watchpoint causing instruction as long as there is watchpoint installed on that location.<br>
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.<br>
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.<br>
<br>
More discussion on this can be found here:<br>
<a href="https://reviews.llvm.org/D24610" rel="noreferrer" target="_blank">https://reviews.llvm.org/D2461<wbr>0</a><br>
<br>
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.<br>
<br>
I have also attached a test-case that tests this scenario.<br>
<br>
<a href="https://reviews.llvm.org/D25057" rel="noreferrer" target="_blank">https://reviews.llvm.org/D2505<wbr>7</a><br>
<br>
Files:<br>
  packages/Python/lldbsuite/test<wbr>/functionalities/watchpoint/<wbr>multi_watchpoint_slots/<wbr>Makefile<br>
  packages/Python/lldbsuite/test<wbr>/functionalities/watchpoint/<wbr>multi_watchpoint_slots/TestWat<wbr>chpointMultipleSlots.py<br>
  packages/Python/lldbsuite/test<wbr>/functionalities/watchpoint/<wbr>multi_watchpoint_slots/main.c<br>
  source/Plugins/Process/Linux/N<wbr>ativeRegisterContextLinux_arm.<wbr>cpp<br>
  source/Plugins/Process/Linux/N<wbr>ativeRegisterContextLinux_arm6<wbr>4.cpp<br>
<br>
</blockquote></div><br></div></div>