omjavaid added inline comments.
================
Comment at: source/Target/Target.cpp:714
@@ -713,2 +713,3 @@
{
uint32_t num_current_watchpoints =
target->GetWatchpointList().GetSize();
+ if (num_supported_hardware_watchpoints == 0)
----------------
clayborg wrote:
> This logic isn't necessarily correct. If we have the ability to watch N bytes
> at a time with a single hardware watchpoint, we might have 1 hardware
> watchpoint that is able to watch multiple things. So for code like:
>
> ```
> char buffer[8] = ...;
> ```
>
> then we watch "buffer[1]" and "buffer[7]", we could actually have 2
> watchpoints but only use 1 hardware watchpoint. We really should be allowing
> each process plug-in to try and set the watchpoint and return the correct
> error instead of generically trying to catch _anything_ at the target level.
> So it seems like this code should be removed and moved into RegisterContext
> and allow each register context plug-in to respond correctly as only the it
> will know what can be done.
>
I am seeing this a little different. GetWatchpointSupportInfo has to provide
information on number of watchpoint resources a target supports.
We should only fail a watchpoint on the basis of GetWatchpointSupportInfo if it
returns zero that is for a particular hardware we have no watchpoint support.
For the case where we want to utilize same hardware resource to watch multiple
watchpoints, we should try to manage that in register context where we can keep
track of watchpoints previously installed without getting the target backend
involved. First we can attempt to intall a watchpoint using an existing
resource and if that fails we can use a new hardware resource. Return failure
if watchpoint resources are used up. This way you can put multiple watchpoint
on same address without having to use up hardware watchpoint resources. I will
post this solution of arm and aarch64 soon.
================
Comment at: source/Target/Target.cpp:721
@@ +720,3 @@
+ }
+ else if (num_current_watchpoints >= num_supported_hardware_watchpoints)
+ {
----------------
I am going to remove this case from final commit as we may have more
watchpoints utilizing one or more hardware watchpoint resources.
http://reviews.llvm.org/D21164
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
