labath requested changes to this revision. labath added a comment. This revision now requires changes to proceed.
In https://reviews.llvm.org/D24610#553331, @omjavaid wrote: > This is a new version of what seems to me fully implementing functionality we > intend to have here. > > On a second thought nuking ClearHardwareWatchpoint function seems to be the > wrong approach here. I spent some time taking different approaches and it > turns out that if we do not ClearHardwareWatchpoint when back-end asks us to > remove it then we wont be able to step over watchpoints. On ARM targets we > have to first clear and then reinstall watchpoints to step over the > watchpoint instruction. > > On the other hand if we call > NativeRegisterContextLinux_arm::ClearHardwareWatchpoint then that watchpoint > stands removed if call is just to delete watch on one of the bytes. And if we > follow up with creating a new watchpoint on a different word the slot being > used may appear vaccant which is actually inconsistent behavior. > > So I have a new approach that does clear watchpoint registers if > NativeRegisterContextLinux_arm::ClearHardwareWatchpoint is called but we > still track reference counts by re-introducing refcount that I removed in my > last patch. This will mean that a follow up create may fail just because > there are still references to disabled watchpoint and watchpoint slots are > still not vaccant. I have made changes to the test to reflect this behaviour. > > Please comment if you have any reservation about this approach. Hmm.... I am indeed starting to have serious reservations about this. The more I think about this, the more it starts to look like a big hack. So, now ClearHardwareWatchpoint still maintains a refcount on the number of users of the watchpoint slot, but it disables the slot everytime the slot usage count is decremented ? (as opposed to when the refcount reaches zero). And this is supposed to be the reason that step-over watchpoint (a pretty critical piece of functionality) works ? And the reason why we are still able to do the watchpoints is that before a continue (but not before a single-step, because that would break the previous item) we nuke the watchpoint registers (and their reference counts) and start over? I am not convinced that having watchpoint slot sharing is important enough to balance the amount of technical debt this introduces. https://reviews.llvm.org/D24610 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
