[Lldb-commits] [PATCH] D86388: Fix use-after-free in ThreadPlan, and add test.

2020-08-27 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I want to separate out two things here. One is whether lldb should internally ask questions of a thread once we've invalidated the thread list before running, the other is how we present threads to the user while the process is running. I was only suggesting

[Lldb-commits] [PATCH] D86388: Fix use-after-free in ThreadPlan, and add test.

2020-08-26 Thread Nicholas Allegra via Phabricator via lldb-commits
comex added a comment. Disabling the thread list while the target is running sounds like a pretty complex change. For example, what should happen if a Python script calls `lldb.process.GetThreadAtIndex(n)` while the target is running, which currently works? And is it really the right

[Lldb-commits] [PATCH] D86388: Fix use-after-free in ThreadPlan, and add test.

2020-08-26 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. We should be able to calculate ShouldReportRun before we actually set the run going. That's better than just querying potentially stale threads. It would also be good to find a way to prevent ourselves from consulting the thread list after we've decided to invalidate

[Lldb-commits] [PATCH] D86388: Fix use-after-free in ThreadPlan, and add test.

2020-08-25 Thread Nicholas Allegra via Phabricator via lldb-commits
comex added a comment. The sequence I found is: - `WillResume` - `DoResume` sends `eBroadcastBitAsyncContinue` to `ProcessGDBRemote::AsyncThread` - `AsyncThread` calls `process->SetPrivateState(eStateRunning);` - …which sends `eBroadcastBitStateChanged` back to the main thread, handled by

[Lldb-commits] [PATCH] D86388: Fix use-after-free in ThreadPlan, and add test.

2020-08-25 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. What's calling into the thread plans when WillResume has already been called? That seems wrong, since the thread list is in an uncertain state, having been cleared out for resume and not reset after the stop. It seems to me it would be a better fix to ensure that we

[Lldb-commits] [PATCH] D86388: Fix use-after-free in ThreadPlan, and add test.

2020-08-25 Thread Nicholas Allegra via Phabricator via lldb-commits
comex added a comment. In D86388#2234418 , @jingham wrote: > I'm confused as to how this patch actually fixes the problem. When the > thread gets removed from the thread list, it should get Destroy called on it > - which should set m_destroy_called,

[Lldb-commits] [PATCH] D86388: Fix use-after-free in ThreadPlan, and add test.

2020-08-24 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I'm confused as to how this patch actually fixes the problem. When the thread gets removed from the thread list, it should get Destroy called on it - which should set m_destroy_called, causing IsValid to return false.. So I am not clear under what circumstances

[Lldb-commits] [PATCH] D86388: Fix use-after-free in ThreadPlan, and add test.

2020-08-22 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments. Comment at: lldb/include/lldb/Target/ThreadPlan.h:603 - Thread *m_thread; // Stores a cached value of the thread, which is set to -// nullptr when the thread resumes. Don't use this anywhere -// but