aleksandr.urakov marked 3 inline comments as done.
aleksandr.urakov added inline comments.
================
Comment at:
lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp:82
-bool RegisterContextWindows::ClearHardwareBreakpoint(uint32_t hw_idx) {
- return false;
-}
+ if (!size || size > 8 || size & (size - 1))
+ return false;
----------------
labath wrote:
> aleksandr.urakov wrote:
> > zturner wrote:
> > > amccarth wrote:
> > > > Clever! It took me a minute or two to figure out what the point of
> > > > that was checking. Perhaps a comment to explain?
> > > Isn't this equivalent to:
> > >
> > > ```
> > > switch (size)
> > > {
> > > case 1:
> > > case 2:
> > > case 4:
> > > case 8:
> > > break;
> > > default:
> > > return false;
> > > }
> > > ```
> > >
> > > ? That definitely seems much clearer.
> > >
> > > I'm also pretty sure that on x86 you can't add a 64-bit watch, So you'd
> > > have to do something different depending on the target bitness if you
> > > want this to be correct for x86.
> > Yes, it is equivalent, I've chosen the previous form due to its less
> > verbosity. But you are right, clearance is better (especially after adding
> > the architecture check). Fixed it, thanks!
> .... or, you could just use `llvm::isPowerOf2_32` from `MathExtras.h`.
I didn't know about the such function, thanks! But I think that Zachary's
approach is better in exactly this case (taking in account the bitness check).
================
Comment at:
lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.cpp:87-89
+#if defined(_M_AMD64)
+ case 8:
+#endif
----------------
In the old plugin a required register context is created based on the target
architecture check and the bitness of LLDB, but cross-targets (e.g. WoW64) are
not supported, and even the type of `m_context` is determined statically. So we
can safely determine the max watchpoint size statically too.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67168/new/
https://reviews.llvm.org/D67168
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
