[Lldb-commits] [PATCH] FreeBSD watchpoint support

[John Wolfe]

  Responded inline to Ed Maste's comments.


================
Comment at: source/Plugins/Process/FreeBSD/ProcessMonitor.h:127
@@ +126,3 @@
+    /// This method is provided for use by RegisterContextFreeBSD derivatives.
+    /// FIXME: ???? The FreeBSD implementation of this function should use tid in order
+    ///        to enable support for debugging threaded programs.
----------------
Ed Maste wrote:
> I don't follow the FIXME comment - it is using the tid, no?
I patterned the methods after other FreeBSD register reads/writes which still have this FIXME comments in place.  Those other routines have been updated to use tid.  

I agree the FIXME is not relative here, but was a reminder to clean up the comments in the other register routines.   The FIXME should be removed from these 2 routines and I would be happy to remove the FIXMEs from the other routines that have been converted to use tid at the same time.

================
Comment at: source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_x86.cpp:113
@@ +112,3 @@
+
+#if defined(__FreeBSD__)
+    if (reg >= m_reg_info.first_dr)
----------------
Ed Maste wrote:
> Instead of the FreeBSD-specific case here, can we just put the logic into ::ReadRegisterValue, giving it visibility into our register numbering?
Not easily.   At present, all FreeBSD access to registers - GPR or FPR - are relative to their offset in that OS specific struct.   For the present, I have implemented the debug register reads and writes relative to the register offset in the "struct dbreg".  The debug registers dr0 - dr7 are at the end of the register enumeration. By the time of the ReadDebugRegisterValue call is made, the register number is expressed as an offset (in the set) and the offset for r15 matches the offset of dr0 (as currently implemented).

FreeBSD probably needs something akin to Linux's "struct UserArea", but maybe not quite as detailed.  But that may impact other register operations for FreeBSD.

If the current patch is acceptable as a temporary FreeBSD solution, we can discuss the changes needed to differentiate GPR registers from DB registers in FreeBSD's Read/WriteRegisterValue()




http://llvm-reviews.chandlerc.com/D2572