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

[Oleksiy Vyalov via lldb-commits]

ovyalov added a comment.

Please see my 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");
----------------
Since AddThread is private method of NPL we can make it to return NativeThreadLinuxSP.

================
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2822
@@ -2834,2 +2821,3 @@
 
     // If we have a pending notification, remove this from the set.
+    SignalIfAllThreadsStopped();
----------------
It seems the comment is no longer actual.

================
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:3056
@@ +3055,3 @@
+        if (step_result.Success())
+            SetState(eStateRunning, true);
+        return step_result;
----------------
Should it be eStateStepping?

================
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:3060
@@ -3087,1 +3059,3 @@
+    default:
+        llvm_unreachable("Unhandled state for resume");
     }
----------------
Could you print state value here?

================
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.h:314
@@ -327,9 +313,3 @@
 
-        void
-        RequestStopOnAllRunningThreads();
-
-        Error
-        ThreadDidStop(lldb::tid_t tid, bool initiated_by_llgs);
-
         // Resume the thread with the given thread id using the request_thread_resume_function
         // called. If error_when_already_running is then then an error is raised if we think this
----------------
Please update the comment.

================
Comment at: source/Plugins/Process/Linux/NativeThreadLinux.h:115
@@ -125,2 +114,3 @@
 
+    typedef std::shared_ptr<NativeThreadLinux> NativeThreadLinuxSP;
 } // namespace process_linux
----------------
Nit  - include <memory> for shared_ptr?


http://reviews.llvm.org/D12104