[Lldb-commits] [PATCH] D12104: [NativeProcessLinux] Fix a bug in instruction-stepping over thread creation

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Wed Aug 19 02:57:35 PDT 2015


labath added inline comments.

================
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:1457
@@ -1453,3 +1456,3 @@
 
-        thread_sp = AddThread(pid);
+        thread_sp = std::static_pointer_cast<NativeThreadLinux>(AddThread(pid));
         assert (thread_sp.get() && "failed to create the tracking data for newly created inferior thread");
----------------
ovyalov wrote:
> Since AddThread is private method of NPL we can make it to return NativeThreadLinuxSP.
I've been wanting to do this as well, but I'd prefer to do this as a separate commit, if possible.

================
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:3056
@@ +3055,3 @@
+        if (step_result.Success())
+            SetState(eStateRunning, true);
+        return step_result;
----------------
ovyalov wrote:
> Should it be eStateStepping?
That's a good question. Previously, the state of the whole process was indeed set to eStateStepping, and I have (accidentally) failed to preserve that while refactoring.

However, now that I think about it, this had the problem that the state of the process was nondeterministic and depended on the order in which you resume threads (If you are continuing one thread and stepping another, the process state would depend on which thread you resumed last). The test suite passes, so i think we can assume no one is depending on this stepping state now, but to avoid introducing nondeterministic bugs in the future, I think we should just abolish the stepping state and say that the process is running if any of its threads are stepping or running.

What do you think?


http://reviews.llvm.org/D12104





More information about the lldb-commits mailing list