[Lldb-commits] [PATCH] D24610: LLDB Arm Watchpoints: Use single hardware watchpoint slot to watch multiple bytes where possible

[Muhammad Omair Javaid via lldb-commits]

omjavaid updated this revision to Diff 72723.
omjavaid added a comment.

Give this approach a rethink I dont see a lot of problems with this final implementation unless it fails on other architectures.
We are already hacking our way to have these byte selection watchpoints working in existing code. New code seems to be improving the hack in my opinion.

Let me explain what I am doing and may be you can provide your suggestion and feedback.

Watchpoint Install => Register watchpoint into hardware watchpoint register cache
Watchpoint Enable => Enable in cache and write registers using ptrace interface.

Ideally we should be able to install/uninstall watchpoints in cache and then enable them all on a resume.
In case of arm If a watchpoint is hit we should be able to disable that watchpoint. 
Step over watchpoint instruction and then re-enable the watchpoint.
Our existing implementation will require a lot of changes to do that so thats why here is what i am doing.

SetHardwareWatchpoint => Performs Install and Enable

- If a new watchpoint slot is going to be used we Install and enable.
- For new watchpoint we should be able to complete both Install and or we report an error.
- If a duplicate slot is going to be used we Install and enable if required.
- Install means updating size if needed plus updating ref count.
- Enable means updating registers if size was updated.

ClearHardwareWatchpoint

- Disable and uinstall watchpoint means
- Decrement ref count and clear hardware watchpoint regsiters.
- Advantage of keeping ref counts is:
- If refcount is greater than zero then SetHardwareWatchpoint cannot use this slot for a new watchpoint (new address).
- But SetHardwareWatchpoint can be use this slot to install duplicate watchpoints (same address but different byte or word)

ClearAllHardwareWatchpoint -- Just clear the whole watchpoint cache and call SetHardwareWatchpoint for all available watchpoints.

NativeThreadLinux:

  On Watchpoint Remove -> Invalidate watchpoint cache
  On Resume - > Re-validate watchpoints by creating a new cache and re-enabling all watchpoints

So this fixes our step-over issue and also preserves watchpoint slot if it is being used by multiple watchpoints.

Can you think of any scenarios which might fail for this approach?


https://reviews.llvm.org/D24610

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/NativeThreadLinux.cpp
  source/Plugins/Process/Linux/NativeThreadLinux.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D24610.72723.patch
Type: text/x-patch
Size: 18234 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160927/d8b153b2/attachment-0001.bin>