[Lldb-commits] [PATCH] FreeBSD threaded inferior support

[Ed Maste]

================
Comment at: source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp:148
@@ +147,3 @@
+
+    int resume_signal = 0;
+
----------------
John Wolfe wondered if this should be LLDB_INVALID_SIGNAL_NUMBER instead of 0.

In this case resume_signal is passed to m_monitor->SingleStep() or ->Resume() on line 173/175; these are implemented in ProcessMonitor::SingleStep() in source/Plugins/Process/FreeBSD/ProcessMonitor.cpp and are FreeBSD specific.

>From PTRACE(2):

     PT_STEP       The traced process is single stepped one instruction.  The
                   addr argument should be passed (caddr_t)1.  The data argu-
                   ment provides a signal number to be delivered to the traced
                   process as it resumes execution, or 0 if no signal is to be
                   sent.

So 0 in this case will be interpreted as "no signal."  Agreed that this isn't necessarily clear though.

We could pass LLDB_INVALID_SIGNAL_NUMBER in and map that to 0 in SingleStep() and Resume(), but I'm not sure it's worth it.  At least a comment is in order though.

================
Comment at: source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp:167
@@ +166,3 @@
+        m_monitor->ThreadSuspend(*t_pos, true);
+        do_step = true;
+    }
----------------
John asks
> What is the function of the "do_step = true" in the "m_suspended_tids" loop?

Good catch; I think I had the impression PT_CONTINUE would unsuspend all threads. Looking now though it doesn't look like it should be there, will test without it.

(It could have been a cut-and-pasteo, too.)


================
Comment at: source/Plugins/Process/FreeBSD/ProcessMonitor.cpp:1080
@@ +1079,3 @@
+        return 0;
+    tids = (lwpid_t *)malloc(tdcnt * sizeof(*tids));
+    if (tids == NULL)
----------------
John asks
> If it "safe" to simply "return 0" if the malloc() or PTRACE() calls fail.  Acknowledged that if malloc() fails, lldb is about to fall over.   Is there any instance when this PTRACE() call can fail when there are known threads (tdcnt > 0) and the correct amount of space has been malloc'ed.

I don't think there's any case other than catastrophic failure (e.g., the PID has disappeared, where it would return ESRCH), where things will completely fail anyway.

> It as nit-picking, but should these not be covered by assertions?

I don't think they're assertions, since they're not really invariants known at compile time that we're trying to enforce.  If the failure could actually happen I think it's the responsibility of LLDB infrastructure to handle a failure returned out of ProcessFreeBSD::UpdateThreadList (by reporting to the user, e.g.)



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